feat(cluster): enhance resource management by adding separate resourc…

…e handling for leader and follower (#1199). * feat: enhance RedisCluster resource management by introducing separate resource handling for leader and follower - Added methods to retrieve resources for Redis leader and follower, allowing for more granular resource management. - Updated RedisClusterSpec to deprecate the unified resources field in favor of individual configurations for leader and follower. - Adjusted related tests and YAML configurations to reflect these changes, ensuring consistency and correctness in resource allocation. This update improves the flexibility and efficiency of resource management in RedisCluster deployments. Signed-off-by: drivebyer <wuyangmuc@gmail.com> * fix verify-codegen Signed-off-by: drivebyer <wuyangmuc@gmail.com> * fix codegen Signed-off-by: drivebyer <wuyangmuc@gmail.com> * fix lint Signed-off-by: drivebyer <wuyangmuc@gmail.com> --------- Signed-off-by: drivebyer <wuyangmuc@gmail.com>
OT-CONTAINER-KIT · Jan 5, 2025 · 0d4ce9c · 0d4ce9c
1 parent 888e0ed
commit 0d4ce9c
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 17 deletions.
diff --git a/Makefile b/Makefile
@@ -125,11 +125,12 @@ codegen: generate manifests ## Rebuild all generated code
 .PHONY: verify-codegen
 verify-codegen: codegen
 	@echo Checking codegen is up to date... >&2
-	@git --no-pager diff -- .
-	@echo 'If this test fails, it is because the git diff is non-empty after running "make codegen".' >&2
-	@echo 'To correct this, locally run "make codegen", commit the changes, and re-run tests.' >&2
-	@git diff --quiet --exit-code -- .
-
+	@if [ -n "$$(git status --porcelain)" ]; then \
+		echo "There are uncommitted changes or untracked files after running codegen:" >&2; \
+		git status --porcelain >&2; \
+		echo "To correct this, locally run 'make codegen', commit the changes, and re-run tests." >&2; \
+		exit 1; \
+	fi
 
 ########
 # TEST #

diff --git a/api/v1beta2/rediscluster_types.go b/api/v1beta2/rediscluster_types.go
@@ -58,6 +58,24 @@ func (cr *RedisClusterSpec) GetReplicaCounts(t string) int32 {
 	return *replica
 }
 
+// GetRedisLeaderResources returns the resources for the redis leader, if not set, it will return the default resources
+func (cr *RedisClusterSpec) GetRedisLeaderResources() *corev1.ResourceRequirements {
+	if cr.RedisLeader.Resources != nil {
+		return cr.RedisLeader.Resources
+	}
+
+	return cr.KubernetesConfig.Resources
+}
+
+// GetRedisFollowerResources returns the resources for the redis follower, if not set, it will return the default resources
+func (cr *RedisClusterSpec) GetRedisFollowerResources() *corev1.ResourceRequirements {
+	if cr.RedisFollower.Resources != nil {
+		return cr.RedisFollower.Resources
+	}
+
+	return cr.KubernetesConfig.Resources
+}
+
 // RedisLeader interface will have the redis leader configuration
 type RedisLeader struct {
 	common.RedisLeader            `json:",inline"`

diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -0,0 +1,25 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: mutating-webhook-configuration
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-core-v1-pod
+  failurePolicy: Fail
+  name: ot-mutate-pod.opstree.com
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    resources:
+    - pods
+  sideEffects: None
diff --git a/pkg/controllers/rediscluster/rediscluster_controller_test.go b/pkg/controllers/rediscluster/rediscluster_controller_test.go
@@ -42,7 +42,7 @@ var _ = Describe("Redis Cluster Controller", func() {
 		})
 
 		It("should create all required resources", func() {
-			By("verifying the Redis Cluster Leader StatefulSet is created")
+			By("verifying the Redis Cluster StatefulSet is created")
 			leaderSts := &appsv1.StatefulSet{}
 			Eventually(func() error {
 				return k8sClient.Get(context.Background(), types.NamespacedName{
@@ -80,6 +80,7 @@ var _ = Describe("Redis Cluster Controller", func() {
 			Expect(leaderSts.Spec.Template.Spec.SecurityContext).To(Equal(redisCluster.Spec.PodSecurityContext))
 			Expect(leaderSts.Spec.Template.Spec.Containers[0].Image).To(Equal(redisCluster.Spec.KubernetesConfig.Image))
 			Expect(leaderSts.Spec.Template.Spec.Containers[0].ImagePullPolicy).To(Equal(redisCluster.Spec.KubernetesConfig.ImagePullPolicy))
+			Expect(leaderSts.Spec.Template.Spec.Containers[0].Resources).To(Equal(*redisCluster.Spec.GetRedisLeaderResources()))
 
 			By("verifying Service specifications")
 			expectedLabels := map[string]string{
@@ -104,10 +105,10 @@ var _ = Describe("Redis Cluster Controller", func() {
 			By("verifying Redis Cluster configuration")
 			Expect(leaderSts.Spec.ServiceName).To(Equal(redisCluster.Name + "-leader-headless"))
 
-			By("verifying resource requirements")
+			By("verifying resource requirements") // when set resources in redisLeader, it should be used instead of kubernetesConfig.resources
 			container := leaderSts.Spec.Template.Spec.Containers[0]
-			Expect(container.Resources.Limits).To(Equal(redisCluster.Spec.KubernetesConfig.Resources.Limits))
-			Expect(container.Resources.Requests).To(Equal(redisCluster.Spec.KubernetesConfig.Resources.Requests))
+			Expect(container.Resources.Limits).To(Equal(redisCluster.Spec.RedisLeader.Resources.Limits))
+			Expect(container.Resources.Requests).To(Equal(redisCluster.Spec.RedisLeader.Resources.Requests))
 
 			By("verifying Redis Exporter configuration")
 			var exporterContainer *corev1.Container

diff --git a/pkg/controllers/rediscluster/testdata/full.yaml b/pkg/controllers/rediscluster/testdata/full.yaml
@@ -20,6 +20,14 @@ spec:
       limits:
         cpu: 101m
         memory: 128Mi
+  redisLeader:
+    resources:
+      requests:
+        cpu: 102m
+        memory: 129Mi
+      limits:
+        cpu: 102m
+        memory: 129Mi
   redisExporter:
     enabled: true
     image: quay.io/opstree/redis-exporter:v1.44.0

diff --git a/pkg/k8sutils/redis-cluster.go b/pkg/k8sutils/redis-cluster.go
@@ -109,14 +109,14 @@ func generateRedisClusterInitContainerParams(cr *redisv1beta2.RedisCluster) init
 }
 
 // generateRedisClusterContainerParams generates Redis container information
-func generateRedisClusterContainerParams(ctx context.Context, cl kubernetes.Interface, cr *redisv1beta2.RedisCluster, securityContext *corev1.SecurityContext, readinessProbeDef *corev1.Probe, livenessProbeDef *corev1.Probe, role string) containerParameters {
+func generateRedisClusterContainerParams(ctx context.Context, cl kubernetes.Interface, cr *redisv1beta2.RedisCluster, securityContext *corev1.SecurityContext, readinessProbeDef *corev1.Probe, livenessProbeDef *corev1.Probe, role string, resources *corev1.ResourceRequirements) containerParameters {
 	trueProperty := true
 	falseProperty := false
 	containerProp := containerParameters{
 		Role:            "cluster",
 		Image:           cr.Spec.KubernetesConfig.Image,
 		ImagePullPolicy: cr.Spec.KubernetesConfig.ImagePullPolicy,
-		Resources:       cr.Spec.KubernetesConfig.Resources,
+		Resources:       resources,
 		SecurityContext: securityContext,
 		Port:            cr.Spec.Port,
 	}
@@ -218,7 +218,7 @@ func generateRedisClusterContainerParams(ctx context.Context, cl kubernetes.Inte
 func CreateRedisLeader(ctx context.Context, cr *redisv1beta2.RedisCluster, cl kubernetes.Interface) error {
 	prop := RedisClusterSTS{
 		RedisStateFulType:             "leader",
-		Resources:                     cr.Spec.RedisLeader.Resources,
+		Resources:                     cr.Spec.GetRedisLeaderResources(),
 		SecurityContext:               cr.Spec.RedisLeader.SecurityContext,
 		Affinity:                      cr.Spec.RedisLeader.Affinity,
 		TerminationGracePeriodSeconds: cr.Spec.RedisLeader.TerminationGracePeriodSeconds,
@@ -239,7 +239,7 @@ func CreateRedisLeader(ctx context.Context, cr *redisv1beta2.RedisCluster, cl ku
 func CreateRedisFollower(ctx context.Context, cr *redisv1beta2.RedisCluster, cl kubernetes.Interface) error {
 	prop := RedisClusterSTS{
 		RedisStateFulType:             "follower",
-		Resources:                     cr.Spec.RedisFollower.Resources,
+		Resources:                     cr.Spec.GetRedisFollowerResources(),
 		SecurityContext:               cr.Spec.RedisFollower.SecurityContext,
 		Affinity:                      cr.Spec.RedisFollower.Affinity,
 		TerminationGracePeriodSeconds: cr.Spec.RedisFollower.TerminationGracePeriodSeconds,
@@ -288,7 +288,7 @@ func (service RedisClusterSTS) CreateRedisClusterSetup(ctx context.Context, cr *
 		generateRedisClusterParams(ctx, cr, service.getReplicaCount(cr), service.ExternalConfig, service),
 		redisClusterAsOwner(cr),
 		generateRedisClusterInitContainerParams(cr),
-		generateRedisClusterContainerParams(ctx, cl, cr, service.SecurityContext, service.ReadinessProbe, service.LivenessProbe, service.RedisStateFulType),
+		generateRedisClusterContainerParams(ctx, cl, cr, service.SecurityContext, service.ReadinessProbe, service.LivenessProbe, service.RedisStateFulType, service.Resources),
 		cr.Spec.Sidecars,
 	)
 	if err != nil {

diff --git a/pkg/k8sutils/redis-cluster_test.go b/pkg/k8sutils/redis-cluster_test.go
@@ -433,10 +433,10 @@ func Test_generateRedisClusterContainerParams(t *testing.T) {
 		t.Fatalf("Failed to unmarshal file %s: %v", path, err)
 	}
 
-	actualLeaderContainer := generateRedisClusterContainerParams(context.TODO(), fake.NewSimpleClientset(), input, input.Spec.RedisLeader.SecurityContext, input.Spec.RedisLeader.ReadinessProbe, input.Spec.RedisLeader.LivenessProbe, "leader")
+	actualLeaderContainer := generateRedisClusterContainerParams(context.TODO(), fake.NewSimpleClientset(), input, input.Spec.RedisLeader.SecurityContext, input.Spec.RedisLeader.ReadinessProbe, input.Spec.RedisLeader.LivenessProbe, "leader", input.Spec.GetRedisLeaderResources())
 	assert.EqualValues(t, expectedLeaderContainer, actualLeaderContainer, "Expected %+v, got %+v", expectedLeaderContainer, actualLeaderContainer)
 
-	actualFollowerContainer := generateRedisClusterContainerParams(context.TODO(), fake.NewSimpleClientset(), input, input.Spec.RedisFollower.SecurityContext, input.Spec.RedisFollower.ReadinessProbe, input.Spec.RedisFollower.LivenessProbe, "follower")
+	actualFollowerContainer := generateRedisClusterContainerParams(context.TODO(), fake.NewSimpleClientset(), input, input.Spec.RedisFollower.SecurityContext, input.Spec.RedisFollower.ReadinessProbe, input.Spec.RedisFollower.LivenessProbe, "follower", input.Spec.GetRedisFollowerResources())
 	assert.EqualValues(t, expectedFollowerContainer, actualFollowerContainer, "Expected %+v, got %+v", expectedFollowerContainer, actualFollowerContainer)
 }
 

diff --git a/pkg/webhook/pod_webhook.go b/pkg/webhook/pod_webhook.go
@@ -30,7 +30,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
-//+kubebuilder:webhook:path=/mutate-core-v1-pod,mutating=true,failurePolicy=fail,sideEffects=None,groups=core,resources=pods,verbs=create,versions=v1,name=mpod.kb.io,admissionReviewVersions=v1
+//+kubebuilder:webhook:path=/mutate-core-v1-pod,mutating=true,failurePolicy=fail,sideEffects=None,groups=core,resources=pods,verbs=create,versions=v1,name=ot-mutate-pod.opstree.com,admissionReviewVersions=v1
 
 // PodAntiAffiniytMutate mutate Pods
 type PodAntiAffiniytMutate struct {