Skip to content

Commit b000bd9

Browse files
authored
feat: changes to rollout controller for eviction (#959)
1 parent cb0c5f7 commit b000bd9

File tree

13 files changed

+1531
-477
lines changed

13 files changed

+1531
-477
lines changed

cmd/hubagent/workload/setup.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"go.goms.io/fleet/pkg/controllers/clusterinventory/clusterprofile"
2929
"go.goms.io/fleet/pkg/controllers/clusterresourcebindingwatcher"
3030
"go.goms.io/fleet/pkg/controllers/clusterresourceplacement"
31+
"go.goms.io/fleet/pkg/controllers/clusterresourceplacementeviction"
3132
"go.goms.io/fleet/pkg/controllers/clusterresourceplacementwatcher"
3233
"go.goms.io/fleet/pkg/controllers/clusterschedulingpolicysnapshot"
3334
"go.goms.io/fleet/pkg/controllers/memberclusterplacement"
@@ -206,6 +207,14 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
206207
return err
207208
}
208209

210+
klog.Info("Setting up cluster resource placement eviction controller")
211+
if err := (&clusterresourceplacementeviction.Reconciler{
212+
Client: mgr.GetClient(),
213+
}).SetupWithManager(mgr); err != nil {
214+
klog.ErrorS(err, "Unable to set up cluster resource placement eviction controller")
215+
return err
216+
}
217+
209218
// Set up the work generator
210219
klog.Info("Setting up work generator")
211220
if err := (&workgenerator.Reconciler{

pkg/controllers/clusterresourceplacementeviction/controller.go

Lines changed: 19 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,6 @@ import (
2828
"go.goms.io/fleet/pkg/utils/defaulter"
2929
)
3030

31-
const (
32-
clusterResourcePlacementEvictionValidReason = "ClusterResourcePlacementEvictionValid"
33-
clusterResourcePlacementEvictionInvalidReason = "ClusterResourcePlacementEvictionInvalid"
34-
clusterResourcePlacementEvictionExecutedReason = "ClusterResourcePlacementEvictionExecuted"
35-
clusterResourcePlacementEvictionNotExecutedReason = "ClusterResourcePlacementEvictionNotExecuted"
36-
37-
evictionInvalidMissingCRPMessage = "Failed to find ClusterResourcePlacement targeted by eviction"
38-
evictionInvalidDeletingCRPMessage = "Found deleting ClusterResourcePlacement targeted by eviction"
39-
evictionInvalidMissingCRBMessage = "Failed to find scheduler decision for placement in cluster targeted by eviction"
40-
evictionInvalidMultipleCRBMessage = "Found more than one scheduler decision for placement in cluster targeted by eviction"
41-
evictionValidMessage = "Eviction is valid"
42-
evictionAllowedNoPDBMessage = "Eviction is allowed, no ClusterResourcePlacementDisruptionBudget specified"
43-
evictionAllowedPlacementRemovedMessage = "Eviction is allowed, resources propagated by placement is currently being removed from cluster targeted by eviction"
44-
evictionAllowedPlacementFailedMessage = "Eviction is allowed, placement has failed"
45-
evictionBlockedMisconfiguredPDBSpecifiedMessage = "Eviction is blocked by misconfigured ClusterResourcePlacementDisruptionBudget, either MaxUnavailable is specified or MinAvailable is specified as a percentage for PickAll ClusterResourcePlacement"
46-
evictionBlockedMissingPlacementMessage = "Eviction is blocked, placement has not propagated resources to target cluster yet"
47-
48-
evictionAllowedPDBSpecifiedFmt = "Eviction is allowed by specified ClusterResourcePlacementDisruptionBudget, availablePlacements: %d, totalPlacements: %d"
49-
evictionBlockedPDBSpecifiedFmt = "Eviction is blocked by specified ClusterResourcePlacementDisruptionBudget, availablePlacements: %d, totalPlacements: %d"
50-
)
51-
5231
// Reconciler reconciles a ClusterResourcePlacementEviction object.
5332
type Reconciler struct {
5433
client.Client
@@ -97,8 +76,8 @@ func (r *Reconciler) validateEviction(ctx context.Context, eviction *placementv1
9776
var crp placementv1beta1.ClusterResourcePlacement
9877
if err := r.Client.Get(ctx, types.NamespacedName{Name: eviction.Spec.PlacementName}, &crp); err != nil {
9978
if k8serrors.IsNotFound(err) {
100-
klog.V(2).InfoS(evictionInvalidMissingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
101-
markEvictionInvalid(eviction, evictionInvalidMissingCRPMessage)
79+
klog.V(2).InfoS(condition.EvictionInvalidMissingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
80+
markEvictionInvalid(eviction, condition.EvictionInvalidMissingCRPMessage)
10281
return validationResult, nil
10382
}
10483
return nil, controller.NewAPIServerError(true, err)
@@ -108,8 +87,8 @@ func (r *Reconciler) validateEviction(ctx context.Context, eviction *placementv1
10887
defaulter.SetDefaultsClusterResourcePlacement(&crp)
10988

11089
if crp.DeletionTimestamp != nil {
111-
klog.V(2).InfoS(evictionInvalidDeletingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
112-
markEvictionInvalid(eviction, evictionInvalidDeletingCRPMessage)
90+
klog.V(2).InfoS(condition.EvictionInvalidDeletingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
91+
markEvictionInvalid(eviction, condition.EvictionInvalidDeletingCRPMessage)
11392
return validationResult, nil
11493
}
11594
validationResult.crp = &crp
@@ -126,15 +105,15 @@ func (r *Reconciler) validateEviction(ctx context.Context, eviction *placementv1
126105
if evictionTargetBinding == nil {
127106
evictionTargetBinding = &crbList.Items[i]
128107
} else {
129-
klog.V(2).InfoS(evictionInvalidMultipleCRBMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
130-
markEvictionInvalid(eviction, evictionInvalidMultipleCRBMessage)
108+
klog.V(2).InfoS(condition.EvictionInvalidMultipleCRBMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
109+
markEvictionInvalid(eviction, condition.EvictionInvalidMultipleCRBMessage)
131110
return validationResult, nil
132111
}
133112
}
134113
}
135114
if evictionTargetBinding == nil {
136115
klog.V(2).InfoS("Failed to find cluster resource binding for cluster targeted by eviction", "clusterResourcePlacementEviction", eviction.Name, "targetCluster", eviction.Spec.ClusterName)
137-
markEvictionInvalid(eviction, evictionInvalidMissingCRBMessage)
116+
markEvictionInvalid(eviction, condition.EvictionInvalidMissingCRBMessage)
138117
return validationResult, nil
139118
}
140119
validationResult.crb = evictionTargetBinding
@@ -179,14 +158,14 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
179158
if evictionTargetBinding.GetDeletionTimestamp() != nil {
180159
klog.V(2).InfoS("ClusterResourceBinding targeted by eviction is being deleted",
181160
"clusterResourcePlacementEviction", eviction.Name, "clusterResourceBinding", evictionTargetBinding.Name, "targetCluster", eviction.Spec.ClusterName)
182-
markEvictionExecuted(eviction, evictionAllowedPlacementRemovedMessage)
161+
markEvictionExecuted(eviction, condition.EvictionAllowedPlacementRemovedMessage)
183162
return nil
184163
}
185164

186165
if !isPlacementPresent(evictionTargetBinding) {
187166
klog.V(2).InfoS("No resources have been placed for ClusterResourceBinding in target cluster",
188167
"clusterResourcePlacementEviction", eviction.Name, "clusterResourceBinding", evictionTargetBinding.Name, "targetCluster", eviction.Spec.ClusterName)
189-
markEvictionNotExecuted(eviction, evictionBlockedMissingPlacementMessage)
168+
markEvictionNotExecuted(eviction, condition.EvictionBlockedMissingPlacementMessage)
190169
return nil
191170
}
192171

@@ -197,7 +176,7 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
197176
if err := r.deleteClusterResourceBinding(ctx, evictionTargetBinding); err != nil {
198177
return err
199178
}
200-
markEvictionExecuted(eviction, evictionAllowedPlacementFailedMessage)
179+
markEvictionExecuted(eviction, condition.EvictionAllowedPlacementFailedMessage)
201180
return nil
202181
}
203182

@@ -207,7 +186,7 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
207186
if err = r.deleteClusterResourceBinding(ctx, evictionTargetBinding); err != nil {
208187
return err
209188
}
210-
markEvictionExecuted(eviction, evictionAllowedNoPDBMessage)
189+
markEvictionExecuted(eviction, condition.EvictionAllowedNoPDBMessage)
211190
return nil
212191
}
213192
return controller.NewAPIServerError(true, err)
@@ -216,7 +195,7 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
216195
// handle special case for PickAll CRP.
217196
if crp.Spec.Policy.PlacementType == placementv1beta1.PickAllPlacementType {
218197
if db.Spec.MaxUnavailable != nil || (db.Spec.MinAvailable != nil && db.Spec.MinAvailable.Type == intstr.String) {
219-
markEvictionNotExecuted(eviction, evictionBlockedMisconfiguredPDBSpecifiedMessage)
198+
markEvictionNotExecuted(eviction, condition.EvictionBlockedMisconfiguredPDBSpecifiedMessage)
220199
return nil
221200
}
222201
}
@@ -227,9 +206,9 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
227206
if err := r.deleteClusterResourceBinding(ctx, evictionTargetBinding); err != nil {
228207
return err
229208
}
230-
markEvictionExecuted(eviction, fmt.Sprintf(evictionAllowedPDBSpecifiedFmt, availableBindings, totalBindings))
209+
markEvictionExecuted(eviction, fmt.Sprintf(condition.EvictionAllowedPDBSpecifiedMessageFmt, availableBindings, totalBindings))
231210
} else {
232-
markEvictionNotExecuted(eviction, fmt.Sprintf(evictionBlockedPDBSpecifiedFmt, availableBindings, totalBindings))
211+
markEvictionNotExecuted(eviction, fmt.Sprintf(condition.EvictionBlockedPDBSpecifiedMessageFmt, availableBindings, totalBindings))
233212
}
234213
return nil
235214
}
@@ -315,8 +294,8 @@ func markEvictionValid(eviction *placementv1alpha1.ClusterResourcePlacementEvict
315294
Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid),
316295
Status: metav1.ConditionTrue,
317296
ObservedGeneration: eviction.Generation,
318-
Reason: clusterResourcePlacementEvictionValidReason,
319-
Message: evictionValidMessage,
297+
Reason: condition.ClusterResourcePlacementEvictionValidReason,
298+
Message: condition.EvictionValidMessage,
320299
}
321300
eviction.SetConditions(cond)
322301

@@ -329,7 +308,7 @@ func markEvictionInvalid(eviction *placementv1alpha1.ClusterResourcePlacementEvi
329308
Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid),
330309
Status: metav1.ConditionFalse,
331310
ObservedGeneration: eviction.Generation,
332-
Reason: clusterResourcePlacementEvictionInvalidReason,
311+
Reason: condition.ClusterResourcePlacementEvictionInvalidReason,
333312
Message: message,
334313
}
335314
eviction.SetConditions(cond)
@@ -342,7 +321,7 @@ func markEvictionExecuted(eviction *placementv1alpha1.ClusterResourcePlacementEv
342321
Type: string(placementv1alpha1.PlacementEvictionConditionTypeExecuted),
343322
Status: metav1.ConditionTrue,
344323
ObservedGeneration: eviction.Generation,
345-
Reason: clusterResourcePlacementEvictionExecutedReason,
324+
Reason: condition.ClusterResourcePlacementEvictionExecutedReason,
346325
Message: message,
347326
}
348327
eviction.SetConditions(cond)
@@ -355,7 +334,7 @@ func markEvictionNotExecuted(eviction *placementv1alpha1.ClusterResourcePlacemen
355334
Type: string(placementv1alpha1.PlacementEvictionConditionTypeExecuted),
356335
Status: metav1.ConditionFalse,
357336
ObservedGeneration: eviction.Generation,
358-
Reason: clusterResourcePlacementEvictionNotExecutedReason,
337+
Reason: condition.ClusterResourcePlacementEvictionNotExecutedReason,
359338
Message: message,
360339
}
361340
eviction.SetConditions(cond)

pkg/controllers/clusterresourceplacementeviction/controller_intergration_test.go

Lines changed: 18 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import (
99
"fmt"
1010
"time"
1111

12-
"github.com/google/go-cmp/cmp"
13-
"github.com/google/go-cmp/cmp/cmpopts"
1412
. "github.com/onsi/ginkgo/v2"
1513
. "github.com/onsi/gomega"
1614
k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -23,6 +21,8 @@ import (
2321

2422
placementv1alpha1 "go.goms.io/fleet/apis/placement/v1alpha1"
2523
placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
24+
"go.goms.io/fleet/pkg/utils/condition"
25+
testutilseviction "go.goms.io/fleet/test/utils/eviction"
2626
)
2727

2828
const (
@@ -32,18 +32,6 @@ const (
3232
evictionNameTemplate = "eviction-%d"
3333
)
3434

35-
var (
36-
lessFuncCondition = func(a, b metav1.Condition) bool {
37-
return a.Type < b.Type
38-
}
39-
40-
evictionStatusCmpOptions = cmp.Options{
41-
cmpopts.SortSlices(lessFuncCondition),
42-
cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
43-
cmpopts.EquateEmpty(),
44-
}
45-
)
46-
4735
const (
4836
eventuallyDuration = time.Minute * 2
4937
eventuallyInterval = time.Millisecond * 250
@@ -121,7 +109,10 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", func() {
121109
})
122110

123111
By("Check eviction status", func() {
124-
evictionStatusUpdatedActual := evictionStatusUpdatedActual(&isValidEviction{bool: true, msg: evictionValidMessage}, &isExecutedEviction{bool: false, msg: fmt.Sprintf(evictionBlockedPDBSpecifiedFmt, 0, 1)})
112+
evictionStatusUpdatedActual := testutilseviction.StatusUpdatedActual(
113+
ctx, k8sClient, evictionName,
114+
&testutilseviction.IsValidEviction{IsValid: true, Msg: condition.EvictionValidMessage},
115+
&testutilseviction.IsExecutedEviction{IsExecuted: false, Msg: fmt.Sprintf(condition.EvictionBlockedPDBSpecifiedMessageFmt, 0, 1)})
125116
Eventually(evictionStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed())
126117
})
127118

@@ -211,7 +202,10 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", func() {
211202
})
212203

213204
By("Check eviction status", func() {
214-
evictionStatusUpdatedActual := evictionStatusUpdatedActual(&isValidEviction{bool: true, msg: evictionValidMessage}, &isExecutedEviction{bool: true, msg: fmt.Sprintf(evictionAllowedPDBSpecifiedFmt, 1, 1)})
205+
evictionStatusUpdatedActual := testutilseviction.StatusUpdatedActual(
206+
ctx, k8sClient, evictionName,
207+
&testutilseviction.IsValidEviction{IsValid: true, Msg: condition.EvictionValidMessage},
208+
&testutilseviction.IsExecutedEviction{IsExecuted: true, Msg: fmt.Sprintf(condition.EvictionAllowedPDBSpecifiedMessageFmt, 1, 1)})
215209
Eventually(evictionStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed())
216210
})
217211

@@ -286,7 +280,10 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", func() {
286280
})
287281

288282
By("Check eviction status", func() {
289-
evictionStatusUpdatedActual := evictionStatusUpdatedActual(&isValidEviction{bool: true, msg: evictionValidMessage}, &isExecutedEviction{bool: false, msg: fmt.Sprintf(evictionBlockedPDBSpecifiedFmt, 0, 1)})
283+
evictionStatusUpdatedActual := testutilseviction.StatusUpdatedActual(
284+
ctx, k8sClient, evictionName,
285+
&testutilseviction.IsValidEviction{IsValid: true, Msg: condition.EvictionValidMessage},
286+
&testutilseviction.IsExecutedEviction{IsExecuted: false, Msg: fmt.Sprintf(condition.EvictionBlockedPDBSpecifiedMessageFmt, 0, 1)})
290287
Eventually(evictionStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed())
291288
})
292289

@@ -399,7 +396,10 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", func() {
399396
})
400397

401398
By("Check eviction status", func() {
402-
evictionStatusUpdatedActual := evictionStatusUpdatedActual(&isValidEviction{bool: true, msg: evictionValidMessage}, &isExecutedEviction{bool: true, msg: fmt.Sprintf(evictionAllowedPDBSpecifiedFmt, 2, 2)})
399+
evictionStatusUpdatedActual := testutilseviction.StatusUpdatedActual(
400+
ctx, k8sClient, evictionName,
401+
&testutilseviction.IsValidEviction{IsValid: true, Msg: condition.EvictionValidMessage},
402+
&testutilseviction.IsExecutedEviction{IsExecuted: true, Msg: fmt.Sprintf(condition.EvictionAllowedPDBSpecifiedMessageFmt, 2, 2)})
403403
Eventually(evictionStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed())
404404
})
405405

@@ -423,66 +423,6 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", func() {
423423
})
424424
})
425425

426-
func evictionStatusUpdatedActual(isValid *isValidEviction, isExecuted *isExecutedEviction) func() error {
427-
evictionName := fmt.Sprintf(evictionNameTemplate, GinkgoParallelProcess())
428-
return func() error {
429-
var eviction placementv1alpha1.ClusterResourcePlacementEviction
430-
if err := k8sClient.Get(ctx, types.NamespacedName{Name: evictionName}, &eviction); err != nil {
431-
return err
432-
}
433-
var conditions []metav1.Condition
434-
if isValid != nil {
435-
if isValid.bool {
436-
validCondition := metav1.Condition{
437-
Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid),
438-
Status: metav1.ConditionTrue,
439-
ObservedGeneration: eviction.GetGeneration(),
440-
Reason: clusterResourcePlacementEvictionValidReason,
441-
Message: isValid.msg,
442-
}
443-
conditions = append(conditions, validCondition)
444-
} else {
445-
invalidCondition := metav1.Condition{
446-
Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid),
447-
Status: metav1.ConditionFalse,
448-
ObservedGeneration: eviction.GetGeneration(),
449-
Reason: clusterResourcePlacementEvictionInvalidReason,
450-
Message: isValid.msg,
451-
}
452-
conditions = append(conditions, invalidCondition)
453-
}
454-
}
455-
if isExecuted != nil {
456-
if isExecuted.bool {
457-
executedCondition := metav1.Condition{
458-
Type: string(placementv1alpha1.PlacementEvictionConditionTypeExecuted),
459-
Status: metav1.ConditionTrue,
460-
ObservedGeneration: eviction.GetGeneration(),
461-
Reason: clusterResourcePlacementEvictionExecutedReason,
462-
Message: isExecuted.msg,
463-
}
464-
conditions = append(conditions, executedCondition)
465-
} else {
466-
notExecutedCondition := metav1.Condition{
467-
Type: string(placementv1alpha1.PlacementEvictionConditionTypeExecuted),
468-
Status: metav1.ConditionFalse,
469-
ObservedGeneration: eviction.GetGeneration(),
470-
Reason: clusterResourcePlacementEvictionNotExecutedReason,
471-
Message: isExecuted.msg,
472-
}
473-
conditions = append(conditions, notExecutedCondition)
474-
}
475-
}
476-
wantStatus := placementv1alpha1.PlacementEvictionStatus{
477-
Conditions: conditions,
478-
}
479-
if diff := cmp.Diff(eviction.Status, wantStatus, evictionStatusCmpOptions...); diff != "" {
480-
return fmt.Errorf("CRP status diff (-got, +want): %s", diff)
481-
}
482-
return nil
483-
}
484-
}
485-
486426
func buildTestPickNCRP(crpName string, clusterCount int32) placementv1beta1.ClusterResourcePlacement {
487427
return placementv1beta1.ClusterResourcePlacement{
488428
ObjectMeta: metav1.ObjectMeta{
@@ -597,13 +537,3 @@ func ensureAllBindingsAreRemoved(crpName string) {
597537
ensureCRBRemoved(bindingList.Items[i].Name)
598538
}
599539
}
600-
601-
type isValidEviction struct {
602-
bool
603-
msg string
604-
}
605-
606-
type isExecutedEviction struct {
607-
bool
608-
msg string
609-
}

0 commit comments

Comments
 (0)