From fcbacf6faf23fb0bb282a8efd5846149bf8ad696 Mon Sep 17 00:00:00 2001 From: drivebyer Date: Sun, 5 Jan 2025 18:22:22 +0800 Subject: [PATCH] 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 --- api/v1beta2/rediscluster_types.go | 33 +++++++++++++++---- ...is.redis.opstreelabs.in_redisclusters.yaml | 3 +- .../rediscluster_controller_test.go | 9 ++--- .../rediscluster/testdata/full.yaml | 8 +++++ pkg/k8sutils/redis-cluster.go | 10 +++--- pkg/k8sutils/redis-cluster_test.go | 4 +-- 6 files changed, 48 insertions(+), 19 deletions(-) diff --git a/api/v1beta2/rediscluster_types.go b/api/v1beta2/rediscluster_types.go index 61349a102..2b0595d71 100644 --- a/api/v1beta2/rediscluster_types.go +++ b/api/v1beta2/rediscluster_types.go @@ -31,13 +31,14 @@ type RedisClusterSpec struct { // +kubebuilder:default:=6379 Port *int `json:"port,omitempty"` // +kubebuilder:default:=v7 - ClusterVersion *string `json:"clusterVersion,omitempty"` - RedisLeader RedisLeader `json:"redisLeader,omitempty"` - RedisFollower RedisFollower `json:"redisFollower,omitempty"` - RedisExporter *RedisExporter `json:"redisExporter,omitempty"` - Storage *ClusterStorage `json:"storage,omitempty"` - PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"` - PriorityClassName string `json:"priorityClassName,omitempty"` + ClusterVersion *string `json:"clusterVersion,omitempty"` + RedisLeader RedisLeader `json:"redisLeader,omitempty"` + RedisFollower RedisFollower `json:"redisFollower,omitempty"` + RedisExporter *RedisExporter `json:"redisExporter,omitempty"` + Storage *ClusterStorage `json:"storage,omitempty"` + PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"` + PriorityClassName string `json:"priorityClassName,omitempty"` + // Deprecated: use kubernetesConfig.Resources instead for both leader and follower Resources *corev1.ResourceRequirements `json:"resources,omitempty"` TLS *TLSConfig `json:"TLS,omitempty"` ACL *ACLConfig `json:"acl,omitempty"` @@ -58,6 +59,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/crd/bases/redis.redis.opstreelabs.in_redisclusters.yaml b/config/crd/bases/redis.redis.opstreelabs.in_redisclusters.yaml index 0e62a3be2..ce9d29465 100644 --- a/config/crd/bases/redis.redis.opstreelabs.in_redisclusters.yaml +++ b/config/crd/bases/redis.redis.opstreelabs.in_redisclusters.yaml @@ -10851,7 +10851,8 @@ spec: type: array type: object resources: - description: ResourceRequirements describes the compute resource requirements. + description: 'Deprecated: use kubernetesConfig.Resources instead for + both leader and follower' properties: claims: description: |- diff --git a/pkg/controllers/rediscluster/rediscluster_controller_test.go b/pkg/controllers/rediscluster/rediscluster_controller_test.go index ee683a32d..46cfc19cb 100644 --- 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 index cd888ab8f..631dee7f3 100644 --- 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 index d60f214ac..dfca804e6 100644 --- 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 index b8881fee9..de16d23f6 100644 --- 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) }