From 31a22885dc30af883236795367e898e45f064b76 Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes Date: Wed, 17 Jul 2024 17:02:41 -0700 Subject: [PATCH] WIP --- pkg/controllers/rollout/controller.go | 22 ++- pkg/controllers/rollout/controller_test.go | 171 +++++++++++++++++++++ 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/pkg/controllers/rollout/controller.go b/pkg/controllers/rollout/controller.go index fc7b862dd..97ca6e59c 100644 --- a/pkg/controllers/rollout/controller.go +++ b/pkg/controllers/rollout/controller.go @@ -9,6 +9,7 @@ package rollout import ( "context" "fmt" + "math" "strconv" "time" @@ -161,10 +162,29 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim // to avoid the case that the rollout process stalling because the time based binding readiness does not trigger any event. // We wait for 1/5 of the UnavailablePeriodSeconds so we can catch the next ready one early. // TODO: only wait the time we need to wait for the first applied but not ready binding to be ready - return runtime.Result{RequeueAfter: time.Duration(*crp.Spec.Strategy.RollingUpdate.UnavailablePeriodSeconds) * time.Second / 5}, + return runtime.Result{RequeueAfter: waitForFirstAppliedButNotReadyBinding(allBindings, time.Now().Add(-time.Duration(*crp.Spec.Strategy.RollingUpdate.UnavailablePeriodSeconds)*time.Second))}, r.updateBindings(ctx, toBeUpdatedBindings) } +// `waitForFirstAppliedButNotReadyBinding` returns the minimum amount of time that needs to elapse before the first +// binding that has been applied but not yet marked as ready becomes ready. +func waitForFirstAppliedButNotReadyBinding(bindings []*fleetv1beta1.ClusterResourceBinding, readyTimeCutOff time.Time) time.Duration { + minValue := time.Duration(math.MaxInt64) + for _, binding := range bindings { + appliedCondition := binding.GetCondition(string(fleetv1beta1.ResourceBindingApplied)) + if condition.IsConditionStatusTrue(appliedCondition, binding.Generation) { + waitTime, bindingReady := isBindingReady(binding, readyTimeCutOff) + if !bindingReady { + if waitTime < minValue { + minValue = waitTime + } + } + } + } + klog.V(2).InfoS("Picked the first applied but ready binding", "waitTime", minValue) + return minValue * time.Second +} + func (r *Reconciler) checkAndUpdateStaleBindingsStatus(ctx context.Context, bindings []*fleetv1beta1.ClusterResourceBinding) error { if len(bindings) == 0 { return nil diff --git a/pkg/controllers/rollout/controller_test.go b/pkg/controllers/rollout/controller_test.go index 1b15d14d2..fe4659ad8 100644 --- a/pkg/controllers/rollout/controller_test.go +++ b/pkg/controllers/rollout/controller_test.go @@ -8,6 +8,7 @@ package rollout import ( "context" "errors" + "math" "testing" "time" @@ -726,6 +727,176 @@ func TestIsBindingReady(t *testing.T) { } } +func TestWaitForFirstAppliedButNotReadyBinding(t *testing.T) { + tests := map[string]struct { + bindings []*fleetv1beta1.ClusterResourceBinding + readyTimeCutOff time.Time + wantWaitDuration time.Duration + }{ + "all bindings ready": { + bindings: []*fleetv1beta1.ClusterResourceBinding{ + { + Status: fleetv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(fleetv1beta1.ResourceBindingApplied), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + { + Type: string(fleetv1beta1.ResourceBindingAvailable), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + }, + }, + }, + { + Status: fleetv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(fleetv1beta1.ResourceBindingApplied), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + { + Type: string(fleetv1beta1.ResourceBindingAvailable), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + }, + }, + }, + }, + readyTimeCutOff: time.Now(), + wantWaitDuration: time.Duration(math.MaxInt64), + }, + "one binding not ready": { + bindings: []*fleetv1beta1.ClusterResourceBinding{ + { + Status: fleetv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(fleetv1beta1.ResourceBindingApplied), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + { + Type: string(fleetv1beta1.ResourceBindingAvailable), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + }, + }, + }, + }, + { + Status: fleetv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(fleetv1beta1.ResourceBindingApplied), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + { + Type: string(fleetv1beta1.ResourceBindingAvailable), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + }, + }, + }, + }, + readyTimeCutOff: time.Now(), + wantWaitDuration: 0, + }, + "multiple bindings not ready": { + bindings: []*fleetv1beta1.ClusterResourceBinding{ + { + Status: fleetv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(fleetv1beta1.ResourceBindingApplied), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + { + Type: string(fleetv1beta1.ResourceBindingAvailable), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + }, + }, + }, + }, + { + Status: fleetv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(fleetv1beta1.ResourceBindingApplied), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + { + Type: string(fleetv1beta1.ResourceBindingAvailable), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + }, + }, + }, + }, + }, + readyTimeCutOff: time.Now(), + wantWaitDuration: time.Duration(math.MaxInt64), + }, + "one binding not applied": { + bindings: []*fleetv1beta1.ClusterResourceBinding{ + { + Status: fleetv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(fleetv1beta1.ResourceBindingApplied), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + }, + { + Type: string(fleetv1beta1.ResourceBindingAvailable), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + }, + }, + }, + { + Status: fleetv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: string(fleetv1beta1.ResourceBindingApplied), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + { + Type: string(fleetv1beta1.ResourceBindingAvailable), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + }, + }, + }, + }, + readyTimeCutOff: time.Now(), + wantWaitDuration: time.Duration(math.MaxInt64), + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + gotWaitDuration := waitForFirstAppliedButNotReadyBinding(tt.bindings, tt.readyTimeCutOff) + if gotWaitDuration != tt.wantWaitDuration { + t.Errorf("waitForFirstAppliedButNotReadyBinding test `%s` gotWaitDuration = %v, wantWaitDuration %v", name, gotWaitDuration, tt.wantWaitDuration) + } + }) + } +} + func TestPickBindingsToRoll(t *testing.T) { noMaxUnavailableCRP := clusterResourcePlacementForTest("test", createPlacementPolicyForTest(fleetv1beta1.PickNPlacementType, 5))