From 950d78a407d1a1a1c40584fd9724b48bdf61ed66 Mon Sep 17 00:00:00 2001 From: Jwalant Modi Date: Wed, 18 Dec 2024 16:28:59 +0530 Subject: [PATCH] Enhanced AerospikeBackupService CR with deployment configuration params. --- api/v1beta1/aerospikebackupservice_types.go | 29 ++- api/v1beta1/aerospikebackupservice_webhook.go | 84 ++++++- .../controller/backup-service/reconciler.go | 46 +++- test/backup_service/backup_service_test.go | 228 +++++++++++++++++- test/backup_service/test_utils.go | 38 +++ 5 files changed, 403 insertions(+), 22 deletions(-) diff --git a/api/v1beta1/aerospikebackupservice_types.go b/api/v1beta1/aerospikebackupservice_types.go index 18b8dffb..4bd783d2 100644 --- a/api/v1beta1/aerospikebackupservice_types.go +++ b/api/v1beta1/aerospikebackupservice_types.go @@ -55,8 +55,12 @@ type AerospikeBackupServiceSpec struct { // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Backup Service Config" Config runtime.RawExtension `json:"config"` + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Pod Configuration" + ServicePodSpec ServicePodSpec `json:"servicePodSpec,omitempty"` + // Resources defines the requests and limits for the backup service container. // Resources.Limits should be more than Resources.Requests. + // Deprecated: Resources field is now part of serviceContainerSpec // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Resources" Resources *corev1.ResourceRequirements `json:"resources,omitempty"` @@ -82,9 +86,7 @@ type AerospikeBackupServiceStatus struct { // It includes: service, backup-policies, storage, secret-agent. Config runtime.RawExtension `json:"config,omitempty"` - // Resources defines the requests and limits for the backup service container. - // Resources.Limits should be more than Resources.Requests. - Resources *corev1.ResourceRequirements `json:"resources,omitempty"` + ServicePodSpec ServicePodSpec `json:"servicePodSpec,omitempty"` // SecretMounts is the list of secret to be mounted in the backup service. SecretMounts []SecretMount `json:"secrets,omitempty"` @@ -103,6 +105,27 @@ type AerospikeBackupServiceStatus struct { Port int32 `json:"port,omitempty"` } +type ServicePodSpec struct { + // ServiceContainerSpec configures the backup service container + // created by the operator. + ServiceContainerSpec ServiceContainerSpec `json:"serviceContainer,omitempty"` + + ObjectMeta AerospikeObjectMeta `json:"metadata,omitempty"` + + // SchedulingPolicy controls pods placement on Kubernetes nodes. + SchedulingPolicy `json:",inline"` + + ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"` +} + +type ServiceContainerSpec struct { + SecurityContext *corev1.SecurityContext `json:"securityContext,omitempty"` + + // Resources defines the requests and limits for the backup service container. + // Resources.Limits should be more than Resources.Requests. + Resources *corev1.ResourceRequirements `json:"resources,omitempty"` +} + // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:metadata:annotations="aerospike-kubernetes-operator/version=3.4.0" diff --git a/api/v1beta1/aerospikebackupservice_webhook.go b/api/v1beta1/aerospikebackupservice_webhook.go index 18057d82..e11ae933 100644 --- a/api/v1beta1/aerospikebackupservice_webhook.go +++ b/api/v1beta1/aerospikebackupservice_webhook.go @@ -18,6 +18,7 @@ package v1beta1 import ( "fmt" + "reflect" set "github.com/deckarep/golang-set/v2" "k8s.io/apimachinery/pkg/runtime" @@ -28,6 +29,7 @@ import ( "sigs.k8s.io/yaml" "github.com/aerospike/aerospike-backup-service/pkg/model" + asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" ) func (r *AerospikeBackupService) SetupWebhookWithManager(mgr ctrl.Manager) error { @@ -36,7 +38,9 @@ func (r *AerospikeBackupService) SetupWebhookWithManager(mgr ctrl.Manager) error Complete() } -// Implemented Defaulter interface for future reference +//nolint:lll // for readability +//+kubebuilder:webhook:path=/mutate-asdb-aerospike-com-v1beta1-aerospikebackupservice,mutating=true,failurePolicy=fail,sideEffects=None,groups=asdb.aerospike.com,resources=aerospikebackupservices,verbs=create;update,versions=v1beta1,name=maerospikebackupservice.kb.io,admissionReviewVersions=v1 + var _ webhook.Defaulter = &AerospikeBackupService{} // Default implements webhook.Defaulter so a webhook will be registered for the type @@ -44,6 +48,10 @@ func (r *AerospikeBackupService) Default() { absLog := logf.Log.WithName(namespacedName(r)) absLog.Info("Setting defaults for aerospikeBackupService") + + if r.Spec.Resources != nil && r.Spec.ServicePodSpec.ServiceContainerSpec.Resources == nil { + r.Spec.ServicePodSpec.ServiceContainerSpec.Resources = r.Spec.Resources + } } //nolint:lll // for readability @@ -65,11 +73,19 @@ func (r *AerospikeBackupService) ValidateCreate() (admission.Warnings, error) { return nil, err } + warn, err := r.validateServicePodSpec() + + if err != nil { + return nil, err + } else if warn != nil { + return warn, nil + } + return nil, nil } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type -func (r *AerospikeBackupService) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) { +func (r *AerospikeBackupService) ValidateUpdate(oldObj runtime.Object) (admission.Warnings, error) { absLog := logf.Log.WithName(namespacedName(r)) absLog.Info("Validate update") @@ -82,6 +98,14 @@ func (r *AerospikeBackupService) ValidateUpdate(_ runtime.Object) (admission.War return nil, err } + warn, err := r.validateServicePodSpec() + + if err != nil { + return nil, err + } else if warn != nil { + return warn, nil + } + return nil, nil } @@ -139,3 +163,59 @@ func (r *AerospikeBackupService) validateBackupServiceSecrets() error { return nil } + +func (r *AerospikeBackupService) validateServicePodSpec() (admission.Warnings, error) { + warn, err := r.validateResourcesField() + if warn != nil { + return warn, nil + } else if err != nil { + return nil, err + } + + if err := r.validateResourceLimitsAndRequests(); err != nil { + return nil, err + } + + if err := validateObjectMeta(&r.Spec.ServicePodSpec.ObjectMeta); err != nil { + return nil, err + } + + return nil, nil +} +func (r *AerospikeBackupService) validateResourcesField() (admission.Warnings, error) { + if r.Spec.Resources != nil && r.Spec.ServicePodSpec.ServiceContainerSpec.Resources != nil { + if !reflect.DeepEqual(r.Spec.Resources, r.Spec.ServicePodSpec.ServiceContainerSpec.Resources) { + return nil, fmt.Errorf("resources mismatched, different resources requirements shouldn't be allowed") + } else { + return []string{"resources field in spec is deprecated, " + + "resources field is now part of servicePodSpec.serviceContainer"}, nil + } + } + + return nil, nil +} +func (r *AerospikeBackupService) validateResourceLimitsAndRequests() error { + if r.Spec.ServicePodSpec.ServiceContainerSpec.Resources != nil { + resources := r.Spec.ServicePodSpec.ServiceContainerSpec.Resources + if resources.Limits != nil && resources.Requests != nil && + ((resources.Limits.Cpu().Cmp(*resources.Requests.Cpu()) < 0) || + (resources.Limits.Memory().Cmp(*resources.Requests.Memory()) < 0)) { + return fmt.Errorf("resources.Limits cannot be less than resource.Requests. Resources %v", + resources) + } + } + + return nil +} +func validateObjectMeta(objectMeta *AerospikeObjectMeta) error { + for label := range objectMeta.Labels { + if label == asdbv1.AerospikeAppLabel || label == asdbv1.AerospikeCustomResourceLabel { + return fmt.Errorf( + "label: %s is automatically defined by operator and shouldn't be specified by user", + label, + ) + } + } + + return nil +} diff --git a/internal/controller/backup-service/reconciler.go b/internal/controller/backup-service/reconciler.go index 258ad2ec..9a893c4a 100644 --- a/internal/controller/backup-service/reconciler.go +++ b/internal/controller/backup-service/reconciler.go @@ -25,6 +25,7 @@ import ( asdbv1beta1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1beta1" "github.com/aerospike/aerospike-kubernetes-operator/internal/controller/common" "github.com/aerospike/aerospike-kubernetes-operator/pkg/utils" + lib "github.com/aerospike/aerospike-management-lib" ) type serviceConfig struct { @@ -314,12 +315,6 @@ func (r *SingleBackupServiceReconciler) getDeploymentObject() (*app.Deployment, svcLabels := utils.LabelsForAerospikeBackupService(r.aeroBackupService.Name) volumeMounts, volumes := r.getVolumeAndMounts() - resources := corev1.ResourceRequirements{} - - if r.aeroBackupService.Spec.Resources != nil { - resources = *r.aeroBackupService.Spec.Resources - } - svcConf, err := r.getBackupServiceConfig() if err != nil { return nil, err @@ -358,7 +353,6 @@ func (r *SingleBackupServiceReconciler) getDeploymentObject() (*app.Deployment, Image: r.aeroBackupService.Spec.Image, ImagePullPolicy: corev1.PullIfNotPresent, VolumeMounts: volumeMounts, - Resources: resources, Ports: containerPorts, }, }, @@ -392,9 +386,44 @@ func (r *SingleBackupServiceReconciler) getDeploymentObject() (*app.Deployment, }, } + r.updateDeploymentFromPodSpec(deploy) + return deploy, nil } +func (r *SingleBackupServiceReconciler) updateDeploymentFromPodSpec(deploy *app.Deployment) { + r.updateDeploymentSchedulingPolicy(deploy) + + defaultLabels := utils.LabelsForAerospikeBackupService(r.aeroBackupService.Name) + userDefinedLabels := r.aeroBackupService.Spec.ServicePodSpec.ObjectMeta.Labels + mergedLabels := utils.MergeLabels(defaultLabels, userDefinedLabels) + deploy.Spec.Template.ObjectMeta.Labels = mergedLabels + + deploy.Spec.Template.ObjectMeta.Annotations = r.aeroBackupService.Spec.ServicePodSpec.ObjectMeta.Annotations + + deploy.Spec.Template.Spec.ImagePullSecrets = r.aeroBackupService.Spec.ServicePodSpec.ImagePullSecrets + + r.updateBackupServiceContainer(deploy) +} + +func (r *SingleBackupServiceReconciler) updateDeploymentSchedulingPolicy(deploy *app.Deployment) { + deploy.Spec.Template.Spec.Affinity = r.aeroBackupService.Spec.ServicePodSpec.Affinity + deploy.Spec.Template.Spec.NodeSelector = r.aeroBackupService.Spec.ServicePodSpec.NodeSelector + deploy.Spec.Template.Spec.Tolerations = r.aeroBackupService.Spec.ServicePodSpec.Tolerations +} + +func (r *SingleBackupServiceReconciler) updateBackupServiceContainer(deploy *app.Deployment) { + resources := r.aeroBackupService.Spec.ServicePodSpec.ServiceContainerSpec.Resources + if resources != nil { + deploy.Spec.Template.Spec.Containers[0].Resources = *resources + } else { + deploy.Spec.Template.Spec.Containers[0].Resources = corev1.ResourceRequirements{} + } + + deploy.Spec.Template.Spec.Containers[0].SecurityContext = + r.aeroBackupService.Spec.ServicePodSpec.ServiceContainerSpec.SecurityContext +} + func (r *SingleBackupServiceReconciler) getVolumeAndMounts() ([]corev1.VolumeMount, []corev1.Volume) { volumes := make([]corev1.Volume, 0, len(r.aeroBackupService.Spec.SecretMounts)) volumeMounts := make([]corev1.VolumeMount, 0, len(r.aeroBackupService.Spec.SecretMounts)) @@ -674,7 +703,8 @@ func (r *SingleBackupServiceReconciler) CopySpecToStatus() *asdbv1beta1.Aerospik status := asdbv1beta1.AerospikeBackupServiceStatus{} status.Image = r.aeroBackupService.Spec.Image status.Config = r.aeroBackupService.Spec.Config - status.Resources = r.aeroBackupService.Spec.Resources + statusServicePodSpec := lib.DeepCopy(r.aeroBackupService.Spec.ServicePodSpec).(asdbv1beta1.ServicePodSpec) + status.ServicePodSpec = statusServicePodSpec status.SecretMounts = r.aeroBackupService.Spec.SecretMounts status.Service = r.aeroBackupService.Spec.Service diff --git a/test/backup_service/backup_service_test.go b/test/backup_service/backup_service_test.go index 3719986d..3c9acfb2 100644 --- a/test/backup_service/backup_service_test.go +++ b/test/backup_service/backup_service_test.go @@ -1,15 +1,20 @@ package backupservice import ( + "context" "encoding/json" + "fmt" "net/http" + "reflect" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + "sigs.k8s.io/controller-runtime/pkg/client" + asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" asdbv1beta1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1beta1" "github.com/aerospike/aerospike-kubernetes-operator/internal/controller/common" ) @@ -122,6 +127,41 @@ var _ = Describe( Expect(err.Error()).To( ContainSubstring("backup-routines field cannot be specified in backup service config")) }) + + It("Should fail adding reserved label", func() { + backupService, err = NewBackupService() + Expect(err).ToNot(HaveOccurred()) + backupService.Spec.ServicePodSpec.ObjectMeta.Labels = map[string]string{ + asdbv1.AerospikeAppLabel: "test", + } + err = DeployBackupService(k8sClient, backupService) + Expect(err).Should(HaveOccurred()) + }) + + It("Should fail for request exceeding limit", func() { + backupService, err = NewBackupService() + Expect(err).ToNot(HaveOccurred()) + + resourceMem := resource.MustParse("3Gi") + resourceCPU := resource.MustParse("250m") + limitMem := resource.MustParse("2Gi") + limitCPU := resource.MustParse("200m") + + resources := &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resourceCPU, + corev1.ResourceMemory: resourceMem, + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: limitCPU, + corev1.ResourceMemory: limitMem, + }, + } + + backupService.Spec.ServicePodSpec.ServiceContainerSpec.Resources = resources + err = DeployBackupService(k8sClient, backupService) + Expect(err).Should(HaveOccurred()) + }) }, ) @@ -178,9 +218,9 @@ var _ = Describe( Expect(err).ToNot(HaveOccurred()) // Change Pod spec - backupService.Spec.Resources = &corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("0.5"), + backupService.Spec.ServicePodSpec = asdbv1beta1.ServicePodSpec{ + ObjectMeta: asdbv1beta1.AerospikeObjectMeta{ + Labels: map[string]string{"label-test-1": "test-1"}, }, } @@ -197,11 +237,11 @@ var _ = Describe( It("Should change K8s service type when service type is changed in CR", func() { backupService, err = NewBackupService() Expect(err).ToNot(HaveOccurred()) - err := DeployBackupService(k8sClient, backupService) + err = DeployBackupService(k8sClient, backupService) Expect(err).ToNot(HaveOccurred()) - svc, err := getBackupK8sServiceObj(k8sClient, name, namespace) - Expect(err).ToNot(HaveOccurred()) + svc, sErr := getBackupK8sServiceObj(k8sClient, name, namespace) + Expect(sErr).ToNot(HaveOccurred()) Expect(svc.Spec.Type).To(Equal(corev1.ServiceTypeClusterIP)) // Get backup service object @@ -228,9 +268,9 @@ var _ = Describe( // Check backup service health using LB IP Eventually(func() bool { - resp, err := http.Get("http://" + svc.Status.LoadBalancer.Ingress[0].IP + ":8081/health") - if err != nil { - pkgLog.Error(err, "Failed to get health") + resp, gErr := http.Get("http://" + svc.Status.LoadBalancer.Ingress[0].IP + ":8081/health") + if gErr != nil { + pkgLog.Error(gErr, "Failed to get health") return false } @@ -241,6 +281,176 @@ var _ = Describe( }) + It("Should validate annotations and labels addition", func() { + backupService, err = NewBackupService() + Expect(err).ToNot(HaveOccurred()) + backupService.Spec.ServicePodSpec.ObjectMeta.Labels = map[string]string{"label-test-1": "test-1"} + backupService.Spec.ServicePodSpec.ObjectMeta.Annotations = map[string]string{"annotation-test-1": "test-1"} + err = DeployBackupService(k8sClient, backupService) + Expect(err).ToNot(HaveOccurred()) + + backupService, err = getBackupServiceObj(k8sClient, name, namespace) + Expect(err).ToNot(HaveOccurred()) + deploy, dErr := getBackupServiceDeployment(k8sClient, backupService) + Expect(dErr).ToNot(HaveOccurred()) + + By("Validating Annotations") + actual := deploy.Spec.Template.ObjectMeta.Annotations + valid := validateLabelsOrAnnotations(actual, map[string]string{"annotation-test-1": "test-1"}) + Expect(valid).To( + BeTrue(), "Unable to find annotations", + ) + + By("Validating Labels") + actual = deploy.Spec.Template.ObjectMeta.Labels + valid = validateLabelsOrAnnotations(actual, map[string]string{"label-test-1": "test-1"}) + Expect(valid).To( + BeTrue(), "Unable to find labels", + ) + }) + + It("Should validate SchedulingPolicy", func() { + backupService, err = NewBackupService() + Expect(err).ToNot(HaveOccurred()) + + nodeList, nErr := getNodeList(context.TODO(), k8sClient) + Expect(nErr).ToNot(HaveOccurred()) + Expect(len(nodeList.Items)).ToNot(BeZero()) + + By("Validating Affinity") + nodeName := nodeList.Items[0].Name + affinity := &corev1.Affinity{} + ns := &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ + { + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "kubernetes.io/hostname", + Operator: corev1.NodeSelectorOpIn, + Values: []string{nodeName}, + }, + }, + }, + }, + } + affinity.NodeAffinity = &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: ns, + } + backupService.Spec.ServicePodSpec.Affinity = affinity + err = DeployBackupService(k8sClient, backupService) + Expect(err).ToNot(HaveOccurred()) + + backupService, err = getBackupServiceObj(k8sClient, name, namespace) + Expect(err).ToNot(HaveOccurred()) + podList, gErr := getBackupServicePodList(k8sClient, backupService) + Expect(gErr).ToNot(HaveOccurred()) + Expect(len(podList.Items)).To(Equal(1)) + Expect(podList.Items[0].Spec.NodeName).Should(Equal(nodeName)) + }) + + It("Should validate container resources and securitycontext", func() { + backupService, err = NewBackupService() + Expect(err).ToNot(HaveOccurred()) + err = DeployBackupService(k8sClient, backupService) + Expect(err).ToNot(HaveOccurred()) + + resourceMem := resource.MustParse("1Gi") + resourceCPU := resource.MustParse("200m") + limitMem := resource.MustParse("2Gi") + limitCPU := resource.MustParse("300m") + + res := &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resourceCPU, + corev1.ResourceMemory: resourceMem, + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: limitCPU, + corev1.ResourceMemory: limitMem, + }, + } + + sc := &corev1.SecurityContext{Privileged: new(bool)} + + By("Validating Resources") + updateBackupServiceResources(k8sClient, res) + + By("Remove Resources") + updateBackupServiceResources(k8sClient, nil) + + By("Validating SecurityContext") + updateBackupServiceSecurityContext(k8sClient, sc) + + By("Remove SecurityContext") + updateBackupServiceSecurityContext(k8sClient, nil) + }) }) }, ) + +func updateBackupServiceResources(k8sClient client.Client, res *corev1.ResourceRequirements) { + backupService, err := getBackupServiceObj(k8sClient, name, namespace) + Expect(err).ToNot(HaveOccurred()) + + backupService.Spec.ServicePodSpec.ServiceContainerSpec.Resources = res + err = updateBackupService(k8sClient, backupService) + Expect(err).ToNot(HaveOccurred()) + + err = validateBackupServiceResources(k8sClient, res) + Expect(err).ToNot(HaveOccurred()) +} + +func validateBackupServiceResources(k8sClient client.Client, res *corev1.ResourceRequirements) error { + backupService, err := getBackupServiceObj(k8sClient, name, namespace) + if err != nil { + return err + } + + deploy, err := getBackupServiceDeployment(k8sClient, backupService) + if err != nil { + return err + } + + // res can not be null in deploy spec + if res == nil { + res = &corev1.ResourceRequirements{} + } + + actual := deploy.Spec.Template.Spec.Containers[0].Resources + if !reflect.DeepEqual(&actual, res) { + return fmt.Errorf("resource not matching. want %v, got %v", *res, actual) + } + + return nil +} + +func updateBackupServiceSecurityContext(k8sClient client.Client, sc *corev1.SecurityContext) { + backupService, err := getBackupServiceObj(k8sClient, name, namespace) + Expect(err).ToNot(HaveOccurred()) + + backupService.Spec.ServicePodSpec.ServiceContainerSpec.SecurityContext = sc + err = updateBackupService(k8sClient, backupService) + Expect(err).ToNot(HaveOccurred()) + + err = validateBackupServiceSecurityContext(k8sClient, sc) + Expect(err).ToNot(HaveOccurred()) +} + +func validateBackupServiceSecurityContext(k8sClient client.Client, sc *corev1.SecurityContext) error { + backupService, err := getBackupServiceObj(k8sClient, name, namespace) + if err != nil { + return err + } + + deploy, err := getBackupServiceDeployment(k8sClient, backupService) + if err != nil { + return err + } + + actual := deploy.Spec.Template.Spec.Containers[0].SecurityContext + if !reflect.DeepEqual(actual, sc) { + return fmt.Errorf("security context not matching") + } + + return nil +} diff --git a/test/backup_service/test_utils.go b/test/backup_service/test_utils.go index a4a75884..61419c02 100644 --- a/test/backup_service/test_utils.go +++ b/test/backup_service/test_utils.go @@ -298,3 +298,41 @@ func DeleteBackupService( return nil } + +func getBackupServiceDeployment(k8sClient client.Client, + backupService *asdbv1beta1.AerospikeBackupService) (*app.Deployment, error) { + deployment := &app.Deployment{} + if err := k8sClient.Get(context.TODO(), types.NamespacedName{ + Namespace: backupService.Namespace, + Name: backupService.Name, + }, deployment); err != nil { + return nil, err + } + + return deployment, nil +} + +func validateLabelsOrAnnotations( + actual map[string]string, expected map[string]string, +) bool { + for key, val := range expected { + if v, ok := actual[key]; ok { + if v == val { + return true + } + } + } + + return false +} + +func getNodeList(ctx context.Context, k8sClient client.Client) ( + *corev1.NodeList, error, +) { + nodeList := &corev1.NodeList{} + if err := k8sClient.List(ctx, nodeList); err != nil { + return nil, err + } + + return nodeList, nil +}