Skip to content

Commit

Permalink
Ensure that Daemonset controller has acted on the Daemonset.
Browse files Browse the repository at this point in the history
In status.Compute() -> checkGenericProperties() -> checkGeneration(),
we check that metadata.generation is equal to status.observedGeneration
but don't return an InProgress status if either of them is unset.

This may be reasonable for generic resource where we are unsure if these
fields would be set, but for daemonsets, we *know* that these fields get
set by the controller and so we should ensure that.

This addresses #548
  • Loading branch information
rohitagarwal003 committed Feb 18, 2022
1 parent cb75cc8 commit 4a633f6
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 1 deletion.
42 changes: 42 additions & 0 deletions pkg/kstatus/status/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,16 @@ func replicasetConditions(u *unstructured.Unstructured) (*Result, error) {

// daemonsetConditions return standardized Conditions for DaemonSet
func daemonsetConditions(u *unstructured.Unstructured) (*Result, error) {
// We check that the latest generation is equal to observed generation as
// part of checking generic properties but in that case, we are lenient and
// skip the check if those fields are unset. For daemonset, we know that if
// the daemonset controller has acted on a resource, these fields would not
// be unset. So, we ensure that here.
res, err := checkGenerationSet(u)
if err != nil || res != nil {
return res, err
}

obj := u.UnstructuredContent()

// replicas
Expand Down Expand Up @@ -345,6 +355,38 @@ func daemonsetConditions(u *unstructured.Unstructured) (*Result, error) {
}, nil
}

// checkGenerationSet checks that the metadata.generation and
// status.observedGeneration fields are set.
func checkGenerationSet(u *unstructured.Unstructured) (*Result, error) {
_, found, err := unstructured.NestedInt64(u.Object, "metadata", "generation")
if err != nil {
return nil, fmt.Errorf("looking up metadata.generation from resource: %w", err)
}
if !found {
message := fmt.Sprintf("%s metadata.generation not found", u.GetKind())
return &Result{
Status: InProgressStatus,
Message: message,
Conditions: []Condition{newReconcilingCondition("NoGeneration", message)},
}, nil
}

_, found, err = unstructured.NestedInt64(u.Object, "status", "observedGeneration")
if err != nil {
return nil, fmt.Errorf("looking up status.observedGeneration from resource: %w", err)
}
if !found {
message := fmt.Sprintf("%s status.observedGeneration not found", u.GetKind())
return &Result{
Status: InProgressStatus,
Message: message,
Conditions: []Condition{newReconcilingCondition("NoObservedGeneration", message)},
}, nil
}

return nil, nil
}

// pvcConditions return standardized Conditions for PVC
func pvcConditions(u *unstructured.Unstructured) (*Result, error) {
obj := u.UnstructuredContent()
Expand Down
29 changes: 28 additions & 1 deletion pkg/kstatus/status/status_compute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,21 @@ status:
numberReady: 4
observedGeneration: 1
`
var dsDifferentGeneration = `
apiVersion: apps/v1
kind: DaemonSet
metadata:
name: test
namespace: qual
generation: 2
status:
desiredNumberScheduled: 4
currentNumberScheduled: 4
updatedNumberScheduled: 4
numberAvailable: 4
numberReady: 4
observedGeneration: 1
`

var dsLessReady = `
apiVersion: apps/v1
Expand Down Expand Up @@ -543,7 +558,7 @@ func TestDaemonsetStatus(t *testing.T) {
expectedConditions: []Condition{{
Type: ConditionReconciling,
Status: corev1.ConditionTrue,
Reason: "NoDesiredNumber",
Reason: "NoObservedGeneration",
}},
absentConditionTypes: []ConditionType{
ConditionStalled,
Expand Down Expand Up @@ -582,6 +597,18 @@ func TestDaemonsetStatus(t *testing.T) {
ConditionStalled,
},
},
"dsDifferentGeneration": {
spec: dsDifferentGeneration,
expectedStatus: InProgressStatus,
expectedConditions: []Condition{{
Type: ConditionReconciling,
Status: corev1.ConditionTrue,
Reason: "LatestGenerationNotObserved",
}},
absentConditionTypes: []ConditionType{
ConditionStalled,
},
},
"dsLessAvailable": {
spec: dsLessAvailable,
expectedStatus: InProgressStatus,
Expand Down

0 comments on commit 4a633f6

Please sign in to comment.