Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Reaper Control Plane Deployment Mode #1388

Merged
merged 1 commit into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG/CHANGELOG-1.19.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,5 @@ Changelog for the K8ssandra Operator, new PRs should update the `unreleased` sec
When cutting a new release, update the `unreleased` heading to the tag being generated and date, like `## vX.Y.Z - YYYY-MM-DD` and create a new placeholder section for `unreleased` entries.

## unreleased

* [FEATURE] [#1277](https://github.com/k8ssandra/k8ssandra-operator/issues/1277) Introduce Reaper's Control Plane deployment mode
3 changes: 2 additions & 1 deletion apis/k8ssandra/v1alpha1/k8ssandracluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ type K8ssandraClusterSpec struct {
Stargate *stargateapi.StargateClusterTemplate `json:"stargate,omitempty"`

// Reaper defines the desired deployment characteristics for Reaper in this K8ssandraCluster.
// If this is non-nil, Reaper will be deployed on every Cassandra datacenter in this K8ssandraCluster.
// If this is non-nil, Reaper might be deployed on every Cassandra datacenter in this K8ssandraCluster, unless
// there is a Control Plane Reaper present. In that case, the K8ssandraCluster will get registered to it.
// +optional
Reaper *reaperapi.ReaperClusterTemplate `json:"reaper,omitempty"`

Expand Down
29 changes: 17 additions & 12 deletions apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ var (
ErrNoReaperAccessMode = fmt.Errorf("reaper StorageConfig.AccessModes not set")
ErrNoReaperResourceRequests = fmt.Errorf("reaper StorageConfig.Resources.Requests not set")
ErrNoReaperStorageRequest = fmt.Errorf("reaper StorageConfig.Resources.Requests.Storage not set")
ErrNoReaperPerDcWithLocal = fmt.Errorf("reaper DeploymentModePerDc is only supported when using cassandra storage")
)

// log is for logging in this package.
Expand Down Expand Up @@ -294,21 +295,25 @@ func (r *K8ssandraCluster) validateReaper() error {
if r.Spec.Reaper == nil {
return nil
}
if r.Spec.Reaper.StorageType != reaperapi.StorageTypeLocal {
return nil
}
if r.Spec.Reaper.StorageConfig == nil {
if r.Spec.Reaper.StorageType == reaperapi.StorageTypeLocal && r.Spec.Reaper.StorageConfig == nil {
return ErrNoReaperStorageConfig
}
// not checking StorageClassName because Kubernetes will use a default one if it's not set
if r.Spec.Reaper.StorageConfig.AccessModes == nil {
return ErrNoReaperAccessMode
}
if r.Spec.Reaper.StorageConfig.Resources.Requests == nil {
return ErrNoReaperResourceRequests
if r.Spec.Reaper.StorageType == reaperapi.StorageTypeLocal {
// we're checking for validity of the storage config in Reaper's webhook too, so this is a duplicate of that
// for now, I don't see a better way of reusing code to validate the storage config
// not checking StorageClassName because Kubernetes will use a default one if it's not set
if r.Spec.Reaper.StorageConfig.AccessModes == nil {
return ErrNoReaperAccessMode
}
if r.Spec.Reaper.StorageConfig.Resources.Requests == nil {
return ErrNoReaperResourceRequests
}
if r.Spec.Reaper.StorageConfig.Resources.Requests.Storage().IsZero() {
return ErrNoReaperStorageRequest
}
}
if r.Spec.Reaper.StorageConfig.Resources.Requests.Storage().IsZero() {
return ErrNoReaperStorageRequest
if r.Spec.Reaper.StorageType == reaperapi.StorageTypeLocal && r.Spec.Reaper.DeploymentMode == reaperapi.DeploymentModePerDc {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we need to enforce this here. Reaper won't come up, in this situation, so it's not the end of the world. On the other hand, it broke one e2e test, so perhaps it'll break stuff for users out there also.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny it broke an e2e test given none of them should use local as storage mode 🤔

Copy link
Contributor Author

@rzvoncek rzvoncek Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've actually added a local storage mode to one of the fixtures in the StatefulSet PR here. This is used by the CreateReaperAndDatacenter e2e test.
The idea of doing this was to cover the STS situation without adding a brand new e2e test, which I suppose we should try to avoid.
Here, for the CP mode, the change seems to me being big enough to warrant a new scenario tho.

return ErrNoReaperPerDcWithLocal
}
return nil
}
6 changes: 6 additions & 0 deletions apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,12 @@ func testReaperStorage(t *testing.T) {
reaperWithoutStorageSize.Spec.Reaper.StorageConfig.Resources.Requests = corev1.ResourceList{}
err = reaperWithoutStorageSize.validateK8ssandraCluster()
required.Error(err)

kc := createClusterObjWithCassandraConfig("kc-with-per-dc-reaper-and-local-storage", "ns")
kc.Spec.Reaper = minimalInMemoryReaperConfig.DeepCopy()
kc.Spec.Reaper.DeploymentMode = reaperapi.DeploymentModePerDc
err = kc.validateK8ssandraCluster()
required.Equal(ErrNoReaperPerDcWithLocal, err)
}

// TestValidateUpdateNumTokens is a unit test for numTokens updates.
Expand Down
46 changes: 23 additions & 23 deletions apis/reaper/v1alpha1/reaper_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@ import (
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.

const (
DeploymentModeSingle = "SINGLE"
DeploymentModePerDc = "PER_DC"
ReaperLabel = "k8ssandra.io/reaper"
DefaultKeyspace = "reaper_db"
StorageTypeCassandra = "cassandra"
StorageTypeLocal = "local"
DeploymentModeSingle = "SINGLE"
DeploymentModePerDc = "PER_DC"
DeploymentModeControlPlane = "CONTROL_PLANE"
ReaperLabel = "k8ssandra.io/reaper"
DefaultKeyspace = "reaper_db"
StorageTypeCassandra = "cassandra"
StorageTypeLocal = "local"
)

type ReaperTemplate struct {
Expand Down Expand Up @@ -235,31 +236,30 @@ type ReaperClusterTemplate struct {

// +optional
// +kubebuilder:default="PER_DC"
// +kubebuilder:validation:Enum:=PER_DC;SINGLE
// +kubebuilder:validation:Enum:=PER_DC;SINGLE;CONTROL_PLANE
DeploymentMode string `json:"deploymentMode,omitempty"`

// When there is a CONTROL_PLANE Reaper out there, this field allows registering a K8ssandra cluster to it.
// Populating this field disables some operator behaviour related to setting Reaper up.
// +optional
ReaperRef corev1.ObjectReference `json:"reaperRef,omitempty"`
}

// EnsureDeploymentMode ensures that a deployment mode is SINGLE if we use the local storage type. This is to prevent
// several instances of Reapers with local storage that would interfere with each other.
func (t *ReaperClusterTemplate) EnsureDeploymentMode() bool {
if t != nil {
if t.StorageType == StorageTypeLocal {
if t.DeploymentMode != DeploymentModeSingle {
t.DeploymentMode = DeploymentModeSingle
return true
}
}
}
return false
func (rct *ReaperClusterTemplate) HasReaperRef() bool {
return rct != nil && rct.ReaperRef.Name != ""
}

func (rct *ReaperClusterTemplate) IsControlPlane() bool {
return rct != nil && rct.DeploymentMode == DeploymentModeControlPlane
}

// CassandraDatacenterRef references the target Cassandra DC that Reaper should manage.
// TODO this object could be used by Stargate too; which currently cannot locate DCs outside of its own namespace.
type CassandraDatacenterRef struct {

// The datacenter name.
// +kubebuilder:validation:Required
Name string `json:"name"`
// +optional
Name string `json:"name,omitempty"`

// The datacenter namespace. If empty, the datacenter will be assumed to reside in the same namespace as the Reaper
// instance.
Expand All @@ -274,8 +274,8 @@ type ReaperSpec struct {
// DatacenterRef is the reference of a CassandraDatacenter resource that this Reaper instance should manage. It will
// also be used as the backend for persisting Reaper's state. Reaper must be able to access the JMX port (7199 by
// default) and the CQL port (9042 by default) on this DC.
// +kubebuilder:validation:Required
DatacenterRef CassandraDatacenterRef `json:"datacenterRef"`
// +optional
DatacenterRef CassandraDatacenterRef `json:"datacenterRef,omitempty"`

// DatacenterAvailability indicates to Reaper its deployment in relation to the target datacenter's network.
// For single-DC clusters, the default (ALL) is fine. For multi-DC clusters, it is recommended to use EACH,
Expand Down
44 changes: 0 additions & 44 deletions apis/reaper/v1alpha1/reaper_types_test.go

This file was deleted.

1 change: 1 addition & 0 deletions apis/reaper/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 49 additions & 5 deletions charts/k8ssandra-operator/crds/k8ssandra-operator-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26559,7 +26559,8 @@ spec:
reaper:
description: |-
Reaper defines the desired deployment characteristics for Reaper in this K8ssandraCluster.
If this is non-nil, Reaper will be deployed on every Cassandra datacenter in this K8ssandraCluster.
If this is non-nil, Reaper might be deployed on every Cassandra datacenter in this K8ssandraCluster, unless
there is a Control Plane Reaper present. In that case, the K8ssandraCluster will get registered to it.
properties:
ServiceAccountName:
default: default
Expand Down Expand Up @@ -27600,6 +27601,7 @@ spec:
enum:
- PER_DC
- SINGLE
- CONTROL_PLANE
type: string
heapSize:
anyOf:
Expand Down Expand Up @@ -28439,6 +28441,52 @@ spec:
format: int32
type: integer
type: object
reaperRef:
description: |-
When there is a CONTROL_PLANE Reaper out there, this field allows registering a K8ssandra cluster to it.
Populating this field disables some operator behaviour related to setting Reaper up.
properties:
apiVersion:
description: API version of the referent.
type: string
fieldPath:
description: |-
If referring to a piece of an object instead of an entire object, this string
should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2].
For example, if the object reference is to a container within a pod, this would take on a value like:
"spec.containers{name}" (where "name" refers to the name of the container that triggered
the event) or if no container name is specified "spec.containers[2]" (container with
index 2 in this pod). This syntax is chosen only to have some well-defined way of
referencing a part of an object.
TODO: this design is not final and this field is subject to change in the future.
type: string
kind:
description: |-
Kind of the referent.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
type: string
name:
description: |-
Name of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
type: string
namespace:
description: |-
Namespace of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/
type: string
resourceVersion:
description: |-
Specific resourceVersion to which this reference is made, if any.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency
type: string
uid:
description: |-
UID of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids
type: string
type: object
x-kubernetes-map-type: atomic
resources:
description: Main Container resources.
properties:
Expand Down Expand Up @@ -33695,8 +33743,6 @@ spec:
The datacenter namespace. If empty, the datacenter will be assumed to reside in the same namespace as the Reaper
instance.
type: string
required:
- name
type: object
heapSize:
anyOf:
Expand Down Expand Up @@ -35282,8 +35328,6 @@ spec:
type: string
type: object
x-kubernetes-map-type: atomic
required:
- datacenterRef
type: object
status:
description: ReaperStatus defines the observed state of Reaper
Expand Down
50 changes: 49 additions & 1 deletion config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26497,7 +26497,8 @@ spec:
reaper:
description: |-
Reaper defines the desired deployment characteristics for Reaper in this K8ssandraCluster.
If this is non-nil, Reaper will be deployed on every Cassandra datacenter in this K8ssandraCluster.
If this is non-nil, Reaper might be deployed on every Cassandra datacenter in this K8ssandraCluster, unless
there is a Control Plane Reaper present. In that case, the K8ssandraCluster will get registered to it.
properties:
ServiceAccountName:
default: default
Expand Down Expand Up @@ -27538,6 +27539,7 @@ spec:
enum:
- PER_DC
- SINGLE
- CONTROL_PLANE
type: string
heapSize:
anyOf:
Expand Down Expand Up @@ -28377,6 +28379,52 @@ spec:
format: int32
type: integer
type: object
reaperRef:
description: |-
When there is a CONTROL_PLANE Reaper out there, this field allows registering a K8ssandra cluster to it.
Populating this field disables some operator behaviour related to setting Reaper up.
properties:
apiVersion:
description: API version of the referent.
type: string
fieldPath:
description: |-
If referring to a piece of an object instead of an entire object, this string
should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2].
For example, if the object reference is to a container within a pod, this would take on a value like:
"spec.containers{name}" (where "name" refers to the name of the container that triggered
the event) or if no container name is specified "spec.containers[2]" (container with
index 2 in this pod). This syntax is chosen only to have some well-defined way of
referencing a part of an object.
TODO: this design is not final and this field is subject to change in the future.
type: string
kind:
description: |-
Kind of the referent.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
type: string
name:
description: |-
Name of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
type: string
namespace:
description: |-
Namespace of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/
type: string
resourceVersion:
description: |-
Specific resourceVersion to which this reference is made, if any.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency
type: string
uid:
description: |-
UID of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids
type: string
type: object
x-kubernetes-map-type: atomic
resources:
description: Main Container resources.
properties:
Expand Down
4 changes: 0 additions & 4 deletions config/crd/bases/reaper.k8ssandra.io_reapers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1181,8 +1181,6 @@ spec:
The datacenter namespace. If empty, the datacenter will be assumed to reside in the same namespace as the Reaper
instance.
type: string
required:
- name
type: object
heapSize:
anyOf:
Expand Down Expand Up @@ -2768,8 +2766,6 @@ spec:
type: string
type: object
x-kubernetes-map-type: atomic
required:
- datacenterRef
type: object
status:
description: ReaperStatus defines the observed state of Reaper
Expand Down
4 changes: 2 additions & 2 deletions controllers/k8ssandra/dcconfigs.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ func (r *K8ssandraClusterReconciler) createDatacenterConfigs(
cassandra.AllowAlterRfDuringRangeMovement(dcConfig)
}

// Inject Reaper settings
if kc.Spec.Reaper != nil {
// Inject Reaper settings, unless we just reference an existing Reaper
if kc.Spec.Reaper != nil && kc.Spec.Reaper.ReaperRef.Name == "" {
reaper.AddReaperSettingsToDcConfig(kc.Spec.Reaper.DeepCopy(), dcConfig, kc.Spec.IsAuthEnabled())
}

Expand Down
Loading
Loading