Add datacenter to existing cluster [K8SSAND-615][K8SSAND-784]

[jsanda]

What this PR does:

Which issue(s) this PR fixes:
Fixes #21

Checklist

• Changes manually tested
• Automated Tests added/updated
• Documentation added/updated
• CHANGELOG.md updated (not required for documentation PRs)
• CLA Signed: DataStax CLA

[jsanda]

I think we need to change when we reconcile the Stargate auth and Reaper schemas. The schemas should be reconciled after each CassandraDatacenter is reconciled.

Here is how things are currently done in K8ssandraReconciler (this is in the main branch as well):

	var actualDcs []*cassdcapi.CassandraDatacenter
	if recResult, dcs := r.reconcileDatacenters(ctx, kc, kcLogger); recResult.Completed() {
		return recResult.Output()
	} else {
		actualDcs = dcs
	}

	kcLogger.Info("All dcs reconciled")

	if recResult := r.reconcileStargateAuthSchema(ctx, kc, actualDcs, kcLogger); recResult.Completed() {
		return recResult.Output()
	}

	if recResult := r.reconcileReaperSchema(ctx, kc, actualDcs, kcLogger); recResult.Completed() {
		return recResult.Output()
	}

	if recResult := r.reconcileStargateAndReaper(ctx, kc, actualDcs, kcLogger); recResult.Completed() {
		return recResult.Output()
	}

We first reconcile all CassandraDatacenters. We will requeue until all DCs are ready. Then we reconcile the Stargate auth and Reaper schemas. These keyspaces are created if necessary. Lastly, we reconcile Stargate and Reaper across all DCs. In other words, reconcileStargateAndReaper iterates through the CassandraDatacenters and adds/updates/removes Stargate and Reaper as necessary for each one.

There are a number of changes to the reconcileDatacenters method in my add-dc branch. I'll try to highlight the relevant parts and leave out the rest:

	for idx, dcTemplate := range kc.Spec.Cassandra.Datacenters {
                ...
		if err = remoteClient.Get(ctx, dcKey, actualDc); err == nil {
			...

                         logger.Info("The datacenter is ready")

			actualDcs = append(actualDcs, actualDc)

			if recResult := r.updateReplicationOfSystemKeyspaces(ctx, kc, desiredDc, remoteClient, logger); recResult.Completed() {
				return recResult, actualDcs
			}

			if rebuildNeeded {
				// TODO We need to handle the Stargate auth and Reaper keyspaces here.
				if recResult := r.updateUserKeyspacesReplication(ctx, kc, desiredDc, remoteClient, logger); recResult.Completed() {
					return recResult, actualDcs
				}

				if recResult := reconcileDcRebuild(ctx, actualDc, actualDcs, remoteClient, logger); recResult.Completed() {
					return recResult, actualDcs
				}
			}
		} else {
                        ...
		}
	}

After the current DC is ready, updateReplicationOfSystemKeyspaces is called. It updates replication for system keyspaces, e.g., system_auth, if necessary. For example, suppose the DC has been scaled up from 1 to 3 C* nodes. The replication factor would change to 3.

If rebuildNeeded is true, we call updateUserKeyspacesReplication to add replicas for the new DC. Lastly, we perform the rebuild by calling reconcileDcRebuild.

Note the earlier TODO about handling the Stargate and Reaper keyspaces. If Stargate and Reaper are already installed we need to update the replication for their keyspaces before doing the rebuild so that the keyspaces are included in the streaming. It is easy enough to add the necessary calls where the TODO is.

Now let's say we have an existing K8ssandraCluster that does not have Stargate and Reaper enabled. Suppose we update the K8ssandraCluster by adding a second DC and enabling Stargate and Reaper in each DC. With the code as is, the keyspaces won't get created until after the rebuild of dc2. I think we are ok in this case.

As a last example, consider removing a DC from the cluster. One of the first steps we perform in this process is to stop replicating to the DC. We can't do that if we reconcile the Stargate and Reaper keyspaces after reconciling all DCs.

Based on my findings I am going to move the calls to reconcile the Stargate and Reaper keyspaces to happen after each DC is ready.

@adutra any thoughts/concerns?

[adutra]

@jsanda sorry for the late reply. The suggested changes make sense and look good to me. I suppose many tests will fail because the order in which they wait for components to show up, but other than that, I don't see any red flags.

[burmanm]

When adding a new cluster, we don't actually have any replication rule updates. I don't see here if we actually verify that some secrets are even copied to new clustrs? It's not strictly necessary per-datacenter of course, since datacenter could be replicated to an existing cluster in another namespace, but I wonder if these kind of issues came to mind (at least in documentation)

[jsanda]

verifyReplicatedSecretReconciled is called in each test. It verifies that the ReplicationDone condition gets set.

[burmanm]

I'm slightly confused here, why do we want only Ready ones if initialized is true, but everything if Initialized is false? Feels unnatural why this path should ever be taken instead of just returning nothing.

[burmanm]

Or is this path taken when we initialize all the datacenters the first time K8ssandraCluster is created?

[jsanda]

Or is this path taken when we initialize all the datacenters the first time K8ssandraCluster is created?

short answer - yes

long answer...

We configure replication of system keyspaces by setting a couple system properties:

• cassandra.system_distributed_replication_dc_names
• cassandra.system_distributed_replication_per_dc

management-api looks for these properties and applies changes to the system_auth keyspace using internal Cassandra APIs, not CQL. If we are creating a cluster with 2 dcs, dc1 and dc2, both will be configured for system_auth. Note that what I am describing is already the current behavior in main.

In 4.0 Cassandra won't let you specify the replication for a keyspace that does not exist (at least through CQL). I had to introduce changes in this PR for configuring the replication of the Stargate auth and Reaper keyspaces. Those are configured via CQL calls. So going back to the 2 DCs example, when dc1 becomes ready we can create the keyspaces and specify replication for dc1 but not for dc2. Cassandra won't allow this. This resulted in different handling of system keyspaces vs other keyspaces.

Once I added support for handling non-system keyspaces, I attempted (in vain) to handle system keyspaces the same way to make the code less confusing and more maintainable. There are queries with a LOCAL_ONE consistency executed in dc2 that will fail with auth error because the replica(s) in dc2 need a repair.

In summary, until we implement support for running repairs, at least of system_auth, we have to handle system keyspaces the way we currently do. That means 1) for system keyspaces we need to specify replication for all DCs up front and 2) for non-system keyspaces we cannot specify the replication for all DCs up front.

I will rename the method and add some docs to make it more clear that it is only intended for system keyspaces.

[adutra]

Second round of remarks :-)

[adutra]

Approving this as the e2e failures seem unrelated. A few minor questions are still inflight but can be revisited later. It would be good to have this merged asap.