From ceeb05a3d75b87e627d96a976384b17b10d6c439 Mon Sep 17 00:00:00 2001 From: ravisantoshgudimetla Date: Mon, 10 May 2021 13:51:19 -0400 Subject: [PATCH 1/2] DaemonSet: Fix surgeCount function If the surge is not requested, we should return 0. We are returning an error now as r.MaxSurge is passed down as nil. This commit fixes the issue by setting the surgeCount to 0 if r.MaxSurge is nil. --- pkg/controller/daemon/util/daemonset_util.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/controller/daemon/util/daemonset_util.go b/pkg/controller/daemon/util/daemonset_util.go index f2b58237c20..acff170506c 100644 --- a/pkg/controller/daemon/util/daemonset_util.go +++ b/pkg/controller/daemon/util/daemonset_util.go @@ -144,6 +144,10 @@ func SurgeCount(ds *apps.DaemonSet, numberToSchedule int) (int, error) { if r == nil { return 0, nil } + // If surge is not requested, we should default to 0. + if r.MaxSurge == nil { + return 0, nil + } return intstrutil.GetScaledValueFromIntOrPercent(r.MaxSurge, numberToSchedule, true) } From 6f35e1aea0486f21a4261d6d62d4ab19583826c5 Mon Sep 17 00:00:00 2001 From: ravisantoshgudimetla Date: Mon, 10 May 2021 16:10:40 -0400 Subject: [PATCH 2/2] Run unit test with DSMaxSurgeFlag enabled and disabled --- .../daemon/daemon_controller_test.go | 50 ++++++++++--------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/pkg/controller/daemon/daemon_controller_test.go b/pkg/controller/daemon/daemon_controller_test.go index 1b5c27207bc..1d17099b530 100644 --- a/pkg/controller/daemon/daemon_controller_test.go +++ b/pkg/controller/daemon/daemon_controller_test.go @@ -847,33 +847,37 @@ func TestDaemonSetPodCreateExpectationsError(t *testing.T) { } func TestSimpleDaemonSetUpdatesStatusAfterLaunchingPods(t *testing.T) { - for _, strategy := range updateStrategies() { - ds := newDaemonSet("foo") - ds.Spec.UpdateStrategy = *strategy - manager, podControl, clientset, err := newTestController(ds) - if err != nil { - t.Fatalf("error creating DaemonSets controller: %v", err) - } + dsMaxSurgeFeatureFlags := []bool{false, true} + for _, isEnabled := range dsMaxSurgeFeatureFlags { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DaemonSetUpdateSurge, isEnabled)() + for _, strategy := range updateStrategies() { + ds := newDaemonSet("foo") + ds.Spec.UpdateStrategy = *strategy + manager, podControl, clientset, err := newTestController(ds) + if err != nil { + t.Fatalf("error creating DaemonSets controller: %v", err) + } - var updated *apps.DaemonSet - clientset.PrependReactor("update", "daemonsets", func(action core.Action) (handled bool, ret runtime.Object, err error) { - if action.GetSubresource() != "status" { + var updated *apps.DaemonSet + clientset.PrependReactor("update", "daemonsets", func(action core.Action) (handled bool, ret runtime.Object, err error) { + if action.GetSubresource() != "status" { + return false, nil, nil + } + if u, ok := action.(core.UpdateAction); ok { + updated = u.GetObject().(*apps.DaemonSet) + } return false, nil, nil - } - if u, ok := action.(core.UpdateAction); ok { - updated = u.GetObject().(*apps.DaemonSet) - } - return false, nil, nil - }) + }) - manager.dsStore.Add(ds) - addNodes(manager.nodeStore, 0, 5, nil) - expectSyncDaemonSets(t, manager, ds, podControl, 5, 0, 0) + manager.dsStore.Add(ds) + addNodes(manager.nodeStore, 0, 5, nil) + expectSyncDaemonSets(t, manager, ds, podControl, 5, 0, 0) - // Make sure the single sync() updated Status already for the change made - // during the manage() phase. - if got, want := updated.Status.CurrentNumberScheduled, int32(5); got != want { - t.Errorf("Status.CurrentNumberScheduled = %v, want %v", got, want) + // Make sure the single sync() updated Status already for the change made + // during the manage() phase. + if got, want := updated.Status.CurrentNumberScheduled, int32(5); got != want { + t.Errorf("Status.CurrentNumberScheduled = %v, want %v", got, want) + } } } }