minor updates based on PR review

k8ssandra · Jan 14, 2022 · 2ad44d1 · 2ad44d1
1 parent 626d62f
commit 2ad44d1
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 18 deletions.
diff --git a/controllers/k8ssandra/datacenters.go b/controllers/k8ssandra/datacenters.go
@@ -3,7 +3,6 @@ package k8ssandra
 import (
 	"context"
 	"fmt"
-
 	"github.com/go-logr/logr"
 	cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
 	cassctlapi "github.com/k8ssandra/cass-operator/apis/control/v1alpha1"
@@ -104,6 +103,10 @@ func (r *K8ssandraClusterReconciler) reconcileDatacenters(ctx context.Context, k
 		if err = remoteClient.Get(ctx, dcKey, actualDc); err == nil {
 			// cassdc already exists, we'll update it
 
+			// We need to reevaluate rebuildNeeded again. After the CassandraDatacenter is
+			// created, rebuildNeeded will be false on a subsequent reconciliation when we hit this
+			// point, so we have to check for the presence of the rebuild label on the actual
+			// CassandraDatacenter.
 			if _, rebuildNeeded = actualDc.Labels[api.RebuildLabel]; rebuildNeeded {
 				desiredDc.Labels[api.RebuildLabel] = "true"
 				// We need to recompute and reset the resource annotation here. On the

diff --git a/controllers/k8ssandra/k8ssandracluster_controller_test.go b/controllers/k8ssandra/k8ssandracluster_controller_test.go
@@ -73,8 +73,8 @@ func TestK8ssandraCluster(t *testing.T) {
 
 	reconcilerConfig := config.InitConfig()
 
-	reconcilerConfig.DefaultDelay = 1 * time.Second
-	reconcilerConfig.LongDelay = 5 * time.Second
+	reconcilerConfig.DefaultDelay = 100 * time.Millisecond
+	reconcilerConfig.LongDelay = 300 * time.Millisecond
 
 	err := testEnv.Start(ctx, t, func(mgr manager.Manager, clientCache *clientcache.ClientCache, clusters []cluster.Cluster) error {
 		err := (&K8ssandraClusterReconciler{

diff --git a/controllers/k8ssandra/replication.go b/controllers/k8ssandra/replication.go
@@ -76,7 +76,7 @@ func (r *K8ssandraClusterReconciler) updateReplicationOfSystemKeyspaces(
 	}
 
 	keyspaces := []string{"system_traces", "system_distributed", "system_auth"}
-	datacenters := cassandra.GetDatacentersForReplication(kc)
+	datacenters := cassandra.GetDatacentersForSystemReplication(kc)
 	replication := cassandra.ComputeReplication(3, datacenters...)
 
 	logger.Info("Preparing to update replication for system keyspaces", "replication", replication)

diff --git a/pkg/cassandra/datacenter_test.go b/pkg/cassandra/datacenter_test.go
@@ -358,10 +358,6 @@ func TestNewDatacenter_Fail_NoStorageConfig(t *testing.T) {
 }
 
 func TestDatacentersReplication(t *testing.T) {
-	t.Run("EachDcContainsKeyspaces", testEachDcContainsKeyspaces)
-}
-
-func testEachDcContainsKeyspaces(t *testing.T) {
 	assert := assert.New(t)
 
 	replication := &Replication{

diff --git a/pkg/cassandra/util.go b/pkg/cassandra/util.go
@@ -33,7 +33,13 @@ func DatacenterStopping(dc *cassdcapi.CassandraDatacenter) bool {
 	return dc.GetConditionStatus(cassdcapi.DatacenterStopped) == corev1.ConditionTrue && dc.Status.CassandraOperatorProgress == cassdcapi.ProgressUpdating
 }
 
-func GetDatacentersForReplication(kc *api.K8ssandraCluster) []api.CassandraDatacenterTemplate {
+// GetDatacentersForSystemReplication determines the DCs that should be included for
+// replication. This function should only be used for system keyspaces. Replication for
+// system keyspaces is initially set through the management-api, not CQL. This allows us
+// to specify non-existent DCs for replication even though Cassandra 4 does not allow that.
+// That cannot be done when configuration replication through CQL which is why this func
+// should only be used for system keyspaces.
+func GetDatacentersForSystemReplication(kc *api.K8ssandraCluster) []api.CassandraDatacenterTemplate {
 	var datacenters []api.CassandraDatacenterTemplate
 	if kc.Status.GetConditionStatus(api.CassandraInitialized) == corev1.ConditionTrue {
 		datacenters = make([]api.CassandraDatacenterTemplate, 0)
@@ -51,7 +57,7 @@ func GetDatacentersForReplication(kc *api.K8ssandraCluster) []api.CassandraDatac
 func ComputeSystemReplication(kc *api.K8ssandraCluster) SystemReplication {
 	rf := 3.0
 
-	datacenters := GetDatacentersForReplication(kc)
+	datacenters := GetDatacentersForSystemReplication(kc)
 
 	for _, dc := range datacenters {
 		rf = math.Min(rf, float64(dc.Size))

diff --git a/test/e2e/suite_test.go b/test/e2e/suite_test.go
@@ -77,7 +77,7 @@ func TestOperator(t *testing.T) {
 	}))
 	t.Run("AddDcToCluster", e2eTest(ctx, &e2eTestOpts{
 		testFunc: addDcToCluster,
-		fixture: "add-dc",
+		fixture:  "add-dc",
 	}))
 	t.Run("CreateMultiStargateAndDatacenter", e2eTest(ctx, &e2eTestOpts{
 		testFunc:                     createStargateAndDatacenter,
@@ -641,7 +641,7 @@ func addDcToCluster(t *testing.T, ctx context.Context, namespace string, f *fram
 		K8sContext: k8sCtx0,
 		NamespacedName: types.NamespacedName{
 			Namespace: namespace,
-			Name: "dc1",
+			Name:      "dc1",
 		},
 	}
 	checkDatacenterReady(t, ctx, dc1Key, f)
@@ -650,7 +650,7 @@ func addDcToCluster(t *testing.T, ctx context.Context, namespace string, f *fram
 		K8sContext: k8sCtx0,
 		NamespacedName: types.NamespacedName{
 			Namespace: namespace,
-			Name: "test-dc1-stargate",
+			Name:      "test-dc1-stargate",
 		},
 	}
 	checkStargateReady(t, f, ctx, sg1Key)
@@ -659,7 +659,7 @@ func addDcToCluster(t *testing.T, ctx context.Context, namespace string, f *fram
 		K8sContext: k8sCtx0,
 		NamespacedName: types.NamespacedName{
 			Namespace: namespace,
-			Name: "test-dc1-reaper",
+			Name:      "test-dc1-reaper",
 		},
 	}
 	checkReaperReady(t, f, ctx, reaper1Key)
@@ -682,7 +682,7 @@ func addDcToCluster(t *testing.T, ctx context.Context, namespace string, f *fram
 			Name: "dc2",
 		},
 		K8sContext: k8sCtx1,
-		Size: 1,
+		Size:       1,
 	})
 	annotations.AddAnnotation(kc, api.DcReplicationAnnotation, `{"dc2": {"ks1": 1, "ks2": 1}}`)
 
@@ -719,14 +719,14 @@ func addDcToCluster(t *testing.T, ctx context.Context, namespace string, f *fram
 				return false
 			}
 			return strings.Contains(output, "'dc1': '1'") && strings.Contains(output, "'dc2': '1'")
-		}, 1 * time.Minute, 5 * time.Second, "failed to verify replication updated for keyspace %s", ks)
+		}, 1*time.Minute, 5*time.Second, "failed to verify replication updated for keyspace %s", ks)
 	}
 
 	sg2Key := framework.ClusterKey{
 		K8sContext: k8sCtx1,
 		NamespacedName: types.NamespacedName{
 			Namespace: namespace,
-			Name: "test-dc2-stargate",
+			Name:      "test-dc2-stargate",
 		},
 	}
 	checkStargateReady(t, f, ctx, sg2Key)
@@ -735,7 +735,7 @@ func addDcToCluster(t *testing.T, ctx context.Context, namespace string, f *fram
 		K8sContext: k8sCtx1,
 		NamespacedName: types.NamespacedName{
 			Namespace: namespace,
-			Name: "test-dc2-reaper",
+			Name:      "test-dc2-reaper",
 		},
 	}
 	checkReaperReady(t, f, ctx, reaper2Key)