Enable existing test that had a bug which caused incorrect assert error

k8ssandra · Mar 6, 2024 · c1668e7 · c1668e7
1 parent ea54b3e
commit c1668e7
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 47 deletions.
diff --git a/pkg/reconciliation/reconcile_racks.go b/pkg/reconciliation/reconcile_racks.go
@@ -222,6 +222,7 @@ func (rc *ReconciliationContext) CheckRackPodTemplate() result.ReconcileResult {
 				} else {
 					partition = int32(rc.desiredRackInformation[idx].NodeCount) - dc.Spec.CanaryUpgradeCount
 				}
+
 				strategy := appsv1.StatefulSetUpdateStrategy{
 					Type: appsv1.RollingUpdateStatefulSetStrategyType,
 					RollingUpdate: &appsv1.RollingUpdateStatefulSetStrategy{
@@ -262,8 +263,7 @@ func (rc *ReconciliationContext) CheckRackPodTemplate() result.ReconcileResult {
 			)
 
 			statefulSet.SetResourceVersion(resVersion)
-			err = rc.Client.Update(rc.Ctx, statefulSet)
-			if err != nil {
+			if err := rc.Client.Update(rc.Ctx, statefulSet); err != nil {
 				if errors.IsInvalid(err) {
 					if err = rc.deleteStatefulSet(statefulSet); err != nil {
 						return result.Error(err)

diff --git a/pkg/reconciliation/reconcile_racks_test.go b/pkg/reconciliation/reconcile_racks_test.go
@@ -288,67 +288,66 @@ func TestCheckRackPodTemplate_SetControllerRefOnStatefulSet(t *testing.T) {
 	assert.Equal(t, rc.statefulSets[0].Name, actualObject.GetName())
 }
 
-// Disabled due to a bug in the controller-runtime: https://github.com/kubernetes-sigs/controller-runtime/issues/1832
-// func TestCheckRackPodTemplate_CanaryUpgrade(t *testing.T) {
-// 	rc, _, cleanpMockSrc := setupTest()
-// 	defer cleanpMockSrc()
+func TestCheckRackPodTemplate_CanaryUpgrade(t *testing.T) {
+	rc, _, cleanpMockSrc := setupTest()
+	defer cleanpMockSrc()
 
-// 	rc.Datacenter.Spec.ServerVersion = "6.8.2"
-// 	rc.Datacenter.Spec.Racks = []api.Rack{
-// 		{Name: "rack1", Zone: "zone-1"},
-// 	}
+	rc.Datacenter.Spec.ServerVersion = "6.8.2"
+	rc.Datacenter.Spec.Racks = []api.Rack{
+		{Name: "rack1", DeprecatedZone: "zone-1"},
+	}
 
-// 	if err := rc.CalculateRackInformation(); err != nil {
-// 		t.Fatalf("failed to calculate rack information: %s", err)
-// 	}
+	if err := rc.CalculateRackInformation(); err != nil {
+		t.Fatalf("failed to calculate rack information: %s", err)
+	}
 
-// 	result := rc.CheckRackCreation()
-// 	assert.False(t, result.Completed(), "CheckRackCreation did not complete as expected")
+	result := rc.CheckRackCreation()
+	assert.False(t, result.Completed(), "CheckRackCreation did not complete as expected")
 
-// 	if err := rc.Client.Update(rc.Ctx, rc.Datacenter); err != nil {
-// 		t.Fatalf("failed to add rack to cassandradatacenter: %s", err)
-// 	}
+	if err := rc.Client.Update(rc.Ctx, rc.Datacenter); err != nil {
+		t.Fatalf("failed to add rack to cassandradatacenter: %s", err)
+	}
 
-// 	result = rc.CheckRackPodTemplate()
-// 	_, err := result.Output()
+	result = rc.CheckRackPodTemplate()
+	_, err := result.Output()
 
-// 	assert.True(t, result.Completed())
-// 	assert.Nil(t, err)
-// 	previousLabels := rc.statefulSets[0].Spec.Template.Labels
+	assert.True(t, result.Completed())
+	assert.Nil(t, err)
+	previousLabels := rc.statefulSets[0].Spec.Template.Labels
 
-// 	rc.Datacenter.Spec.CanaryUpgrade = true
-// 	rc.Datacenter.Spec.CanaryUpgradeCount = 1
-// 	rc.Datacenter.Spec.ServerVersion = "6.8.3"
+	rc.Datacenter.Spec.CanaryUpgrade = true
+	rc.Datacenter.Spec.CanaryUpgradeCount = 1
+	rc.Datacenter.Spec.ServerVersion = "6.8.3"
+	partition := rc.Datacenter.Spec.CanaryUpgradeCount
 
-// 	result = rc.CheckRackPodTemplate()
-// 	_, err = result.Output()
+	result = rc.CheckRackPodTemplate()
+	_, err = result.Output()
 
-// 	assert.True(t, result.Completed())
-// 	assert.Nil(t, err)
+	assert.True(t, result.Completed())
+	assert.Nil(t, err)
 
-// 	assert.Equal(t, rc.Datacenter.Status.CassandraOperatorProgress, api.ProgressUpdating)
+	assert.Equal(t, rc.Datacenter.Status.CassandraOperatorProgress, api.ProgressUpdating)
 
-// 	partition := &rc.Datacenter.Spec.CanaryUpgradeCount
-// 	expectedStrategy := appsv1.StatefulSetUpdateStrategy{
-// 		Type: appsv1.RollingUpdateStatefulSetStrategyType,
-// 		RollingUpdate: &appsv1.RollingUpdateStatefulSetStrategy{
-// 			Partition: partition,
-// 		},
-// 	}
+	expectedStrategy := appsv1.StatefulSetUpdateStrategy{
+		Type: appsv1.RollingUpdateStatefulSetStrategyType,
+		RollingUpdate: &appsv1.RollingUpdateStatefulSetStrategy{
+			Partition: &partition,
+		},
+	}
 
-// 	assert.Equal(t, expectedStrategy, rc.statefulSets[0].Spec.UpdateStrategy)
+	assert.Equal(t, expectedStrategy, rc.statefulSets[0].Spec.UpdateStrategy)
 
-// 	rc.statefulSets[0].Status.Replicas = 2
-// 	rc.statefulSets[0].Status.ReadyReplicas = 2
-// 	rc.statefulSets[0].Status.CurrentReplicas = 1
-// 	rc.statefulSets[0].Status.UpdatedReplicas = 1
+	rc.statefulSets[0].Status.Replicas = 2
+	rc.statefulSets[0].Status.ReadyReplicas = 2
+	rc.statefulSets[0].Status.CurrentReplicas = 1
+	rc.statefulSets[0].Status.UpdatedReplicas = 1
 
-// 	result = rc.CheckRackPodTemplate()
+	result = rc.CheckRackPodTemplate()
 
-// 	assert.EqualValues(t, previousLabels, rc.statefulSets[0].Spec.Template.Labels)
+	assert.EqualValues(t, previousLabels, rc.statefulSets[0].Spec.Template.Labels)
 
-// 	assert.True(t, result.Completed())
-// }
+	assert.True(t, result.Completed())
+}
 
 func TestReconcilePods(t *testing.T) {
 	t.Skip()