Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 55 additions & 13 deletions tools/fleet/cmd/draincluster/draincluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,19 +186,19 @@ func (o *drainOptions) drain(ctx context.Context) (bool, error) {
return false, fmt.Errorf("failed to wait for eviction %s for CRP %s targeting member cluster %s to reach terminal state: %w", evictionName, crpName, o.clusterName, err)
}

// TODO: add safeguards to check if eviction conditions are set to unknown.
validCondition := eviction.GetCondition(string(placementv1beta1.PlacementEvictionConditionTypeValid))
if validCondition != nil && validCondition.Status == metav1.ConditionFalse {
// check to see if CRP is missing or CRP is being deleted or CRB is missing.
if validCondition.Reason == condition.EvictionInvalidMissingCRPMessage ||
validCondition.Reason == condition.EvictionInvalidDeletingCRPMessage ||
validCondition.Reason == condition.EvictionInvalidMissingCRBMessage {
log.Printf("eviction %s is invalid with reason %s for CRP %s targeting member cluster %s, but drain will succeed", evictionName, validCondition.Reason, crpName, o.clusterName)
continue
}
}
executedCondition := eviction.GetCondition(string(placementv1beta1.PlacementEvictionConditionTypeExecuted))
if executedCondition == nil || executedCondition.Status == metav1.ConditionFalse {
// Classify the terminal eviction. A missing or Unknown Valid/Executed
// condition is never treated as success, so an uncertain eviction is not
// reported as a completed drain (see evaluateEviction).
switch evaluateEviction(&eviction) {
case evictionResultInvalidButDrained:
Comment thread
britaniar marked this conversation as resolved.
validCondition := eviction.GetCondition(string(placementv1beta1.PlacementEvictionConditionTypeValid))
log.Printf("eviction %s is invalid with reason %s for CRP %s targeting member cluster %s, but drain will succeed", evictionName, validCondition.Reason, crpName, o.clusterName)
continue
Comment thread
britaniar marked this conversation as resolved.
case evictionResultIndeterminate:
isDrainSuccessful = false
log.Printf("eviction %s has a missing or unknown Valid or Executed condition for CRP %s targeting member cluster %s; cannot confirm drain", evictionName, crpName, o.clusterName)
continue
case evictionResultNotExecuted:
isDrainSuccessful = false
log.Printf("eviction %s was not executed successfully for CRP %s targeting member cluster %s", evictionName, crpName, o.clusterName)
continue
Expand All @@ -218,6 +218,48 @@ func (o *drainOptions) drain(ctx context.Context) (bool, error) {
return isDrainSuccessful, nil
}

// evictionResult classifies the terminal state of a ClusterResourcePlacementEviction
// based on its Valid and Executed conditions.
type evictionResult int

const (
// evictionResultIndeterminate means the eviction's Valid or Executed condition
// is missing or Unknown, so the drain cannot be confirmed.
evictionResultIndeterminate evictionResult = iota
// evictionResultInvalidButDrained means the eviction is invalid for a reason
// that still leaves the cluster drained (missing/deleting CRP or missing CRB).
evictionResultInvalidButDrained
// evictionResultNotExecuted means the eviction did not execute.
evictionResultNotExecuted
// evictionResultExecuted means the eviction executed successfully.
evictionResultExecuted
)

// evaluateEviction inspects a terminal eviction's Valid and Executed conditions and
// classifies the outcome. A missing or Unknown condition is never treated as success.
func evaluateEviction(eviction *placementv1beta1.ClusterResourcePlacementEviction) evictionResult {
validCondition := eviction.GetCondition(string(placementv1beta1.PlacementEvictionConditionTypeValid))
if validCondition == nil || validCondition.Status == metav1.ConditionUnknown {
return evictionResultIndeterminate
}
// An invalid eviction whose reason is a missing/deleting CRP or a missing CRB
// still means the cluster is drained.
if validCondition.Status == metav1.ConditionFalse &&
(validCondition.Reason == condition.EvictionInvalidMissingCRPMessage ||
validCondition.Reason == condition.EvictionInvalidDeletingCRPMessage ||
validCondition.Reason == condition.EvictionInvalidMissingCRBMessage) {
return evictionResultInvalidButDrained
}
executedCondition := eviction.GetCondition(string(placementv1beta1.PlacementEvictionConditionTypeExecuted))
if executedCondition == nil || executedCondition.Status == metav1.ConditionUnknown {
return evictionResultIndeterminate
}
if executedCondition.Status == metav1.ConditionFalse {
return evictionResultNotExecuted
}
return evictionResultExecuted
}

func (o *drainOptions) cordon(ctx context.Context) error {
// add taint to member cluster to ensure resources aren't scheduled on it.
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
Expand Down
103 changes: 103 additions & 0 deletions tools/fleet/cmd/draincluster/draincluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (

clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1"
placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
"github.com/kubefleet-dev/kubefleet/pkg/utils/condition"
toolsutils "github.com/kubefleet-dev/kubefleet/tools/utils"
)

Expand Down Expand Up @@ -510,3 +511,105 @@ func serviceScheme(t *testing.T) *runtime.Scheme {
}
return scheme
}

func TestEvaluateEviction(t *testing.T) {
validType := string(placementv1beta1.PlacementEvictionConditionTypeValid)
executedType := string(placementv1beta1.PlacementEvictionConditionTypeExecuted)

tests := []struct {
name string
conditions []metav1.Condition
want evictionResult
}{
{
name: "valid true, executed true",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionTrue, Reason: "Valid"},
{Type: executedType, Status: metav1.ConditionTrue, Reason: "Executed"},
},
want: evictionResultExecuted,
},
{
name: "valid true, executed false",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionTrue, Reason: "Valid"},
{Type: executedType, Status: metav1.ConditionFalse, Reason: "NotExecuted"},
},
want: evictionResultNotExecuted,
},
{
name: "valid true, executed unknown",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionTrue, Reason: "Valid"},
{Type: executedType, Status: metav1.ConditionUnknown, Reason: "Unknown"},
},
want: evictionResultIndeterminate,
},
{
name: "valid true, executed missing",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionTrue, Reason: "Valid"},
},
want: evictionResultIndeterminate,
},
{
name: "valid unknown",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionUnknown, Reason: "Unknown"},
},
want: evictionResultIndeterminate,
},
{
name: "valid missing",
conditions: nil,
want: evictionResultIndeterminate,
},
{
name: "valid false, reason missing CRP",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionFalse, Reason: condition.EvictionInvalidMissingCRPMessage},
},
want: evictionResultInvalidButDrained,
},
{
name: "valid false, reason deleting CRP",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionFalse, Reason: condition.EvictionInvalidDeletingCRPMessage},
},
want: evictionResultInvalidButDrained,
},
{
name: "valid false, reason missing CRB",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionFalse, Reason: condition.EvictionInvalidMissingCRBMessage},
},
want: evictionResultInvalidButDrained,
},
Comment thread
britaniar marked this conversation as resolved.
{
name: "valid false with unrecognized reason, executed true",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionFalse, Reason: "SomeOtherReason"},
{Type: executedType, Status: metav1.ConditionTrue, Reason: "Executed"},
},
want: evictionResultExecuted,
},
{
name: "valid false with unrecognized reason, executed unknown",
conditions: []metav1.Condition{
{Type: validType, Status: metav1.ConditionFalse, Reason: "SomeOtherReason"},
{Type: executedType, Status: metav1.ConditionUnknown, Reason: "Unknown"},
},
want: evictionResultIndeterminate,
},
}
Comment thread
britaniar marked this conversation as resolved.

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
eviction := &placementv1beta1.ClusterResourcePlacementEviction{}
eviction.SetConditions(tc.conditions...)
if got := evaluateEviction(eviction); got != tc.want {
t.Errorf("evaluateEviction() = %v, want %v", got, tc.want)
}
})
}
}
Loading