Skip to content

Commit

Permalink
changes to rollout controller for eviction
Browse files Browse the repository at this point in the history
  • Loading branch information
@Arvindthiru
Arvindthiru committed Dec 18, 2024
1 parent 96fd96a commit 21b20a8
Show file tree
Hide file tree
Showing 13 changed files with 1,499 additions and 429 deletions.
9 changes: 9 additions & 0 deletions cmd/hubagent/workload/setup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"go.goms.io/fleet/pkg/controllers/clusterinventory/clusterprofile"
"go.goms.io/fleet/pkg/controllers/clusterresourcebindingwatcher"
"go.goms.io/fleet/pkg/controllers/clusterresourceplacement"
"go.goms.io/fleet/pkg/controllers/clusterresourceplacementeviction"
"go.goms.io/fleet/pkg/controllers/clusterresourceplacementwatcher"
"go.goms.io/fleet/pkg/controllers/clusterschedulingpolicysnapshot"
"go.goms.io/fleet/pkg/controllers/memberclusterplacement"
Expand Down Expand Up @@ -206,6 +207,14 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
return err
}

klog.Info("Setting up cluster resource placement eviction controller")
if err := (&clusterresourceplacementeviction.Reconciler{
Client: mgr.GetClient(),
}).SetupWithManager(mgr); err != nil {
klog.ErrorS(err, "Unable to set up cluster resource placement eviction controller")
return err
}

// Set up the work generator
klog.Info("Setting up work generator")
if err := (&workgenerator.Reconciler{
Expand Down
59 changes: 19 additions & 40 deletions pkg/controllers/clusterresourceplacementeviction/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,6 @@ import (
"go.goms.io/fleet/pkg/utils/defaulter"
)

const (
clusterResourcePlacementEvictionValidReason = "ClusterResourcePlacementEvictionValid"
clusterResourcePlacementEvictionInvalidReason = "ClusterResourcePlacementEvictionInvalid"
clusterResourcePlacementEvictionExecutedReason = "ClusterResourcePlacementEvictionExecuted"
clusterResourcePlacementEvictionNotExecutedReason = "ClusterResourcePlacementEvictionNotExecuted"

evictionInvalidMissingCRPMessage = "Failed to find ClusterResourcePlacement targeted by eviction"
evictionInvalidDeletingCRPMessage = "Found deleting ClusterResourcePlacement targeted by eviction"
evictionInvalidMissingCRBMessage = "Failed to find scheduler decision for placement in cluster targeted by eviction"
evictionInvalidMultipleCRBMessage = "Found more than one scheduler decision for placement in cluster targeted by eviction"
evictionValidMessage = "Eviction is valid"
evictionAllowedNoPDBMessage = "Eviction is allowed, no ClusterResourcePlacementDisruptionBudget specified"
evictionAllowedPlacementRemovedMessage = "Eviction is allowed, resources propagated by placement is currently being removed from cluster targeted by eviction"
evictionAllowedPlacementFailedMessage = "Eviction is allowed, placement has failed"
evictionBlockedMisconfiguredPDBSpecifiedMessage = "Eviction is blocked by misconfigured ClusterResourcePlacementDisruptionBudget, either MaxUnavailable is specified or MinAvailable is specified as a percentage for PickAll ClusterResourcePlacement"
evictionBlockedMissingPlacementMessage = "Eviction is blocked, placement has not propagated resources to target cluster yet"

evictionAllowedPDBSpecifiedFmt = "Eviction is allowed by specified ClusterResourcePlacementDisruptionBudget, availablePlacements: %d, totalPlacements: %d"
evictionBlockedPDBSpecifiedFmt = "Eviction is blocked by specified ClusterResourcePlacementDisruptionBudget, availablePlacements: %d, totalPlacements: %d"
)

// Reconciler reconciles a ClusterResourcePlacementEviction object.
type Reconciler struct {
client.Client
Expand Down Expand Up @@ -97,8 +76,8 @@ func (r *Reconciler) validateEviction(ctx context.Context, eviction *placementv1
var crp placementv1beta1.ClusterResourcePlacement
if err := r.Client.Get(ctx, types.NamespacedName{Name: eviction.Spec.PlacementName}, &crp); err != nil {
if k8serrors.IsNotFound(err) {
klog.V(2).InfoS(evictionInvalidMissingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
markEvictionInvalid(eviction, evictionInvalidMissingCRPMessage)
klog.V(2).InfoS(condition.EvictionInvalidMissingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
markEvictionInvalid(eviction, condition.EvictionInvalidMissingCRPMessage)
return validationResult, nil
}
return nil, controller.NewAPIServerError(true, err)
Expand All @@ -108,8 +87,8 @@ func (r *Reconciler) validateEviction(ctx context.Context, eviction *placementv1
defaulter.SetDefaultsClusterResourcePlacement(&crp)

if crp.DeletionTimestamp != nil {
klog.V(2).InfoS(evictionInvalidDeletingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
markEvictionInvalid(eviction, evictionInvalidDeletingCRPMessage)
klog.V(2).InfoS(condition.EvictionInvalidDeletingCRPMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
markEvictionInvalid(eviction, condition.EvictionInvalidDeletingCRPMessage)
return validationResult, nil
}
validationResult.crp = &crp
Expand All @@ -126,15 +105,15 @@ func (r *Reconciler) validateEviction(ctx context.Context, eviction *placementv1
if evictionTargetBinding == nil {
evictionTargetBinding = &crbList.Items[i]
} else {
klog.V(2).InfoS(evictionInvalidMultipleCRBMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
markEvictionInvalid(eviction, evictionInvalidMultipleCRBMessage)
klog.V(2).InfoS(condition.EvictionInvalidMultipleCRBMessage, "clusterResourcePlacementEviction", eviction.Name, "clusterResourcePlacement", eviction.Spec.PlacementName)
markEvictionInvalid(eviction, condition.EvictionInvalidMultipleCRBMessage)
return validationResult, nil
}
}
}
if evictionTargetBinding == nil {
klog.V(2).InfoS("Failed to find cluster resource binding for cluster targeted by eviction", "clusterResourcePlacementEviction", eviction.Name, "targetCluster", eviction.Spec.ClusterName)
markEvictionInvalid(eviction, evictionInvalidMissingCRBMessage)
markEvictionInvalid(eviction, condition.EvictionInvalidMissingCRBMessage)
return validationResult, nil
}
validationResult.crb = evictionTargetBinding
Expand Down Expand Up @@ -179,14 +158,14 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
if evictionTargetBinding.GetDeletionTimestamp() != nil {
klog.V(2).InfoS("ClusterResourceBinding targeted by eviction is being deleted",
"clusterResourcePlacementEviction", eviction.Name, "clusterResourceBinding", evictionTargetBinding.Name, "targetCluster", eviction.Spec.ClusterName)
markEvictionExecuted(eviction, evictionAllowedPlacementRemovedMessage)
markEvictionExecuted(eviction, condition.EvictionAllowedPlacementRemovedMessage)
return nil
}

if !isPlacementPresent(evictionTargetBinding) {
klog.V(2).InfoS("No resources have been placed for ClusterResourceBinding in target cluster",
"clusterResourcePlacementEviction", eviction.Name, "clusterResourceBinding", evictionTargetBinding.Name, "targetCluster", eviction.Spec.ClusterName)
markEvictionNotExecuted(eviction, evictionBlockedMissingPlacementMessage)
markEvictionNotExecuted(eviction, condition.EvictionBlockedMissingPlacementMessage)
return nil
}

Expand All @@ -197,7 +176,7 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
if err := r.deleteClusterResourceBinding(ctx, evictionTargetBinding); err != nil {
return err
}
markEvictionExecuted(eviction, evictionAllowedPlacementFailedMessage)
markEvictionExecuted(eviction, condition.EvictionAllowedPlacementFailedMessage)
return nil
}

Expand All @@ -207,7 +186,7 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
if err = r.deleteClusterResourceBinding(ctx, evictionTargetBinding); err != nil {
return err
}
markEvictionExecuted(eviction, evictionAllowedNoPDBMessage)
markEvictionExecuted(eviction, condition.EvictionAllowedNoPDBMessage)
return nil
}
return controller.NewAPIServerError(true, err)
Expand All @@ -216,7 +195,7 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
// handle special case for PickAll CRP.
if crp.Spec.Policy.PlacementType == placementv1beta1.PickAllPlacementType {
if db.Spec.MaxUnavailable != nil || (db.Spec.MinAvailable != nil && db.Spec.MinAvailable.Type == intstr.String) {
markEvictionNotExecuted(eviction, evictionBlockedMisconfiguredPDBSpecifiedMessage)
markEvictionNotExecuted(eviction, condition.EvictionBlockedMisconfiguredPDBSpecifiedMessage)
return nil
}
}
Expand All @@ -227,9 +206,9 @@ func (r *Reconciler) executeEviction(ctx context.Context, validationResult *evic
if err := r.deleteClusterResourceBinding(ctx, evictionTargetBinding); err != nil {
return err
}
markEvictionExecuted(eviction, fmt.Sprintf(evictionAllowedPDBSpecifiedFmt, availableBindings, totalBindings))
markEvictionExecuted(eviction, fmt.Sprintf(condition.EvictionAllowedPDBSpecifiedMessageFmt, availableBindings, totalBindings))
} else {
markEvictionNotExecuted(eviction, fmt.Sprintf(evictionBlockedPDBSpecifiedFmt, availableBindings, totalBindings))
markEvictionNotExecuted(eviction, fmt.Sprintf(condition.EvictionBlockedPDBSpecifiedMessageFmt, availableBindings, totalBindings))
}
return nil
}
Expand Down Expand Up @@ -315,8 +294,8 @@ func markEvictionValid(eviction *placementv1alpha1.ClusterResourcePlacementEvict
Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid),
Status: metav1.ConditionTrue,
ObservedGeneration: eviction.Generation,
Reason: clusterResourcePlacementEvictionValidReason,
Message: evictionValidMessage,
Reason: condition.ClusterResourcePlacementEvictionValidReason,
Message: condition.EvictionValidMessage,
}
eviction.SetConditions(cond)

Expand All @@ -329,7 +308,7 @@ func markEvictionInvalid(eviction *placementv1alpha1.ClusterResourcePlacementEvi
Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid),
Status: metav1.ConditionFalse,
ObservedGeneration: eviction.Generation,
Reason: clusterResourcePlacementEvictionInvalidReason,
Reason: condition.ClusterResourcePlacementEvictionInvalidReason,
Message: message,
}
eviction.SetConditions(cond)
Expand All @@ -342,7 +321,7 @@ func markEvictionExecuted(eviction *placementv1alpha1.ClusterResourcePlacementEv
Type: string(placementv1alpha1.PlacementEvictionConditionTypeExecuted),
Status: metav1.ConditionTrue,
ObservedGeneration: eviction.Generation,
Reason: clusterResourcePlacementEvictionExecutedReason,
Reason: condition.ClusterResourcePlacementEvictionExecutedReason,
Message: message,
}
eviction.SetConditions(cond)
Expand All @@ -355,7 +334,7 @@ func markEvictionNotExecuted(eviction *placementv1alpha1.ClusterResourcePlacemen
Type: string(placementv1alpha1.PlacementEvictionConditionTypeExecuted),
Status: metav1.ConditionFalse,
ObservedGeneration: eviction.Generation,
Reason: clusterResourcePlacementEvictionNotExecutedReason,
Reason: condition.ClusterResourcePlacementEvictionNotExecutedReason,
Message: message,
}
eviction.SetConditions(cond)
Expand Down
106 changes: 18 additions & 88 deletions pkg/controllers/clusterresourceplacementeviction/controller_intergration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import (
"fmt"
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -23,6 +21,8 @@ import (

placementv1alpha1 "go.goms.io/fleet/apis/placement/v1alpha1"
placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
"go.goms.io/fleet/pkg/utils/condition"
testutilseviction "go.goms.io/fleet/test/utils/eviction"
)

const (
Expand All @@ -32,18 +32,6 @@ const (
evictionNameTemplate = "eviction-%d"
)

var (
lessFuncCondition = func(a, b metav1.Condition) bool {
return a.Type < b.Type
}

evictionStatusCmpOptions = cmp.Options{
cmpopts.SortSlices(lessFuncCondition),
cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
cmpopts.EquateEmpty(),
}
)

const (
eventuallyDuration = time.Minute * 2
eventuallyInterval = time.Millisecond * 250
Expand Down Expand Up @@ -121,7 +109,10 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", func() {
})

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

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

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

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

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

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

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

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

func evictionStatusUpdatedActual(isValid *isValidEviction, isExecuted *isExecutedEviction) func() error {
evictionName := fmt.Sprintf(evictionNameTemplate, GinkgoParallelProcess())
return func() error {
var eviction placementv1alpha1.ClusterResourcePlacementEviction
if err := k8sClient.Get(ctx, types.NamespacedName{Name: evictionName}, &eviction); err != nil {
return err
}
var conditions []metav1.Condition
if isValid != nil {
if isValid.bool {
validCondition := metav1.Condition{
Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid),
Status: metav1.ConditionTrue,
ObservedGeneration: eviction.GetGeneration(),
Reason: clusterResourcePlacementEvictionValidReason,
Message: isValid.msg,
}
conditions = append(conditions, validCondition)
} else {
invalidCondition := metav1.Condition{
Type: string(placementv1alpha1.PlacementEvictionConditionTypeValid),
Status: metav1.ConditionFalse,
ObservedGeneration: eviction.GetGeneration(),
Reason: clusterResourcePlacementEvictionInvalidReason,
Message: isValid.msg,
}
conditions = append(conditions, invalidCondition)
}
}
if isExecuted != nil {
if isExecuted.bool {
executedCondition := metav1.Condition{
Type: string(placementv1alpha1.PlacementEvictionConditionTypeExecuted),
Status: metav1.ConditionTrue,
ObservedGeneration: eviction.GetGeneration(),
Reason: clusterResourcePlacementEvictionExecutedReason,
Message: isExecuted.msg,
}
conditions = append(conditions, executedCondition)
} else {
notExecutedCondition := metav1.Condition{
Type: string(placementv1alpha1.PlacementEvictionConditionTypeExecuted),
Status: metav1.ConditionFalse,
ObservedGeneration: eviction.GetGeneration(),
Reason: clusterResourcePlacementEvictionNotExecutedReason,
Message: isExecuted.msg,
}
conditions = append(conditions, notExecutedCondition)
}
}
wantStatus := placementv1alpha1.PlacementEvictionStatus{
Conditions: conditions,
}
if diff := cmp.Diff(eviction.Status, wantStatus, evictionStatusCmpOptions...); diff != "" {
return fmt.Errorf("CRP status diff (-got, +want): %s", diff)
}
return nil
}
}

func buildTestPickNCRP(crpName string, clusterCount int32) placementv1beta1.ClusterResourcePlacement {
return placementv1beta1.ClusterResourcePlacement{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -597,13 +537,3 @@ func ensureAllBindingsAreRemoved(crpName string) {
ensureCRBRemoved(bindingList.Items[i].Name)
}
}

type isValidEviction struct {
bool
msg string
}

type isExecutedEviction struct {
bool
msg string
}
Loading

0 comments on commit 21b20a8

Please sign in to comment.