From 7f155b49bf0e41e56c8bde6963cb4dfbd91ace14 Mon Sep 17 00:00:00 2001 From: Disaiah Bennett Date: Tue, 6 Feb 2024 17:25:20 -0500 Subject: [PATCH] [ACM-8576] Updated MCE status response for resource applying (#585) * updated MCE status response for resource applying Signed-off-by: Disaiah Bennett * fixed conditional statement Signed-off-by: Disaiah Bennett * removed additional invalid status Signed-off-by: Disaiah Bennett * updated MCE status condition Signed-off-by: Disaiah Bennett * updated controller Signed-off-by: Disaiah Bennett * added test cases for condition status Signed-off-by: Disaiah Bennett --------- Signed-off-by: Disaiah Bennett --- api/v1/multiclusterengine_types.go | 30 +- controllers/backplaneconfig_controller.go | 18 +- pkg/status/condition.go | 29 ++ pkg/status/condition_test.go | 333 ++++++++++++++++++++++ pkg/status/status.go | 5 + 5 files changed, 405 insertions(+), 10 deletions(-) create mode 100644 pkg/status/condition_test.go diff --git a/api/v1/multiclusterengine_types.go b/api/v1/multiclusterengine_types.go index 5e9fec7e..9ccfc744 100644 --- a/api/v1/multiclusterengine_types.go +++ b/api/v1/multiclusterengine_types.go @@ -148,23 +148,35 @@ const ( MultiClusterEnginePhaseUninstalling PhaseType = "Uninstalling" MultiClusterEnginePhaseError PhaseType = "Error" MultiClusterEnginePhaseUnimplemented PhaseType = "Unimplemented" + MultiClusterEnginePhaseUpdating PhaseType = "Updating" ) type MultiClusterEngineConditionType string // These are valid conditions of the multiclusterengine. const ( - // Available means the deployment is available, ie. at least the minimum available - // replicas required are up and running for at least minReadySeconds. + /* + Available means the deployment is available, ie. at least the minimum available + replicas required are up and running for at least minReadySeconds. + */ MultiClusterEngineAvailable MultiClusterEngineConditionType = "Available" - // Progressing means the deployment is progressing. Progress for a deployment is - // considered when a new replica set is created or adopted, and when new pods scale - // up or old pods scale down. Progress is not estimated for paused deployments or - // when progressDeadlineSeconds is not specified. + /* + Progressing means the deployment is progressing. Progress for a deployment is + considered when a new replica set is created or adopted, and when new pods scale + up or old pods scale down. Progress is not estimated for paused deployments or + when progressDeadlineSeconds is not specified. + */ MultiClusterEngineProgressing MultiClusterEngineConditionType = "Progressing" - // Failure is added in a deployment when one of its pods fails to be created - // or deleted. - MultiClusterEngineFailure MultiClusterEngineConditionType = "MultiClusterEngineFailure" + /* + ComponentFailure is added in a deployment when one of its pods fails to be created + or deleted. + */ + MultiClusterEngineComponentFailure MultiClusterEngineConditionType = "ComponentFailure" + /* + Failure is added in a deployment when one of its pods fails to be created + or deleted. + */ + MultiClusterEngineFailure MultiClusterEngineConditionType = "Failure" ) type MultiClusterEngineCondition struct { diff --git a/controllers/backplaneconfig_controller.go b/controllers/backplaneconfig_controller.go index 65545008..e6695de1 100644 --- a/controllers/backplaneconfig_controller.go +++ b/controllers/backplaneconfig_controller.go @@ -174,6 +174,10 @@ func (r *MultiClusterEngineReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, nil } + // reset the status conditions for failures that has occurred in previous iterations. + backplaneConfig.Status.Conditions = status.FilterOutConditionWithSubString(backplaneConfig.Status.Conditions, + backplanev1.MultiClusterEngineComponentFailure) + // reset status manager r.StatusManager.Reset("") for _, c := range backplaneConfig.Status.Conditions { @@ -982,7 +986,19 @@ func (r *MultiClusterEngineReconciler) applyTemplate(ctx context.Context, backpl force := true err := r.Client.Patch(ctx, template, client.Apply, &client.PatchOptions{Force: &force, FieldManager: "backplane-operator"}) if err != nil { - return ctrl.Result{}, fmt.Errorf("error applying object Name: %s Kind: %s Error: %w", template.GetName(), template.GetKind(), err) + errMessage := fmt.Errorf( + "error applying object Name: %s Kind: %s Error: %w", template.GetName(), template.GetKind(), err) + + condType := fmt.Sprintf("%v: %v (Kind:%v)", backplanev1.MultiClusterEngineComponentFailure, + template.GetName(), template.GetKind()) + + r.StatusManager.AddCondition( + status.NewCondition( + backplanev1.MultiClusterEngineConditionType(condType), metav1.ConditionTrue, + status.ApplyFailedReason, errMessage.Error()), + ) + + return ctrl.Result{}, errMessage } } return ctrl.Result{}, nil diff --git a/pkg/status/condition.go b/pkg/status/condition.go index bf4c7246..5e11eb60 100644 --- a/pkg/status/condition.go +++ b/pkg/status/condition.go @@ -2,11 +2,15 @@ package status import ( + "strings" + v1 "github.com/stolostron/backplane-operator/api/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( + // ApplyFailedReason is added when the hub fails to apply a resource + ApplyFailedReason = "FailedApplyingComponent" // ComponentsAvailableReason is when all desired components are running successfully ComponentsAvailableReason = "ComponentsAvailable" // ComponentsUnavailableReason is when one or more components are in an unready state @@ -88,3 +92,28 @@ func filterOutCondition(conditions []v1.MultiClusterEngineCondition, condType v1 } return newConditions } + +// FilterOutConditionWithSubString returns a new slice of hub conditions without conditions with the provided type. +func FilterOutConditionWithSubString(conditions []v1.MultiClusterEngineCondition, condType v1.MultiClusterEngineConditionType) []v1.MultiClusterEngineCondition { + var newConditions []v1.MultiClusterEngineCondition + for _, c := range conditions { + if strings.Contains(string(c.Type), string(condType)) { + continue + } + newConditions = append(newConditions, c) + } + return newConditions +} + +/* +ConditionPresentWithSubstring returns true or false if a MultiClusterEngineCondition is +present with a target substring. +*/ +func ConditionPresentWithSubstring(conditions []v1.MultiClusterEngineCondition, substring string) bool { + for _, c := range conditions { + if strings.Contains(string(c.Type), substring) { + return true + } + } + return false +} diff --git a/pkg/status/condition_test.go b/pkg/status/condition_test.go new file mode 100644 index 00000000..82aaa93c --- /dev/null +++ b/pkg/status/condition_test.go @@ -0,0 +1,333 @@ +// Copyright Contributors to the Open Cluster Management project +package status + +import ( + "fmt" + "testing" + + bpv1 "github.com/stolostron/backplane-operator/api/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func Test_NewCondition(t *testing.T) { + tests := []struct { + name string + condType bpv1.MultiClusterEngineConditionType + status metav1.ConditionStatus + reason string + message string + want bool + }{ + { + name: "added new multiclusterengine condition", + condType: bpv1.MultiClusterEngineAvailable, + status: metav1.ConditionTrue, + reason: DeploySuccessReason, + message: "All components deployed", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cond := NewCondition(tt.condType, tt.status, tt.reason, tt.message) + + if got := cond.Type != tt.condType; got { + t.Errorf("%s: expected condition condType: %v to equal: %v, got %v", tt.name, cond.Type, tt.condType, got) + } + + if got := cond.Status != tt.status; got { + t.Errorf("%s: expected condition status: %v to equal: %v, got %v", tt.name, cond.Status, tt.status, got) + } + + if got := cond.Reason != tt.reason; got { + t.Errorf("%s: expected condition reason: %v to equal: %v, got %v", tt.name, cond.Reason, tt.reason, got) + } + + if got := cond.Message != tt.message; got { + t.Errorf("%s: expected condition message: %v to equal: %v, got %v", tt.name, cond.Message, tt.message, got) + } + }) + } +} + +func Test_setCondition(t *testing.T) { + tests := []struct { + name string + conditions []bpv1.MultiClusterEngineCondition + want int + }{ + { + name: "set single multiclusterengine condition", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Reason: DeploySuccessReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineProgressing, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + want: 1, + }, + { + name: "set multiple multiclusterengine conditions", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Reason: DeploySuccessReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineProgressing, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + { + Message: "All components deployed", + Reason: ComponentsAvailableReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineAvailable, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + want: 2, + }, + { + name: "set duplicate multiclusterengine conditions", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Reason: DeploySuccessReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineProgressing, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + { + Message: "All components deployed", + Reason: ComponentsAvailableReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineAvailable, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + want: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + newCond := bpv1.MultiClusterEngineCondition{ + Reason: DeploySuccessReason, + Status: metav1.ConditionUnknown, + Type: bpv1.MultiClusterEngineProgressing, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + } + + cond := setCondition(tt.conditions, newCond) + + if len(cond) != tt.want { + t.Errorf("%s: expected condition to exist: %d, got %v", tt.name, tt.want, len(cond)) + } + }) + } +} + +func Test_getCondition(t *testing.T) { + tests := []struct { + name string + conditions []bpv1.MultiClusterEngineCondition + condType bpv1.MultiClusterEngineConditionType + want bool + }{ + { + name: "get multiclusterengine condition", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Message: "failed to create typed patch object (mutlicluster-engine/foo; apps/v1, Kind=Deployment)", + Reason: ApplyFailedReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineComponentFailure, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + { + Message: "setting up operator", + Reason: ComponentsUnavailableReason, + Status: metav1.ConditionFalse, + Type: bpv1.MultiClusterEngineProgressing, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + condType: bpv1.MultiClusterEngineComponentFailure, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cond := getCondition(tt.conditions, tt.condType) + + exists := cond != nil + if exists != tt.want { + t.Errorf("%s: expected condition to exist: %t, got %v", tt.name, tt.want, exists) + } + }) + } +} + +func Test_filterOutCondition(t *testing.T) { + tests := []struct { + name string + conditions []bpv1.MultiClusterEngineCondition + condType bpv1.MultiClusterEngineConditionType + want int + }{ + { + name: "filter out multiclusterengine condition", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Message: "failed to create typed patch object (mutlicluster-engine/foo; apps/v1, Kind=Deployment)", + Reason: ApplyFailedReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineComponentFailure, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + { + Message: "setting up operator", + Reason: ComponentsUnavailableReason, + Status: metav1.ConditionFalse, + Type: bpv1.MultiClusterEngineProgressing, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + condType: bpv1.MultiClusterEngineComponentFailure, + want: 1, + }, + { + name: "filter out no multiclusterengine condition", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Message: "failed to create typed patch object (mutlicluster-engine/foo; apps/v1, Kind=Deployment)", + Reason: ApplyFailedReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineComponentFailure, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + { + Message: "setting up operator", + Reason: ComponentsUnavailableReason, + Status: metav1.ConditionFalse, + Type: bpv1.MultiClusterEngineProgressing, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + condType: bpv1.MultiClusterEngineAvailable, + want: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cond := filterOutCondition(tt.conditions, tt.condType) + + if len(cond) != tt.want { + t.Errorf("%s: expected condition to exist: %v, got %v", tt.name, tt.want, len(cond)) + } + }) + } +} + +func Test_FilterOutConditionWithSubString(t *testing.T) { + tests := []struct { + name string + conditions []bpv1.MultiClusterEngineCondition + condType bpv1.MultiClusterEngineConditionType + subString string + want int + }{ + { + name: "filter out multiclusterengine condition by substring", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Message: "failed to create typed patch object (mutlicluster-engine/foo; apps/v1, Kind=Deployment)", + Reason: ApplyFailedReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineConditionType(fmt.Sprintf("%v: %v (Kind: %v)", bpv1.MultiClusterEngineComponentFailure, "foo", "Deployment")), + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + condType: bpv1.MultiClusterEngineComponentFailure, + subString: string(bpv1.MultiClusterEngineComponentFailure), + want: 0, + }, + { + name: "filter out no multiclusterengine condition", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Message: "failed to create typed patch object (mutlicluster-engine/foo; apps/v1, Kind=Deployment)", + Reason: ApplyFailedReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineComponentFailure, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + { + Message: "setting up operator", + Reason: ComponentsUnavailableReason, + Status: metav1.ConditionFalse, + Type: bpv1.MultiClusterEngineProgressing, + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + condType: bpv1.MultiClusterEngineAvailable, + subString: string(bpv1.MultiClusterEngineAvailable), + want: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cond := FilterOutConditionWithSubString(tt.conditions, tt.condType) + + if len(cond) != tt.want { + t.Errorf("%s: expected condition to exist: %v, got %v", tt.name, tt.want, len(cond)) + } + }) + } +} + +func Test_ConditionPresentWithSubstring(t *testing.T) { + tests := []struct { + name string + conditions []bpv1.MultiClusterEngineCondition + subString string + want bool + }{ + { + name: "check if multiclusterengine condition contains substring", + conditions: []bpv1.MultiClusterEngineCondition{ + { + Message: "failed to create typed patch object (mutlicluster-engine/foo; apps/v1, Kind=Deployment)", + Reason: ApplyFailedReason, + Status: metav1.ConditionTrue, + Type: bpv1.MultiClusterEngineConditionType(fmt.Sprintf("%v: %v (Kind: %v)", bpv1.MultiClusterEngineComponentFailure, "foo", "Deployment")), + LastUpdateTime: metav1.Now(), + LastTransitionTime: metav1.Now(), + }, + }, + subString: string(bpv1.MultiClusterEngineComponentFailure), + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if ok := ConditionPresentWithSubstring(tt.conditions, tt.subString); !ok { + t.Errorf("%s: expected condition to exist: %v, got %v", tt.name, tt.want, ok) + } + }) + } +} diff --git a/pkg/status/status.go b/pkg/status/status.go index 52b01e19..81043f64 100644 --- a/pkg/status/status.go +++ b/pkg/status/status.go @@ -116,6 +116,11 @@ func (sm *StatusTracker) reportPhase(mce bpv1.MultiClusterEngine, components []b return bpv1.MultiClusterEnginePhaseError } + // If status contains failure status return error + if ok := ConditionPresentWithSubstring(conditions, string(bpv1.MultiClusterEngineComponentFailure)); ok { + return bpv1.MultiClusterEnginePhaseError + } + // If a component isn't ready show progressing phase if !allComponentsReady(components) { return bpv1.MultiClusterEnginePhaseProgressing