Improved Reaper integration #307

jsanda · 2020-10-31T02:46:12Z

Cass Operator has the ability to deploy Reaper in sidecar mode. Due to #211 it is not very usable. This PR introduces new and improved integration where Reaper is run as a separate deployment. Cass Operator does not deploy Reaper. This is handled by reaper-operator.

A CassandraDatacenter only needs to specify a couple things as illustrated in this example:

apiVersion: cassandra.datastax.com/v1beta1
kind: CassandraDatacenter
metadata:
  name: dc1
spec:
  clusterName: reaper-test
  serverType: cassandra
  serverVersion: 3.11.7
  managementApiAuth:
    insecure: {}
  size: 3
  allowMultipleNodesPerWorker: false
  config:
    jvm-options:
      initial_heap_size: "800m"
      max_heap_size: "800m"
  resources:
    requests:
      cpu: 2
      memory: 2Gi
    limits:
      cpu: 2
      memory: 2Gi
  reaper:
    enabled: true
    # This is optional. cass-operator will generate a secret if this
    # is not set.
    jmxSecretName: reaper-jmx
    name: reaper-cassdc
  storageConfig:
    cassandraDataVolumeClaimSpec:
      storageClassName: server-storage
      accessModes:
      - ReadWriteOnce
      resources:
        requests:
          storage: 5Gi

When Reaper is deployed on its own, it requires remote JMX for running repairs. This PR introduces changes to configure the cluster with remote JMX when .spec.reaper.enabled is true.

.spec.reaper.name specifies the name of a Reaper object. A namespace can also be specified if the Reaper object lives in a different namespace.

Here is what a Reaper manifest looks like:

apiVersion: reaper.cassandra-reaper.io/v1alpha1
kind: Reaper
metadata:
  name: reaper-cassdc
spec:
  serverConfig:
    storageType: cassandra
    jmxUserSecretName: reaper-test-reaper-jmx
    cassandraBackend:
      clusterName: reaper-test
      replication:
        networkTopologyStrategy:
          dc1: 3
      cassandraService: reaper-test-dc1-service

In this example the Reaper object is configured to also use the Cassandra cluster as its backend; however, this is not required.

When Reaper is run in sidecar mode, it will auto register the cluster to which it is attached. This PR includes changes to automatically register the C* cluster with Reaper. It also includes checks to ensure it remains registered.

There are probably more changes needed with this PR, but it is ready for review.

bradfordcp · 2020-10-31T03:06:28Z

If the operator isn't doing anything with this reaper information would it make more sense to use an annotation instead of fields in the spec?

jsanda · 2020-10-31T03:20:57Z

The operator is using it. The operator needs it to make REST calls to the Reaper instance.

On Fri, Oct 30, 2020 at 11:06 PM Christopher Bradford < ***@***.***> wrote: If the operator isn't doing anything with this reaper information would it make more sense to use an annotation instead of fields in the spec? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#307 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABJBOIBJLO5DOGKCITP4M3SNN5MBANCNFSM4TFT3NYQ> .

-- - John

bradfordcp · 2020-10-31T03:23:01Z

Gotcha, I did not pick up on that from the text. I'll dig into this in a bit.

jsanda · 2020-10-31T03:28:38Z

Thanks!

On Fri, Oct 30, 2020 at 11:23 PM Christopher Bradford < ***@***.***> wrote: Gotcha, I did not pick up on that from the text. I'll dig into this in a bit. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#307 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABJBOPH6UF7ZWONGKZPTVTSNN7KFANCNFSM4TFT3NYQ> .

-- - John

jsanda · 2020-10-31T12:54:02Z

If the operator isn't doing anything with this reaper information would it make more sense to use an annotation instead of fields in the spec?

I thought some more about this. The CassandraDatacenter would need to specify two annotations:

one with the name of the Reaper instance
one with the name of the JMX credentials secret

I could then have a controller in reaper-operator that watches CassandraDatacenters and takes action if those annotations are present. In particular, the controller would make the REST call to Reaper to register the Cassandra cluster.

We would still need cass-operator to set up remote JMX though which it is doing based on .spec.reaper.enabled being true. Other than that cass-operator would not have to do anything Reaper specific as it is doing in this PR. I think that would be a plus.

@bradfordcp / @jimdickinson wdyt?

jimdickinson · 2020-10-31T14:37:03Z

operator/pkg/apis/cassandra/v1beta1/cassandradatacenter_types.go

 	DatacenterResuming       DatacenterConditionType = "Resuming"
 	DatacenterRollingRestart DatacenterConditionType = "RollingRestart"
 	DatacenterValid          DatacenterConditionType = "Valid"
+	DatacenterReaperManage   DatacenterConditionType = "Reaper"


nit - did you mean Manage? or Manager or Managed?

jimdickinson · 2020-10-31T14:37:28Z

operator/pkg/apis/cassandra/v1beta1/cassandradatacenter_types.go

 }

 type ReaperConfig struct {
+	// Other properties in ReaperConfig are ignores unless this is set to true. When set to


Suggested change

// Other properties in ReaperConfig are ignores unless this is set to true. When set to

// Other properties in ReaperConfig are ignored unless this is set to true. When set to

jimdickinson · 2020-10-31T14:38:34Z

operator/pkg/apis/cassandra/v1beta1/cassandradatacenter_types.go

 type ReaperConfig struct {
+	// Other properties in ReaperConfig are ignores unless this is set to true. When set to
+	// true, the operator will enable remote JMX in the Cassandra cluster since Reaper
+	// manages repairs via JMX. Note that setting this to true after the Cassandra cluster


nit - is this more clear about the expected behavior?

Suggested change

// manages repairs via JMX. Note that setting this to true after the Cassandra cluster

// manages repairs via JMX. Note that toggling this to true after the Cassandra cluster

jsanda · 2020-11-01T05:08:37Z

I prototyped a new controller in reaper-operator that watches CassandraDatacenters. The code lives here https://github.com/jsanda/reaper-operator/blob/cassdc-controller/controllers/cassandradatacenter_controller.go. I am going to move forward with this approach.

For this PR that means I only need the changes for setting up remote JMX. I am going to update the PR accordingly. I am still going to remove the Reaper sidecar stuff since it was broken.

In summary, setting .spec.reaper.enabled to true will result only result in cass-operator enabling and configuring remote JMX.

jsanda · 2020-11-01T15:01:46Z

@jimdickinson I removed Reaper specific code since that will be handled in reaper-operator. We can probably remove enable_reaper_test_suite.go as well.

I think it would make more sense to add something like:

type RemoteJmxConfig struct  {
        Enabled bool

        JmxSecretName string
}

Remote JMX is not specific to Reaper. As a user who wants to use some remote management tool (other than Reaper) with my cluster, .spec.remoteJmx.enabled would be more obvious and clear than .spec.reaper.enabled. If we add something like this, I would still leave the existing ReaperConfig struct as is for deprecation.

bradfordcp · 2020-11-03T20:47:05Z

Rerunning checks

johntrimble · 2020-11-04T19:25:12Z

operator/pkg/reconciliation/reconcile_racks.go

-	}
+	//if recResult := rc.CheckReaperSchemaInitialized(); recResult.Completed() {
+	//	return recResult.Output()
+	//}


Just delete these lines if not needed?

We want the server config init container to run first so other initContainers can access the server config files.

jsanda · 2020-12-17T03:55:53Z

I am closing this out. See #335 for details.

jsanda added 12 commits October 29, 2020 00:34

initial support for separate Reaper deployment

2ba530f

add reaper readiness check before registering the cluster

ead3c79

comment out reaper sidecar related calls for now

1685057

update unit tests

58cb055

add status condition and check reaper status

7d1319a

create default reaper jmx secret if none specified

f7f0f74

add helper function for jmx secret name

53d99e1

call CheckReaperJmxSecretCreation

78d0f9a

forgot call to actually persist the secret

5d4c7a0

fix logic for jmx secret creation

bc52bfa

update reaper int test to test against separate reaper deployment

0817edb

remove unused code, update ReaperConfig properties, and add some docs

4485471

jsanda requested review from bradfordcp, jimdickinson, johntrimble, respringer and sandoichi as code owners October 31, 2020 02:46

update CRD

d47dbb8

jimdickinson reviewed Oct 31, 2020

View reviewed changes

remove Reaper specific code which will handled in reaper-operator

3942b61

jimdickinson mentioned this pull request Nov 3, 2020

Use C* authentication with Reaper #222

Closed

johntrimble reviewed Nov 4, 2020

View reviewed changes

run the server config init container first

1dadd86

We want the server config init container to run first so other initContainers can access the server config files.

jsanda closed this Dec 17, 2020

	// Other properties in ReaperConfig are ignores unless this is set to true. When set to
	// Other properties in ReaperConfig are ignored unless this is set to true. When set to

	// manages repairs via JMX. Note that setting this to true after the Cassandra cluster
	// manages repairs via JMX. Note that toggling this to true after the Cassandra cluster

Improved Reaper integration #307

Improved Reaper integration #307

Uh oh!

Conversation

jsanda commented Oct 31, 2020

Uh oh!

bradfordcp commented Oct 31, 2020

Uh oh!

jsanda commented Oct 31, 2020 via email

Uh oh!

bradfordcp commented Oct 31, 2020

Uh oh!

jsanda commented Oct 31, 2020 via email

Uh oh!

jsanda commented Oct 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimdickinson Oct 31, 2020

Choose a reason for hiding this comment

Uh oh!

jimdickinson Oct 31, 2020

Choose a reason for hiding this comment

Uh oh!

jimdickinson Oct 31, 2020

Choose a reason for hiding this comment

Uh oh!

jsanda commented Nov 1, 2020

Uh oh!

jsanda commented Nov 1, 2020

Uh oh!

bradfordcp commented Nov 3, 2020

Uh oh!

johntrimble Nov 4, 2020

Choose a reason for hiding this comment

Uh oh!

jsanda commented Dec 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jsanda commented Oct 31, 2020 •

edited

Loading