diff --git a/internal/state/state_skel.go b/internal/state/state_skel.go index 21c37b86c9..4b5ad1569e 100644 --- a/internal/state/state_skel.go +++ b/internal/state/state_skel.go @@ -538,7 +538,8 @@ func (s *stateSkel) isDaemonSetReady(uds *unstructured.Unstructured, reqLogger l ds.Status.CurrentNumberScheduled == 0 { return true, nil } - if ds.Status.DesiredNumberScheduled != 0 && ds.Status.DesiredNumberScheduled == ds.Status.NumberAvailable && + if ds.Status.ObservedGeneration >= ds.Generation && + ds.Status.DesiredNumberScheduled != 0 && ds.Status.DesiredNumberScheduled == ds.Status.NumberAvailable && ds.Status.UpdatedNumberScheduled == ds.Status.NumberAvailable { return true, nil } diff --git a/internal/state/state_skel_reconcile_test.go b/internal/state/state_skel_reconcile_test.go index d3cb90239d..0f61e231fc 100644 --- a/internal/state/state_skel_reconcile_test.go +++ b/internal/state/state_skel_reconcile_test.go @@ -142,8 +142,7 @@ func TestIsDaemonSetReadyErrors(t *testing.T) { }) } -// Characterizes a known gap: the nonzero-desired branch of isDaemonSetReady does not -// re-check ObservedGeneration, so status from a prior generation is reported ready. +// Matching pod counts from an older generation do not confirm the current rollout. func TestIsDaemonSetReadyStaleGeneration(t *testing.T) { staleDaemonSet := &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{Generation: 2}, @@ -158,7 +157,7 @@ func TestIsDaemonSetReadyStaleGeneration(t *testing.T) { skel := &stateSkel{} ready, err := skel.isDaemonSetReady(toUnstructuredDaemonSet(t, staleDaemonSet), logr.Discard()) require.NoError(t, err) - require.True(t, ready, "known gap: stale-generation status is currently treated as ready") + require.False(t, ready, "stale-generation status must not mark the current rollout ready") } func toUnstructuredDeployment(t *testing.T, deployment *appsv1.Deployment) *unstructured.Unstructured { diff --git a/internal/state/state_skel_test.go b/internal/state/state_skel_test.go index 9b8702b03f..3d1236cfb7 100644 --- a/internal/state/state_skel_test.go +++ b/internal/state/state_skel_test.go @@ -563,3 +563,75 @@ func TestGetSupportedGVKs(t *testing.T) { } assert.ElementsMatch(t, expectedGVKs, getSupportedGVKs()) } + +func TestDaemonSetReadinessRequiresObservedGeneration(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + generation int64 + observed int64 + desired int32 + available int32 + updated int32 + wantReady bool + }{ + "initial generation not observed": {1, 0, 2, 2, 2, false}, + "previous generation still ready": {3, 2, 2, 2, 2, false}, + "current generation ready": {3, 3, 2, 2, 2, true}, + "newer observed generation": {3, 4, 2, 2, 2, true}, + "current generation rolling out": {3, 3, 2, 2, 1, false}, + "current generation unavailable": {3, 3, 2, 1, 2, false}, + "OnDelete update not rolled out": {3, 3, 2, 2, 0, false}, + "zero nodes not observed": {3, 2, 0, 0, 0, false}, + "zero nodes observed": {3, 3, 0, 0, 0, true}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + ds := &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{Generation: tc.generation}, + Status: appsv1.DaemonSetStatus{ + ObservedGeneration: tc.observed, + DesiredNumberScheduled: tc.desired, + NumberAvailable: tc.available, + UpdatedNumberScheduled: tc.updated, + }, + } + ready, err := (&stateSkel{}).isDaemonSetReady(toUnstructuredDaemonSet(t, ds), logr.Discard()) + require.NoError(t, err) + require.Equal(t, tc.wantReady, ready) + }) + } +} + +func TestGetSyncStateWaitsForDaemonSetController(t *testing.T) { + t.Parallel() + + ds := &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{Name: "updating-operand", Namespace: "test-ns", Generation: 2}, + Status: appsv1.DaemonSetStatus{ + ObservedGeneration: 1, + DesiredNumberScheduled: 2, + CurrentNumberScheduled: 2, + NumberAvailable: 2, + UpdatedNumberScheduled: 2, + }, + } + k8sClient := fake.NewClientBuilder().WithScheme(skelTestScheme(t)). + WithStatusSubresource(&appsv1.DaemonSet{}).WithObjects(ds).Build() + skel := newTestSkel(t, k8sClient) + objects := []*unstructured.Unstructured{newDaemonSetUnstructured(ds.Name, ds.Namespace)} + + state, err := skel.getSyncState(context.Background(), objects) + require.NoError(t, err) + require.Equal(t, SyncState(SyncStateNotReady), state) + + current := &appsv1.DaemonSet{} + require.NoError(t, k8sClient.Get(context.Background(), client.ObjectKeyFromObject(ds), current)) + current.Status.ObservedGeneration = current.Generation + require.NoError(t, k8sClient.Status().Update(context.Background(), current)) + + state, err = skel.getSyncState(context.Background(), objects) + require.NoError(t, err) + require.Equal(t, SyncState(SyncStateReady), state) +}