Skip to content

🐛 report ClusterObjectSet object counts together, or omit entirely #3020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
perdasilva wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🐛 report ClusterObjectSet object counts together, or omit entirely #3020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions api/v1/clusterobjectset_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,10 @@ type ClusterObjectSetStatus struct {

// objectCounts reports aggregate object counts across all phases.
// Values are sums of the per-phase counts in observedPhases.
// It is omitted until counts have been computed; when present, all count
// fields are reported (including zeros).
// +optional
ObjectCounts ObjectCounts `json:"objectCounts,omitempty"`
ObjectCounts ObjectCounts `json:"objectCounts,omitzero"`

// observedPhases records the content hashes of resolved phases
// at first successful reconciliation. This is used to detect if
Expand Down Expand Up @@ -564,8 +566,10 @@ type ObservedPhase struct {
Digest string `json:"digest"`

// objectCounts reports the number of objects in this phase by state.
// +required
ObjectCounts ObjectCounts `json:"objectCounts"`
// It is omitted until counts have been computed; when present, all count
// fields are reported (including zeros).
// +optional
ObjectCounts ObjectCounts `json:"objectCounts,omitzero"`
}

// ObjectCounts reports the number of objects in a ClusterObjectSet or phase by state.
Expand All @@ -580,24 +584,24 @@ type ObjectCounts struct {
// cluster. During reconcile, this counts objects that the controller
// has found or created. During teardown, this decrements toward zero
// as objects are deleted.
// +optional
// +required
// +kubebuilder:validation:Minimum:=0
// +kubebuilder:validation:Maximum:=1000
Present int64 `json:"present,omitempty"`
Present int64 `json:"present"`

// synced is the number of objects in this phase whose cluster state
// matches the desired state.
// +optional
// +required
// +kubebuilder:validation:Minimum:=0
// +kubebuilder:validation:Maximum:=1000
Synced int64 `json:"synced,omitempty"`
Synced int64 `json:"synced"`

// ready is the number of objects in this phase that are synced
// and pass their assertions.
// +optional
// +required
// +kubebuilder:validation:Minimum:=0
// +kubebuilder:validation:Maximum:=1000
Ready int64 `json:"ready,omitempty"`
Ready int64 `json:"ready"`
}

// +genclient
Expand Down
42 changes: 42 additions & 0 deletions api/v1/clusterobjectset_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package v1

import (
"context"
"encoding/json"
"fmt"
"testing"

Expand Down Expand Up @@ -374,3 +375,44 @@ func configMap() unstructured.Unstructured {
},
}
}

// TestObjectCountsSerializationIncludesZeroCounts guards against the count fields
// being dropped from status when zero. total/present/synced/ready are all required,
// so a "0 of N" count (e.g. ready=0 while a phase is rolling out, or synced/ready=0
// during teardown) must still be serialized rather than omitted via omitempty.
func TestObjectCountsSerializationIncludesZeroCounts(t *testing.T) {
data, err := json.Marshal(ObjectCounts{Total: 1, Present: 1, Synced: 1, Ready: 0})
require.NoError(t, err)
require.JSONEq(t, `{"total":1,"present":1,"synced":1,"ready":0}`, string(data))

// All-zero counts must still serialize every field (nothing omitted).
data, err = json.Marshal(ObjectCounts{})
require.NoError(t, err)
require.JSONEq(t, `{"total":0,"present":0,"synced":0,"ready":0}`, string(data))
}

// TestObjectCountsOmittedWhenZero verifies the parent objectCounts field is optional:
// a zero-valued ObjectCounts is omitted entirely (via omitzero on a value struct —
// omitempty does not omit non-pointer structs), but when any count is non-zero the whole
// struct is serialized with all of its (required) fields, including zeros.
func TestObjectCountsOmittedWhenZero(t *testing.T) {
// Aggregate status: zero objectCounts is omitted.
data, err := json.Marshal(ClusterObjectSetStatus{})
require.NoError(t, err)
require.NotContains(t, string(data), "objectCounts")

// Aggregate status: a non-zero objectCounts serializes with every field.
data, err = json.Marshal(ClusterObjectSetStatus{ObjectCounts: ObjectCounts{Total: 1}})
require.NoError(t, err)
require.Contains(t, string(data), `"objectCounts":{"total":1,"present":0,"synced":0,"ready":0}`)

// Per-phase: zero objectCounts is omitted (name/digest still present).
data, err = json.Marshal(ObservedPhase{Name: "deploy", Digest: "sha256:abc"})
require.NoError(t, err)
require.NotContains(t, string(data), "objectCounts")

// Per-phase: a non-zero objectCounts serializes with every field.
data, err = json.Marshal(ObservedPhase{Name: "deploy", Digest: "sha256:abc", ObjectCounts: ObjectCounts{Present: 2}})
require.NoError(t, err)
require.Contains(t, string(data), `"objectCounts":{"total":0,"present":2,"synced":0,"ready":0}`)
}
2 changes: 2 additions & 0 deletions applyconfigurations/api/v1/clusterobjectsetstatus.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions applyconfigurations/api/v1/observedphase.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 12 additions & 3 deletions ...1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,8 @@ spec:
description: |-
objectCounts reports aggregate object counts across all phases.
Values are sums of the per-phase counts in observedPhases.
It is omitted until counts have been computed; when present, all count
fields are reported (including zeros).
properties:
present:
description: |-
Expand Down Expand Up @@ -665,6 +667,9 @@ spec:
minimum: 0
type: integer
required:
- present
- ready
- synced
Comment thread
perdasilva marked this conversation as resolved.
- total
type: object
observedPhases:
Expand Down Expand Up @@ -701,8 +706,10 @@ spec:
character.
rule: '!format.dns1123Label().validate(self).hasValue()'
objectCounts:
description: objectCounts reports the number of objects in this
phase by state.
description: |-
objectCounts reports the number of objects in this phase by state.
It is omitted until counts have been computed; when present, all count
fields are reported (including zeros).
properties:
present:
description: |-
Expand Down Expand Up @@ -737,12 +744,14 @@ spec:
minimum: 0
type: integer
required:
- present
- ready
- synced
- total
type: object
required:
- digest
- name
- objectCounts
type: object
maxItems: 20
type: array
Expand Down
15 changes: 12 additions & 3 deletions manifests/experimental-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2042,6 +2042,8 @@ spec:
description: |-
objectCounts reports aggregate object counts across all phases.
Values are sums of the per-phase counts in observedPhases.
It is omitted until counts have been computed; when present, all count
fields are reported (including zeros).
properties:
present:
description: |-
Expand Down Expand Up @@ -2076,6 +2078,9 @@ spec:
minimum: 0
type: integer
required:
- present
- ready
- synced
- total
type: object
observedPhases:
Expand Down Expand Up @@ -2112,8 +2117,10 @@ spec:
character.
rule: '!format.dns1123Label().validate(self).hasValue()'
objectCounts:
description: objectCounts reports the number of objects in this
phase by state.
description: |-
objectCounts reports the number of objects in this phase by state.
It is omitted until counts have been computed; when present, all count
fields are reported (including zeros).
properties:
present:
description: |-
Expand Down Expand Up @@ -2148,12 +2155,14 @@ spec:
minimum: 0
type: integer
required:
- present
- ready
- synced
- total
type: object
required:
- digest
- name
- objectCounts
type: object
maxItems: 20
type: array
Expand Down
15 changes: 12 additions & 3 deletions manifests/experimental.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2003,6 +2003,8 @@ spec:
description: |-
objectCounts reports aggregate object counts across all phases.
Values are sums of the per-phase counts in observedPhases.
It is omitted until counts have been computed; when present, all count
fields are reported (including zeros).
properties:
present:
description: |-
Expand Down Expand Up @@ -2037,6 +2039,9 @@ spec:
minimum: 0
type: integer
required:
- present
- ready
- synced
- total
type: object
observedPhases:
Expand Down Expand Up @@ -2073,8 +2078,10 @@ spec:
character.
rule: '!format.dns1123Label().validate(self).hasValue()'
objectCounts:
description: objectCounts reports the number of objects in this
phase by state.
description: |-
objectCounts reports the number of objects in this phase by state.
It is omitted until counts have been computed; when present, all count
fields are reported (including zeros).
properties:
present:
description: |-
Expand Down Expand Up @@ -2109,12 +2116,14 @@ spec:
minimum: 0
type: integer
required:
- present
- ready
- synced
- total
type: object
required:
- digest
- name
- objectCounts
type: object
maxItems: 20
type: array
Expand Down
Loading