Merge pull request #157 from 3scale-ops/fix/missing-msg-in-condition

Ensure message property is always set in RollbackFailed condition
3scale-ops · Nov 4, 2022 · c6a6fe7 · c6a6fe7
2 parents c2791c3 + c3299f1
commit c6a6fe7
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 17 deletions.
diff --git a/pkg/reconcilers/marin3r/envoyconfig/status.go b/pkg/reconcilers/marin3r/envoyconfig/status.go
@@ -64,9 +64,10 @@ func IsStatusReconciled(ec *marin3rv1alpha1.EnvoyConfig, cacheState, publishedVe
 	// Reconcile the RollbackFailedCondition
 	if cacheState != marin3rv1alpha1.RollbackFailedState && ec.Status.Conditions.IsTrueFor(marin3rv1alpha1.RollbackFailedCondition) {
 		ec.Status.Conditions.SetCondition(status.Condition{
-			Type:   marin3rv1alpha1.RollbackFailedCondition,
-			Reason: "Recovered",
-			Status: corev1.ConditionFalse,
+			Type:    marin3rv1alpha1.RollbackFailedCondition,
+			Reason:  "Recovered",
+			Status:  corev1.ConditionFalse,
+			Message: "Recovered from RollbackFailed condition",
 		})
 		ok = false
 
@@ -80,6 +81,14 @@ func IsStatusReconciled(ec *marin3rv1alpha1.EnvoyConfig, cacheState, publishedVe
 		ok = false
 	}
 
+	// Temporary fix for RollbackFailedCondition conditions that are missing  the .Message property, which
+	// will be required in an upcoming release
+	if cond := ec.Status.Conditions.GetCondition(marin3rv1alpha1.RollbackFailedCondition); cond != nil && cond.Message == "" {
+		cond.Message = "Recovered from RollbackFailed condition"
+		ec.Status.Conditions.SetCondition(*cond)
+		ok = false
+	}
+
 	return ok
 }
 

diff --git a/pkg/reconcilers/marin3r/envoyconfig/status_test.go b/pkg/reconcilers/marin3r/envoyconfig/status_test.go
@@ -36,8 +36,8 @@ func TestIsStatusReconciled(t *testing.T) {
 							{Version: "2", Ref: corev1.ObjectReference{Name: "ecr2", Namespace: "test"}},
 						},
 						Conditions: status.Conditions{
-							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse},
-							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse},
+							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse, Message: "a"},
+							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse, Message: "a"},
 						},
 					},
 				},
@@ -68,8 +68,8 @@ func TestIsStatusReconciled(t *testing.T) {
 						CacheState:       pointer.StringPtr(marin3rv1alpha1.InSyncState),
 						ConfigRevisions:  []marin3rv1alpha1.ConfigRevisionRef{},
 						Conditions: status.Conditions{
-							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse},
-							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionTrue},
+							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse, Message: "a"},
+							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionTrue, Message: "a"},
 						},
 					},
 				},
@@ -89,8 +89,8 @@ func TestIsStatusReconciled(t *testing.T) {
 						CacheState:       pointer.StringPtr(marin3rv1alpha1.InSyncState),
 						ConfigRevisions:  []marin3rv1alpha1.ConfigRevisionRef{},
 						Conditions: status.Conditions{
-							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionTrue},
-							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse},
+							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionTrue, Message: "a"},
+							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse, Message: "a"},
 						},
 					},
 				},
@@ -110,8 +110,8 @@ func TestIsStatusReconciled(t *testing.T) {
 						CacheState:       pointer.StringPtr(marin3rv1alpha1.RollbackFailedState),
 						ConfigRevisions:  []marin3rv1alpha1.ConfigRevisionRef{},
 						Conditions: status.Conditions{
-							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse},
-							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse},
+							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse, Message: "a"},
+							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse, Message: "a"},
 						},
 					},
 				},
@@ -131,8 +131,8 @@ func TestIsStatusReconciled(t *testing.T) {
 						CacheState:       pointer.StringPtr(marin3rv1alpha1.InSyncState),
 						ConfigRevisions:  []marin3rv1alpha1.ConfigRevisionRef{},
 						Conditions: status.Conditions{
-							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse},
-							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse},
+							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse, Message: "a"},
+							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse, Message: "a"},
 						},
 					},
 				},
@@ -152,8 +152,8 @@ func TestIsStatusReconciled(t *testing.T) {
 						CacheState:       pointer.StringPtr(marin3rv1alpha1.InSyncState),
 						ConfigRevisions:  []marin3rv1alpha1.ConfigRevisionRef{},
 						Conditions: status.Conditions{
-							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse},
-							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse},
+							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse, Message: "a"},
+							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse, Message: "a"},
 						},
 					},
 				},
@@ -173,8 +173,8 @@ func TestIsStatusReconciled(t *testing.T) {
 						CacheState:       pointer.StringPtr(marin3rv1alpha1.InSyncState),
 						ConfigRevisions:  []marin3rv1alpha1.ConfigRevisionRef{},
 						Conditions: status.Conditions{
-							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse},
-							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse},
+							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse, Message: "a"},
+							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse, Message: "a"},
 						},
 					},
 				},
@@ -196,6 +196,53 @@ func TestIsStatusReconciled(t *testing.T) {
 			},
 			want: false,
 		},
+		{
+			name: "RollbackFailed/Recovered condition is missing the message property",
+			args: args{
+				ec: &marin3rv1alpha1.EnvoyConfig{
+					Status: marin3rv1alpha1.EnvoyConfigStatus{
+						DesiredVersion:   pointer.StringPtr("6ddbcdf795"),
+						PublishedVersion: pointer.StringPtr("6ddbcdf795"),
+						CacheState:       pointer.StringPtr(marin3rv1alpha1.InSyncState),
+						ConfigRevisions: []marin3rv1alpha1.ConfigRevisionRef{
+							{Version: "1", Ref: corev1.ObjectReference{Name: "ecr1", Namespace: "test"}},
+							{Version: "2", Ref: corev1.ObjectReference{Name: "ecr2", Namespace: "test"}},
+						},
+						Conditions: status.Conditions{
+							{Type: marin3rv1alpha1.CacheOutOfSyncCondition, Status: corev1.ConditionFalse, Message: "a"},
+							{Type: marin3rv1alpha1.RollbackFailedCondition, Status: corev1.ConditionFalse},
+						},
+					},
+				},
+				cacheState:       marin3rv1alpha1.InSyncState,
+				publishedVersion: "6ddbcdf795",
+				list: &marin3rv1alpha1.EnvoyConfigRevisionList{
+					Items: []marin3rv1alpha1.EnvoyConfigRevision{
+						{
+							ObjectMeta: metav1.ObjectMeta{Name: "ecr1", Namespace: "test"},
+							Spec:       marin3rv1alpha1.EnvoyConfigRevisionSpec{Version: "1"},
+						},
+						{
+							ObjectMeta: metav1.ObjectMeta{Name: "ecr2", Namespace: "test"},
+							Spec:       marin3rv1alpha1.EnvoyConfigRevisionSpec{Version: "2"},
+						},
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "Status empty, return false",
+			args: args{
+				ec: &marin3rv1alpha1.EnvoyConfig{
+					Status: marin3rv1alpha1.EnvoyConfigStatus{},
+				},
+				cacheState:       marin3rv1alpha1.RollbackState,
+				publishedVersion: "xxxx",
+				list:             &marin3rv1alpha1.EnvoyConfigRevisionList{},
+			},
+			want: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {