operator v1: make spec.resources optional

in webhook, check if at least a single NodePool is specified.
redpanda-data · Sep 11, 2024 · 58151c2 · 58151c2
1 parent e02ac4b
commit 58151c2
Show file tree

Hide file tree

Showing 10 changed files with 141 additions and 66 deletions.
diff --git a/src/go/k8s/api/vectorized/v1alpha1/cluster_types.go b/src/go/k8s/api/vectorized/v1alpha1/cluster_types.go
@@ -196,6 +196,9 @@ type ClusterSpec struct {
 // Replicas, so they can be controlled independently
 // Resources, because this is tied strongly to the actual machine shape backing the NodePool.
 type NodePoolSpec struct {
+	// Name of the NodePool. Must be unique, and must not be "default".
+	// +kubebuilder:validation:MinLength=3
+	// +required
 	Name string `json:"name"`
 	// Replicas determine how big the node pool will be.
 	// +kubebuilder:validation:Minimum=0
@@ -206,13 +209,15 @@ type NodePoolSpec struct {
 	// https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
 	// Storage spec for cluster
+	// +required
 	Storage StorageSpec `json:"storage,omitempty"`
 	// Resources used by redpanda process running in container. Beware that
 	// there are multiple containers running in the redpanda pod and these can
 	// be enabled/disabled and configured from the `sidecars` field. These
 	// containers have separate resources settings and the amount of resources
 	// assigned to these containers will be required on the cluster on top of
 	// the resources defined here
+	// +required
 	Resources RedpandaResourceRequirements `json:"resources"`
 }
 

diff --git a/src/go/k8s/api/vectorized/v1alpha1/cluster_webhook.go b/src/go/k8s/api/vectorized/v1alpha1/cluster_webhook.go
@@ -109,7 +109,7 @@ func (r *Cluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 var _ webhook.Defaulter = &Cluster{}
 
 func redpandaResourceFields(c *Cluster) redpandaResourceField {
-	return redpandaResourceField{&c.Spec.Resources, field.NewPath("spec").Child("resources")}
+	return redpandaResourceField{c.Spec.Resources, field.NewPath("spec").Child("resources")}
 }
 
 func sidecarResourceFields(c *Cluster) []resourceField {
@@ -266,6 +266,49 @@ func (r *Cluster) validateCommon(log logr.Logger) field.ErrorList {
 		allErrs = append(allErrs, r.validateAdditionalConfiguration()...)
 	}
 	allErrs = append(allErrs, r.validatePriorityClassName()...)
+	allErrs = append(allErrs, r.validateNodePools()...)
+	return allErrs
+}
+
+// Validate if NodePools are configured correctly.
+// Make sure, that  at least one NodePool is provided:
+// - Default NodePool by providing non-nil spec.replicas
+// - NodePool(s) configured via spec.nodePools
+func (r *Cluster) validateNodePools() field.ErrorList {
+	var allErrs field.ErrorList
+	if r.Spec.Replicas == nil {
+		// Forbid empty spec.replicas for now, it will be only truly optional once nodePool implementation lands.
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec").Child("replicas"),
+				r.Spec.Replicas,
+				"spec.replicas is a mandatory field, until support for spec.nodePools is implemented"))
+	}
+
+	if r.Spec.Resources == nil {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec").Child("resources"),
+				r.Spec.Replicas,
+				"spec.resources is a mandatory field, until support for spec.nodePools is implemented"))
+	}
+
+	dupes := map[string]struct{}{}
+
+	for i := range r.Spec.NodePools {
+		np := r.Spec.NodePools[i]
+		if _, found := dupes[np.Name]; found {
+			if np.Name == "default" {
+				allErrs = append(allErrs,
+					field.Invalid(field.NewPath("spec").Child("nodePools").Child("name"), np.Name,
+						"NodePool name default is reserved and may not be used in spec.nodePools"))
+			}
+			allErrs = append(allErrs,
+				field.Invalid(
+					field.NewPath("spec").Child("nodePools").Child("name"), np.Name,
+					fmt.Sprintf("duplicated NodePool name: %s", np.Name)))
+		}
+		dupes[np.Name] = struct{}{}
+	}
+
 	return allErrs
 }
 
@@ -724,15 +767,19 @@ func (r *Cluster) validateResources(rf resourceField) field.ErrorList {
 func (r *Cluster) validateRedpandaResources(
 	rf redpandaResourceField,
 ) field.ErrorList {
-	allErrs := r.validateResources(resourceField{&rf.resources.ResourceRequirements, rf.path})
+	var allErrs field.ErrorList
 
-	// Memory redpanda (if set) must be less than or equal to request (if set)
-	if !rf.resources.Requests.Memory().IsZero() && !rf.resources.RedpandaMemory().IsZero() && rf.resources.Requests.Memory().Cmp(*rf.resources.RedpandaMemory()) == -1 {
-		allErrs = append(allErrs,
-			field.Invalid(
-				rf.path.Child("redpanda").Child("memory"),
-				rf.resources.Requests.Memory(),
-				"requests.memory < redpanda.memory; decrease or remove redpanda.memory"))
+	if rf.resources != nil {
+		allErrs = append(allErrs, r.validateResources(resourceField{&rf.resources.ResourceRequirements, rf.path})...)
+
+		// Memory redpanda (if set) must be less than or equal to request (if set)
+		if !rf.resources.Requests.Memory().IsZero() && !rf.resources.RedpandaMemory().IsZero() && rf.resources.Requests.Memory().Cmp(*rf.resources.RedpandaMemory()) == -1 {
+			allErrs = append(allErrs,
+				field.Invalid(
+					rf.path.Child("redpanda").Child("memory"),
+					rf.resources.Requests.Memory(),
+					"requests.memory < redpanda.memory; decrease or remove redpanda.memory"))
+		}
 	}
 
 	return allErrs
@@ -770,26 +817,28 @@ func (r *Cluster) validateRedpandaMemory() field.ErrorList {
 	}
 	var allErrs field.ErrorList
 
-	// Ensure a requested 2GB of memory per core
-	requests := r.Spec.Resources.Requests.DeepCopy()
-	requests.Cpu().RoundUp(0)
-	requestedCores := requests.Cpu().Value()
-	if r.Spec.Resources.Requests.Memory().Value() < requestedCores*MinimumMemoryPerCore {
-		allErrs = append(allErrs,
-			field.Invalid(
-				field.NewPath("spec").Child("resources").Child("requests").Child("memory"),
-				r.Spec.Resources.Requests.Memory(),
-				"requests.memory < 2Gi per core; either decrease requests.cpu or increase requests.memory"))
-	}
+	if r.Spec.Resources != nil {
+		// Ensure a requested 2GB of memory per core
+		requests := r.Spec.Resources.Requests.DeepCopy()
+		requests.Cpu().RoundUp(0)
+		requestedCores := requests.Cpu().Value()
+		if r.Spec.Resources.Requests.Memory().Value() < requestedCores*MinimumMemoryPerCore {
+			allErrs = append(allErrs,
+				field.Invalid(
+					field.NewPath("spec").Child("resources").Child("requests").Child("memory"),
+					r.Spec.Resources.Requests.Memory(),
+					"requests.memory < 2Gi per core; either decrease requests.cpu or increase requests.memory"))
+		}
 
-	redpandaCores := r.Spec.Resources.RedpandaCPU().Value()
-	minimumMemoryPerCore := int64(math.Floor(MinimumMemoryPerCore * RedpandaMemoryAllocationRatio))
-	if !r.Spec.Resources.RedpandaMemory().IsZero() && r.Spec.Resources.RedpandaMemory().Value() < redpandaCores*minimumMemoryPerCore {
-		allErrs = append(allErrs,
-			field.Invalid(
-				field.NewPath("spec").Child("resources").Child("redpanda").Child("memory"),
-				r.Spec.Resources.Requests.Memory(),
-				"redpanda.memory < 2Gi per core; either decrease redpanda.cpu or increase redpanda.memory"))
+		redpandaCores := r.Spec.Resources.RedpandaCPU().Value()
+		minimumMemoryPerCore := int64(math.Floor(MinimumMemoryPerCore * RedpandaMemoryAllocationRatio))
+		if !r.Spec.Resources.RedpandaMemory().IsZero() && r.Spec.Resources.RedpandaMemory().Value() < redpandaCores*minimumMemoryPerCore {
+			allErrs = append(allErrs,
+				field.Invalid(
+					field.NewPath("spec").Child("resources").Child("redpanda").Child("memory"),
+					r.Spec.Resources.Requests.Memory(),
+					"redpanda.memory < 2Gi per core; either decrease redpanda.cpu or increase redpanda.memory"))
+		}
 	}
 
 	return allErrs
@@ -804,19 +853,21 @@ func (r *Cluster) validateRedpandaCoreChanges(old *Cluster) field.ErrorList {
 	}
 	var allErrs field.ErrorList
 
-	oldCPURequest := old.Spec.Resources.RedpandaCPU()
-	newCPURequest := r.Spec.Resources.RedpandaCPU()
-	if oldCPURequest != nil && newCPURequest != nil {
-		oldCores := oldCPURequest.Value()
-		newCores := newCPURequest.Value()
+	if old.Spec.Resources != nil && r.Spec.Resources != nil {
+		oldCPURequest := old.Spec.Resources.RedpandaCPU()
+		newCPURequest := r.Spec.Resources.RedpandaCPU()
+		if oldCPURequest != nil && newCPURequest != nil {
+			oldCores := oldCPURequest.Value()
+			newCores := newCPURequest.Value()
 
-		if newCores < oldCores {
-			minAllowedCPU := (oldCores-1)*1000 + 1
-			allErrs = append(allErrs,
-				field.Invalid(
-					field.NewPath("spec").Child("resources").Child("requests").Child("cpu"),
-					r.Spec.Resources.Requests.Cpu(),
-					fmt.Sprintf("CPU request must not be decreased; increase requests.cpu or redpanda.cpu to at least %dm", minAllowedCPU)))
+			if newCores < oldCores {
+				minAllowedCPU := (oldCores-1)*1000 + 1
+				allErrs = append(allErrs,
+					field.Invalid(
+						field.NewPath("spec").Child("resources").Child("requests").Child("cpu"),
+						r.Spec.Resources.Requests.Cpu(),
+						fmt.Sprintf("CPU request must not be decreased; increase requests.cpu or redpanda.cpu to at least %dm", minAllowedCPU)))
+			}
 		}
 	}
 

diff --git a/src/go/k8s/api/vectorized/v1alpha1/cluster_webhook_test.go b/src/go/k8s/api/vectorized/v1alpha1/cluster_webhook_test.go
@@ -98,7 +98,7 @@ func TestDefault(t *testing.T) {
 			Spec: v1alpha1.ClusterSpec{
 				Replicas:      ptr.To(int32(1)),
 				Configuration: v1alpha1.RedpandaConfig{},
-				Resources: v1alpha1.RedpandaResourceRequirements{
+				Resources: &v1alpha1.RedpandaResourceRequirements{
 					ResourceRequirements: corev1.ResourceRequirements{
 						Requests: corev1.ResourceList{
 							corev1.ResourceMemory: resource.MustParse("1Gi"),
@@ -123,7 +123,7 @@ func TestDefault(t *testing.T) {
 				Configuration: v1alpha1.RedpandaConfig{
 					SchemaRegistry: &v1alpha1.SchemaRegistryAPI{},
 				},
-				Resources: v1alpha1.RedpandaResourceRequirements{
+				Resources: &v1alpha1.RedpandaResourceRequirements{
 					ResourceRequirements: corev1.ResourceRequirements{
 						Requests: corev1.ResourceList{
 							corev1.ResourceMemory: resource.MustParse("1Gi"),
@@ -150,7 +150,7 @@ func TestDefault(t *testing.T) {
 						Port: 999,
 					},
 				},
-				Resources: v1alpha1.RedpandaResourceRequirements{
+				Resources: &v1alpha1.RedpandaResourceRequirements{
 					ResourceRequirements: corev1.ResourceRequirements{
 						Requests: corev1.ResourceList{
 							corev1.ResourceMemory: resource.MustParse("1Gi"),
@@ -179,7 +179,7 @@ func TestDefault(t *testing.T) {
 						},
 					},
 				},
-				Resources: v1alpha1.RedpandaResourceRequirements{
+				Resources: &v1alpha1.RedpandaResourceRequirements{
 					ResourceRequirements: corev1.ResourceRequirements{
 						Requests: corev1.ResourceList{
 							corev1.ResourceMemory: resource.MustParse("1Gi"),
@@ -251,7 +251,7 @@ func TestValidateUpdate(t *testing.T) {
 		Spec: v1alpha1.ClusterSpec{
 			Replicas:      ptr.To(replicas3),
 			Configuration: v1alpha1.RedpandaConfig{},
-			Resources: v1alpha1.RedpandaResourceRequirements{
+			Resources: &v1alpha1.RedpandaResourceRequirements{
 				ResourceRequirements: corev1.ResourceRequirements{
 					Requests: corev1.ResourceList{
 						corev1.ResourceCPU:    resource.MustParse("1"),
@@ -339,7 +339,7 @@ func TestValidateUpdate_NoError(t *testing.T) {
 				SchemaRegistry: &v1alpha1.SchemaRegistryAPI{Port: 127},
 				PandaproxyAPI:  []v1alpha1.PandaproxyAPI{{Port: 128}},
 			},
-			Resources: v1alpha1.RedpandaResourceRequirements{
+			Resources: &v1alpha1.RedpandaResourceRequirements{
 				ResourceRequirements: corev1.ResourceRequirements{
 					Requests: corev1.ResourceList{
 						corev1.ResourceMemory: resource.MustParse("2Gi"),
@@ -644,6 +644,9 @@ func TestValidateUpdate_NoError(t *testing.T) {
 
 	t.Run("resource limits/requests on redpanda resources", func(t *testing.T) {
 		c := redpandaCluster.DeepCopy()
+		if c.Spec.Resources == nil {
+			c.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{}
+		}
 		c.Spec.Resources.Limits = corev1.ResourceList{
 			corev1.ResourceMemory: resource.MustParse("1Gi"),
 			corev1.ResourceCPU:    resource.MustParse("1"),
@@ -723,6 +726,10 @@ func TestValidateUpdate_NoError(t *testing.T) {
 	for _, tc := range decreaseCases {
 		t.Run(fmt.Sprintf("CPU request change from %s to %s", tc.initial, tc.target), func(t *testing.T) {
 			oldCluster := redpandaCluster.DeepCopy()
+			if oldCluster.Spec.Resources == nil {
+				oldCluster.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{}
+			}
+
 			oldCluster.Spec.Resources.Requests = corev1.ResourceList{
 				corev1.ResourceMemory: resource.MustParse("20Gi"),
 				corev1.ResourceCPU:    resource.MustParse(tc.initial),
@@ -733,6 +740,10 @@ func TestValidateUpdate_NoError(t *testing.T) {
 			}
 
 			newCluster := redpandaCluster.DeepCopy()
+			if newCluster.Spec.Resources == nil {
+				newCluster.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{}
+			}
+
 			newCluster.Spec.Resources.Requests = corev1.ResourceList{
 				corev1.ResourceMemory: resource.MustParse("20Gi"),
 				corev1.ResourceCPU:    resource.MustParse(tc.target),
@@ -840,7 +851,7 @@ func TestCreation(t *testing.T) {
 
 	t.Run("incorrect memory (need 2GB per core)", func(t *testing.T) {
 		memory := redpandaCluster.DeepCopy()
-		memory.Spec.Resources = v1alpha1.RedpandaResourceRequirements{
+		memory.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{
 			ResourceRequirements: corev1.ResourceRequirements{
 				Requests: corev1.ResourceList{
 					corev1.ResourceMemory: resource.MustParse("1Gi"),
@@ -856,7 +867,7 @@ func TestCreation(t *testing.T) {
 
 	t.Run("no 2GB per core required when in developer mode", func(t *testing.T) {
 		memory := redpandaCluster.DeepCopy()
-		memory.Spec.Resources = v1alpha1.RedpandaResourceRequirements{
+		memory.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{
 			ResourceRequirements: corev1.ResourceRequirements{
 				Requests: corev1.ResourceList{
 					corev1.ResourceMemory: resource.MustParse("1Gi"),
@@ -873,7 +884,7 @@ func TestCreation(t *testing.T) {
 
 	t.Run("incorrect redpanda memory (need <= request)", func(t *testing.T) {
 		memory := redpandaCluster.DeepCopy()
-		memory.Spec.Resources = v1alpha1.RedpandaResourceRequirements{
+		memory.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{
 			ResourceRequirements: corev1.ResourceRequirements{
 				Requests: corev1.ResourceList{
 					corev1.ResourceMemory: resource.MustParse("2Gi"),
@@ -893,7 +904,7 @@ func TestCreation(t *testing.T) {
 	//nolint:dupl // the values are different
 	t.Run("incorrect redpanda memory (need <= limit)", func(t *testing.T) {
 		memory := redpandaCluster.DeepCopy()
-		memory.Spec.Resources = v1alpha1.RedpandaResourceRequirements{
+		memory.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{
 			ResourceRequirements: corev1.ResourceRequirements{
 				Requests: corev1.ResourceList{
 					corev1.ResourceMemory: resource.MustParse("2Gi"),
@@ -917,7 +928,7 @@ func TestCreation(t *testing.T) {
 	//nolint:dupl // the values are different
 	t.Run("correct redpanda memory", func(t *testing.T) {
 		memory := redpandaCluster.DeepCopy()
-		memory.Spec.Resources = v1alpha1.RedpandaResourceRequirements{
+		memory.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{
 			ResourceRequirements: corev1.ResourceRequirements{
 				Requests: corev1.ResourceList{
 					corev1.ResourceMemory: resource.MustParse("2.223Gi"),
@@ -941,7 +952,7 @@ func TestCreation(t *testing.T) {
 	//nolint:dupl // the values are different
 	t.Run("correct redpanda memory (boundary check)", func(t *testing.T) {
 		memory := redpandaCluster.DeepCopy()
-		memory.Spec.Resources = v1alpha1.RedpandaResourceRequirements{
+		memory.Spec.Resources = &v1alpha1.RedpandaResourceRequirements{
 			ResourceRequirements: corev1.ResourceRequirements{
 				Requests: corev1.ResourceList{
 					corev1.ResourceMemory: resource.MustParse("2Gi"),
@@ -1968,7 +1979,7 @@ func validRedpandaCluster() *v1alpha1.Cluster {
 				SchemaRegistry: &v1alpha1.SchemaRegistryAPI{Port: 130},
 				PandaproxyAPI:  []v1alpha1.PandaproxyAPI{{Port: 132}},
 			},
-			Resources: v1alpha1.RedpandaResourceRequirements{
+			Resources: &v1alpha1.RedpandaResourceRequirements{
 				ResourceRequirements: corev1.ResourceRequirements{
 					Requests: corev1.ResourceList{
 						corev1.ResourceMemory: resource.MustParse("2Gi"),

diff --git a/src/go/k8s/config/crd/bases/redpanda.vectorized.io_clusters.yaml b/src/go/k8s/config/crd/bases/redpanda.vectorized.io_clusters.yaml
@@ -1028,6 +1028,9 @@ spec:
                     Resources, because this is tied strongly to the actual machine shape backing the NodePool.
                   properties:
                     name:
+                      description: Name of the NodePool. Must be unique, and must
+                        not be "default".
+                      minLength: 3
                       type: string
                     nodeSelector:
                       additionalProperties:
@@ -1167,6 +1170,7 @@ spec:
                   required:
                   - name
                   - resources
+                  - storage
                   type: object
                 type: array
               nodeSelector:

diff --git a/src/go/k8s/internal/controller/test/cluster_controller_configuration_test.go b/src/go/k8s/internal/controller/test/cluster_controller_configuration_test.go
@@ -925,7 +925,7 @@ func getInitialTestCluster(
 				},
 				AdminAPI: []v1alpha1.AdminAPI{{Port: 9644}},
 			},
-			Resources: v1alpha1.RedpandaResourceRequirements{
+			Resources: &v1alpha1.RedpandaResourceRequirements{
 				ResourceRequirements: corev1.ResourceRequirements{
 					Limits: corev1.ResourceList{
 						corev1.ResourceCPU:    resource.MustParse("1"),

diff --git a/src/go/k8s/internal/controller/test/cluster_controller_scale_test.go b/src/go/k8s/internal/controller/test/cluster_controller_scale_test.go
@@ -275,7 +275,7 @@ func getClusterWithReplicas(
 					Port: 33145,
 				},
 			},
-			Resources: vectorizedv1alpha1.RedpandaResourceRequirements{
+			Resources: &vectorizedv1alpha1.RedpandaResourceRequirements{
 				ResourceRequirements: corev1.ResourceRequirements{
 					Limits: corev1.ResourceList{
 						corev1.ResourceCPU:    resource.MustParse("1"),