From 9ef895e25a12c7423485fe2a2e23a9eed9a4a283 Mon Sep 17 00:00:00 2001 From: Shubham Dilip Pampattiwar Date: Tue, 3 Aug 2021 15:00:12 -0400 Subject: [PATCH 1/6] add registry controller --- oadp-operator/controllers/registry.go | 356 ++++++++++++++++++ oadp-operator/controllers/velero.go | 11 +- .../controllers/velero_controller.go | 1 + 3 files changed, 364 insertions(+), 4 deletions(-) create mode 100644 oadp-operator/controllers/registry.go diff --git a/oadp-operator/controllers/registry.go b/oadp-operator/controllers/registry.go new file mode 100644 index 00000000000..8383334edcf --- /dev/null +++ b/oadp-operator/controllers/registry.go @@ -0,0 +1,356 @@ +package controllers + +import ( + "fmt" + "github.com/go-logr/logr" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/pointer" + + velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +// Registry Env var keys +const ( + // AWS registry env vars + RegistryStorageEnvVarKey = "REGISTRY_STORAGE" + RegistryStorageS3AccesskeyEnvVarKey = "REGISTRY_STORAGE_S3_ACCESSKEY" + RegistryStorageS3BucketEnvVarKey = "REGISTRY_STORAGE_S3_BUCKET" + RegistryStorageS3RegionEnvVarKey = "REGISTRY_STORAGE_S3_REGION" + RegistryStorageS3SecretkeyEnvVarKey = "REGISTRY_STORAGE_S3_SECRETKEY" + RegistryStorageS3RegionendpointEnvVarKey = "REGISTRY_STORAGE_S3_REGIONENDPOINT" + RegistryStorageS3RootdirectoryEnvVarKey = "REGISTRY_STORAGE_S3_ROOTDIRECTORY" + RegistryStorageS3SkipverifyEnvVarKey = "REGISTRY_STORAGE_S3_SKIPVERIFY" + // Azure registry env vars + RegistryStorageAzureContainerEnvVarKey = "REGISTRY_STORAGE_AZURE_CONTAINER" + RegistryStorageAzureAccountnameEnvVarKey = "REGISTRY_STORAGE_AZURE_ACCOUNTNAME" + RegistryStorageAzureAccountkeyEnvVarKey = "REGISTRY_STORAGE_AZURE_ACCOUNTKEY" + // GCP registry env vars + RegistryStorageGCSBucket = "REGISTRY_STORAGE_GCS_BUCKET" + RegistryStorageGCSKeyfile = "REGISTRY_STORAGE_GCS_KEYFILE" + RegistryStorageGCSRootdirectory = "REGISTRY_STORAGE_GCS_ROOTDIRECTORY" +) + +// provider specific object storage +const ( + S3 = "s3" + Azure = "azure" + GCS = "gcs" + AWSProvider = "aws" + AzureProvider = "azure" + GCPProvider = "gcp" + Region = "region" + S3URL = "s3Url" + RootDirectory = "rootDirectory" + InsecureSkipTLSVerify = "insecureSkipTLSVerify" + StorageAccount = "storageAccount" +) + +// creating skeleton for provider based env var map +var cloudProviderEnvVarMap = map[string][]corev1.EnvVar{ + "aws": { + { + Name: RegistryStorageEnvVarKey, + Value: S3, + }, + { + Name: RegistryStorageS3AccesskeyEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageS3BucketEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageS3RegionEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageS3SecretkeyEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageS3RegionendpointEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageS3RootdirectoryEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageS3SkipverifyEnvVarKey, + Value: "", + }, + }, + "azure": { + { + Name: RegistryStorageEnvVarKey, + Value: Azure, + }, + { + Name: RegistryStorageAzureContainerEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageAzureAccountnameEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageAzureAccountkeyEnvVarKey, + Value: "", + }, + }, + "gcp": { + { + Name: RegistryStorageEnvVarKey, + Value: GCS, + }, + { + Name: RegistryStorageGCSBucket, + Value: "", + }, + { + Name: RegistryStorageGCSKeyfile, + Value: "", + }, + { + Name: RegistryStorageGCSRootdirectory, + Value: "", + }, + }, +} + +func (r *VeleroReconciler) ReconcileRegistries(log logr.Logger) (bool, error) { + + bslLabels := map[string]string{ + "app.kubernetes.io/name": "oadp-operator-velero", + "app.kubernetes.io/managed-by": "oadp-operator", + "app.kubernetes.io/component": "bsl", + } + bslListOptions := client.MatchingLabels(bslLabels) + backupStorageLocationList := velerov1.BackupStorageLocationList{} + + // Fetch the configured backupstoragelocations + if err := r.List(r.Context, &backupStorageLocationList, bslListOptions); err != nil { + return false, err + } + + // Loop through all the configured BSLs and create registry for each of them + for _, bsl := range backupStorageLocationList.Items { + registryDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: constructBSLRegistryName(&bsl), + Namespace: VeleoNamespace, + }, + } + + op, err := controllerutil.CreateOrUpdate(r.Context, r.Client, registryDeployment, func() error { + + // Setting Registry Deployment selector if a new object is created as it is immutable + if registryDeployment.ObjectMeta.CreationTimestamp.IsZero() { + registryDeployment.Spec.Selector = &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "component": constructBSLRegistryName(&bsl), + }, + } + } + + // Setting controller owner reference on the registry deployment + err := controllerutil.SetControllerReference(&bsl, registryDeployment, r.Scheme) + if err != nil { + return err + } + + // update the Registry Deployment template + registryDeployment = r.buildRegistryDeployment(registryDeployment, &bsl) + return nil + }) + + if err != nil { + return false, err + } + + //TODO: Review registry deployment status and report errors and conditions + + if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + // Trigger event to indicate registry deployment was created or updated + r.EventRecorder.Event(registryDeployment, + corev1.EventTypeNormal, + "RegistryDeploymentReconciled", + fmt.Sprintf("performed %s on registry deployment %s/%s", op, registryDeployment.Namespace, registryDeployment.Name), + ) + } + + } + + return true, nil +} + +// Construct and update the registry deployment for a bsl +func (r *VeleroReconciler) buildRegistryDeployment(registryDeployment *appsv1.Deployment, bsl *velerov1.BackupStorageLocation) *appsv1.Deployment { + registryDeployment.Labels = r.getBSLLabels(bsl) + + registryDeployment.Spec = appsv1.DeploymentSpec{ + Replicas: pointer.Int32(1), + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "component": constructBSLRegistryName(bsl), + }, + }, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyAlways, + Containers: r.buildRegistryContainer(bsl), + }, + }, + } + return registryDeployment +} + +func (r *VeleroReconciler) getBSLLabels(bsl *velerov1.BackupStorageLocation) map[string]string { + labels := map[string]string{ + "app.kubernetes.io/name": BackupStorageLocation, + "app.kubernetes.io/instance": constructBSLRegistryName(bsl), + "app.kubernetes.io/managed-by": OADPOperator, + "app.kubernetes.io/component": Registry, + } + return labels +} + +func constructBSLRegistryName(bsl *velerov1.BackupStorageLocation) string { + return "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry" +} + +func (r *VeleroReconciler) buildRegistryContainer(bsl *velerov1.BackupStorageLocation) []corev1.Container { + containers := []corev1.Container{ + { + Image: RegistryImage, + Name: constructBSLRegistryName(bsl) + "-container", + Ports: []corev1.ContainerPort{ + { + ContainerPort: 5000, + Protocol: corev1.ProtocolTCP, + }, + }, + Env: getRegistryEnvVars(bsl), + LivenessProbe: &corev1.Probe{ + Handler: corev1.Handler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/v2/_catalog?n=5", + Port: intstr.IntOrString{IntVal: 5000}, + }, + }, + PeriodSeconds: 5, + TimeoutSeconds: 3, + InitialDelaySeconds: 15, + }, + ReadinessProbe: &corev1.Probe{ + Handler: corev1.Handler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/v2/_catalog?n=5", + Port: intstr.IntOrString{IntVal: 5000}, + }, + }, + PeriodSeconds: 5, + TimeoutSeconds: 3, + InitialDelaySeconds: 15, + }, + }, + } + + return containers +} + +func getRegistryEnvVars(bsl *velerov1.BackupStorageLocation) []corev1.EnvVar { + envVar := []corev1.EnvVar{} + if bsl.Spec.Provider == AWSProvider { + envVar = buildRegistryEnvVars(bsl, cloudProviderEnvVarMap[AWSProvider], AWSProvider) + } + + if bsl.Spec.Provider == AzureProvider { + envVar = buildRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider], AzureProvider) + } + + if bsl.Spec.Provider == GCPProvider { + envVar = buildRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider], GCPProvider) + } + + return envVar +} + +func buildRegistryEnvVars(bsl *velerov1.BackupStorageLocation, providerEnvVar []corev1.EnvVar, provider string) []corev1.EnvVar { + + updatedEnvVar := providerEnvVar + // build AWS provider env vars + if provider == AWSProvider { + for _, key := range updatedEnvVar { + //TODO: This needs to be fetch from the provider secret + if key.Name == RegistryStorageS3AccesskeyEnvVarKey { + key.Value = "" + } + + if key.Name == RegistryStorageS3BucketEnvVarKey { + key.Value = bsl.Spec.StorageType.ObjectStorage.Bucket + } + + if key.Name == RegistryStorageS3RegionEnvVarKey { + key.Value = bsl.Spec.Config[Region] + } + //TODO: This needs to be fetch from the provider secret + if key.Name == RegistryStorageS3SecretkeyEnvVarKey { + key.Value = "" + } + + if key.Name == RegistryStorageS3RegionendpointEnvVarKey && bsl.Spec.Config[S3URL] != "" { + key.Value = bsl.Spec.Config[S3URL] + } + + if key.Name == RegistryStorageS3RootdirectoryEnvVarKey && bsl.Spec.Config[RootDirectory] != "" { + key.Value = bsl.Spec.Config[RootDirectory] + } + + if key.Name == RegistryStorageS3SkipverifyEnvVarKey && bsl.Spec.Config[InsecureSkipTLSVerify] != "" { + key.Value = bsl.Spec.Config[InsecureSkipTLSVerify] + } + } + } + + // build Azure env vars + if provider == AzureProvider { + for _, key := range updatedEnvVar { + if key.Name == RegistryStorageAzureContainerEnvVarKey { + key.Value = bsl.Spec.StorageType.ObjectStorage.Bucket + } + + if key.Name == RegistryStorageAzureAccountnameEnvVarKey { + key.Value = bsl.Spec.Config[StorageAccount] + } + //TODO: This needs to be fetch from the provider secret + if key.Name == RegistryStorageAzureAccountkeyEnvVarKey { + key.Value = "" + } + } + } + + // build GCP env vars + if provider == GCPProvider { + for _, key := range updatedEnvVar { + if key.Name == RegistryStorageGCSBucket { + key.Value = bsl.Spec.StorageType.ObjectStorage.Bucket + } + //TODO: This needs to be fetch from the provider secret + if key.Name == RegistryStorageGCSKeyfile { + key.Value = "" + } + if key.Name == RegistryStorageGCSRootdirectory && bsl.Spec.Config[RootDirectory] != "" { + key.Value = bsl.Spec.Config[RootDirectory] + } + } + } + + return updatedEnvVar +} diff --git a/oadp-operator/controllers/velero.go b/oadp-operator/controllers/velero.go index e98e0573345..113a799602e 100644 --- a/oadp-operator/controllers/velero.go +++ b/oadp-operator/controllers/velero.go @@ -14,10 +14,12 @@ import ( ) const ( - Velero = "velero" - VeleoNamespace = "oadp-operator" - OADPOperator = "oadp-operator" - Server = "server" + Velero = "velero" + VeleoNamespace = "oadp-operator" + OADPOperator = "oadp-operator" + Server = "server" + Registry = "Registry" + BackupStorageLocation = "BackupStorageLocation" //TODO: Check for default secret names VeleroAWSSecretName = "cloud-credentials" VeleroAzureSecretName = "cloud-credentials-azure" @@ -47,6 +49,7 @@ const ( AzurePluginImage = "quay.io/konveyor/velero-plugin-for-microsoft-azure:latest" GCPPluginImage = "quay.io/konveyor/velero-plugin-for-gcp:latest" CSIPluginImage = "quay.io/konveyor/velero-plugin-for-csi:latest" + RegistryImage = "quay.io/konveyor/registry:latest" ) // Plugin names diff --git a/oadp-operator/controllers/velero_controller.go b/oadp-operator/controllers/velero_controller.go index 5ce410d5686..8768d17b891 100644 --- a/oadp-operator/controllers/velero_controller.go +++ b/oadp-operator/controllers/velero_controller.go @@ -76,6 +76,7 @@ func (r *VeleroReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr _, err := ReconcileBatch(r.Log, r.ValidateBackupStorageLocations, r.ReconcileBackupStorageLocations, + r.ReconcileRegistries, r.ValidateVolumeSnapshotLocations, r.ReconcileVolumeSnapshotLocations, r.ReconcileResticDaemonset, From 93317aa9a41f5576043f170d4bb0991114465b3b Mon Sep 17 00:00:00 2001 From: Shubham Dilip Pampattiwar Date: Wed, 4 Aug 2021 15:05:37 -0400 Subject: [PATCH 2/6] rework getting provider env var --- oadp-operator/controllers/registry.go | 121 +++++++++++++------------- oadp-operator/controllers/velero.go | 12 +-- 2 files changed, 65 insertions(+), 68 deletions(-) diff --git a/oadp-operator/controllers/registry.go b/oadp-operator/controllers/registry.go index 8383334edcf..66fce5aaed6 100644 --- a/oadp-operator/controllers/registry.go +++ b/oadp-operator/controllers/registry.go @@ -192,7 +192,7 @@ func (r *VeleroReconciler) ReconcileRegistries(log logr.Logger) (bool, error) { // Construct and update the registry deployment for a bsl func (r *VeleroReconciler) buildRegistryDeployment(registryDeployment *appsv1.Deployment, bsl *velerov1.BackupStorageLocation) *appsv1.Deployment { - registryDeployment.Labels = r.getBSLLabels(bsl) + registryDeployment.Labels = r.getRegistryBSLLabels(bsl) registryDeployment.Spec = appsv1.DeploymentSpec{ Replicas: pointer.Int32(1), @@ -211,9 +211,9 @@ func (r *VeleroReconciler) buildRegistryDeployment(registryDeployment *appsv1.De return registryDeployment } -func (r *VeleroReconciler) getBSLLabels(bsl *velerov1.BackupStorageLocation) map[string]string { +func (r *VeleroReconciler) getRegistryBSLLabels(bsl *velerov1.BackupStorageLocation) map[string]string { labels := map[string]string{ - "app.kubernetes.io/name": BackupStorageLocation, + "app.kubernetes.io/name": OADPOperatorVelero, "app.kubernetes.io/instance": constructBSLRegistryName(bsl), "app.kubernetes.io/managed-by": OADPOperator, "app.kubernetes.io/component": Registry, @@ -268,89 +268,86 @@ func (r *VeleroReconciler) buildRegistryContainer(bsl *velerov1.BackupStorageLoc func getRegistryEnvVars(bsl *velerov1.BackupStorageLocation) []corev1.EnvVar { envVar := []corev1.EnvVar{} if bsl.Spec.Provider == AWSProvider { - envVar = buildRegistryEnvVars(bsl, cloudProviderEnvVarMap[AWSProvider], AWSProvider) + envVar = getAWSRegistryEnvVars(bsl, cloudProviderEnvVarMap[AWSProvider]) } if bsl.Spec.Provider == AzureProvider { - envVar = buildRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider], AzureProvider) + envVar = getAzureRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider]) } if bsl.Spec.Provider == GCPProvider { - envVar = buildRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider], GCPProvider) + envVar = getGCPRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider]) } return envVar } -func buildRegistryEnvVars(bsl *velerov1.BackupStorageLocation, providerEnvVar []corev1.EnvVar, provider string) []corev1.EnvVar { +func getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation, awsEnvVars []corev1.EnvVar) []corev1.EnvVar { - updatedEnvVar := providerEnvVar - // build AWS provider env vars - if provider == AWSProvider { - for _, key := range updatedEnvVar { - //TODO: This needs to be fetch from the provider secret - if key.Name == RegistryStorageS3AccesskeyEnvVarKey { - key.Value = "" - } + for i, _ := range awsEnvVars { + //TODO: This needs to be fetched from the provider secret + if awsEnvVars[i].Name == RegistryStorageS3AccesskeyEnvVarKey { + awsEnvVars[i].Value = "" + } - if key.Name == RegistryStorageS3BucketEnvVarKey { - key.Value = bsl.Spec.StorageType.ObjectStorage.Bucket - } + if awsEnvVars[i].Name == RegistryStorageS3BucketEnvVarKey { + awsEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket + } - if key.Name == RegistryStorageS3RegionEnvVarKey { - key.Value = bsl.Spec.Config[Region] - } - //TODO: This needs to be fetch from the provider secret - if key.Name == RegistryStorageS3SecretkeyEnvVarKey { - key.Value = "" - } + if awsEnvVars[i].Name == RegistryStorageS3RegionEnvVarKey { + awsEnvVars[i].Value = bsl.Spec.Config[Region] + } + //TODO: This needs to be fetched from the provider secret + if awsEnvVars[i].Name == RegistryStorageS3SecretkeyEnvVarKey { + awsEnvVars[i].Value = "" + } - if key.Name == RegistryStorageS3RegionendpointEnvVarKey && bsl.Spec.Config[S3URL] != "" { - key.Value = bsl.Spec.Config[S3URL] - } + if awsEnvVars[i].Name == RegistryStorageS3RegionendpointEnvVarKey && bsl.Spec.Config[S3URL] != "" { + awsEnvVars[i].Value = bsl.Spec.Config[S3URL] + } - if key.Name == RegistryStorageS3RootdirectoryEnvVarKey && bsl.Spec.Config[RootDirectory] != "" { - key.Value = bsl.Spec.Config[RootDirectory] - } + if awsEnvVars[i].Name == RegistryStorageS3RootdirectoryEnvVarKey && bsl.Spec.Config[RootDirectory] != "" { + awsEnvVars[i].Value = bsl.Spec.Config[RootDirectory] + } - if key.Name == RegistryStorageS3SkipverifyEnvVarKey && bsl.Spec.Config[InsecureSkipTLSVerify] != "" { - key.Value = bsl.Spec.Config[InsecureSkipTLSVerify] - } + if awsEnvVars[i].Name == RegistryStorageS3SkipverifyEnvVarKey && bsl.Spec.Config[InsecureSkipTLSVerify] != "" { + awsEnvVars[i].Value = bsl.Spec.Config[InsecureSkipTLSVerify] } } + return awsEnvVars +} - // build Azure env vars - if provider == AzureProvider { - for _, key := range updatedEnvVar { - if key.Name == RegistryStorageAzureContainerEnvVarKey { - key.Value = bsl.Spec.StorageType.ObjectStorage.Bucket - } +func getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars []corev1.EnvVar) []corev1.EnvVar { - if key.Name == RegistryStorageAzureAccountnameEnvVarKey { - key.Value = bsl.Spec.Config[StorageAccount] - } - //TODO: This needs to be fetch from the provider secret - if key.Name == RegistryStorageAzureAccountkeyEnvVarKey { - key.Value = "" - } + for i, _ := range azureEnvVars { + if azureEnvVars[i].Name == RegistryStorageAzureContainerEnvVarKey { + azureEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket } - } - // build GCP env vars - if provider == GCPProvider { - for _, key := range updatedEnvVar { - if key.Name == RegistryStorageGCSBucket { - key.Value = bsl.Spec.StorageType.ObjectStorage.Bucket - } - //TODO: This needs to be fetch from the provider secret - if key.Name == RegistryStorageGCSKeyfile { - key.Value = "" - } - if key.Name == RegistryStorageGCSRootdirectory && bsl.Spec.Config[RootDirectory] != "" { - key.Value = bsl.Spec.Config[RootDirectory] - } + if azureEnvVars[i].Name == RegistryStorageAzureAccountnameEnvVarKey { + azureEnvVars[i].Value = bsl.Spec.Config[StorageAccount] + } + //TODO: This needs to be fetched from the provider secret + if azureEnvVars[i].Name == RegistryStorageAzureAccountkeyEnvVarKey { + azureEnvVars[i].Value = "" } } + return azureEnvVars +} - return updatedEnvVar +func getGCPRegistryEnvVars(bsl *velerov1.BackupStorageLocation, gcpEnvVars []corev1.EnvVar) []corev1.EnvVar { + + for i, _ := range gcpEnvVars { + if gcpEnvVars[i].Name == RegistryStorageGCSBucket { + gcpEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket + } + //TODO: This needs to be fetched from the provider secret + if gcpEnvVars[i].Name == RegistryStorageGCSKeyfile { + gcpEnvVars[i].Value = "" + } + if gcpEnvVars[i].Name == RegistryStorageGCSRootdirectory && bsl.Spec.Config[RootDirectory] != "" { + gcpEnvVars[i].Value = bsl.Spec.Config[RootDirectory] + } + } + return gcpEnvVars } diff --git a/oadp-operator/controllers/velero.go b/oadp-operator/controllers/velero.go index 113a799602e..de254a96816 100644 --- a/oadp-operator/controllers/velero.go +++ b/oadp-operator/controllers/velero.go @@ -14,12 +14,12 @@ import ( ) const ( - Velero = "velero" - VeleoNamespace = "oadp-operator" - OADPOperator = "oadp-operator" - Server = "server" - Registry = "Registry" - BackupStorageLocation = "BackupStorageLocation" + Velero = "velero" + VeleoNamespace = "oadp-operator" + OADPOperator = "oadp-operator" + OADPOperatorVelero = "oadp-operator-velero" + Server = "server" + Registry = "Registry" //TODO: Check for default secret names VeleroAWSSecretName = "cloud-credentials" VeleroAzureSecretName = "cloud-credentials-azure" From 94773fc29c81e64e85d30658bbb2d593b90f0252 Mon Sep 17 00:00:00 2001 From: Shubham Dilip Pampattiwar Date: Wed, 4 Aug 2021 15:46:52 -0400 Subject: [PATCH 3/6] minor updates --- oadp-operator/controllers/registry.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/oadp-operator/controllers/registry.go b/oadp-operator/controllers/registry.go index 66fce5aaed6..9e62b6ef8e6 100644 --- a/oadp-operator/controllers/registry.go +++ b/oadp-operator/controllers/registry.go @@ -267,23 +267,21 @@ func (r *VeleroReconciler) buildRegistryContainer(bsl *velerov1.BackupStorageLoc func getRegistryEnvVars(bsl *velerov1.BackupStorageLocation) []corev1.EnvVar { envVar := []corev1.EnvVar{} - if bsl.Spec.Provider == AWSProvider { + provider := bsl.Spec.Provider + switch provider { + case AWSProvider: envVar = getAWSRegistryEnvVars(bsl, cloudProviderEnvVarMap[AWSProvider]) - } - if bsl.Spec.Provider == AzureProvider { + case AzureProvider: envVar = getAzureRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider]) - } - if bsl.Spec.Provider == GCPProvider { + case GCPProvider: envVar = getGCPRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider]) } - return envVar } func getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation, awsEnvVars []corev1.EnvVar) []corev1.EnvVar { - for i, _ := range awsEnvVars { //TODO: This needs to be fetched from the provider secret if awsEnvVars[i].Name == RegistryStorageS3AccesskeyEnvVarKey { @@ -318,7 +316,6 @@ func getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation, awsEnvVars []cor } func getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars []corev1.EnvVar) []corev1.EnvVar { - for i, _ := range azureEnvVars { if azureEnvVars[i].Name == RegistryStorageAzureContainerEnvVarKey { azureEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket @@ -336,7 +333,6 @@ func getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars [ } func getGCPRegistryEnvVars(bsl *velerov1.BackupStorageLocation, gcpEnvVars []corev1.EnvVar) []corev1.EnvVar { - for i, _ := range gcpEnvVars { if gcpEnvVars[i].Name == RegistryStorageGCSBucket { gcpEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket From faf4ce1878cb9de2cdef30aef54ceb56f6d327d2 Mon Sep 17 00:00:00 2001 From: Shubham Dilip Pampattiwar Date: Wed, 4 Aug 2021 17:57:01 -0400 Subject: [PATCH 4/6] add unit tests for buildRegistryDeployment --- oadp-operator/controllers/registry.go | 21 ++-- oadp-operator/controllers/registry_test.go | 119 +++++++++++++++++++++ 2 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 oadp-operator/controllers/registry_test.go diff --git a/oadp-operator/controllers/registry.go b/oadp-operator/controllers/registry.go index 9e62b6ef8e6..a8e7f360ee4 100644 --- a/oadp-operator/controllers/registry.go +++ b/oadp-operator/controllers/registry.go @@ -159,15 +159,9 @@ func (r *VeleroReconciler) ReconcileRegistries(log logr.Logger) (bool, error) { } } - // Setting controller owner reference on the registry deployment - err := controllerutil.SetControllerReference(&bsl, registryDeployment, r.Scheme) - if err != nil { - return err - } - // update the Registry Deployment template - registryDeployment = r.buildRegistryDeployment(registryDeployment, &bsl) - return nil + err := r.buildRegistryDeployment(registryDeployment, &bsl) + return err }) if err != nil { @@ -191,7 +185,14 @@ func (r *VeleroReconciler) ReconcileRegistries(log logr.Logger) (bool, error) { } // Construct and update the registry deployment for a bsl -func (r *VeleroReconciler) buildRegistryDeployment(registryDeployment *appsv1.Deployment, bsl *velerov1.BackupStorageLocation) *appsv1.Deployment { +func (r *VeleroReconciler) buildRegistryDeployment(registryDeployment *appsv1.Deployment, bsl *velerov1.BackupStorageLocation) error { + + // Setting controller owner reference on the registry deployment + err := controllerutil.SetControllerReference(bsl, registryDeployment, r.Scheme) + if err != nil { + return err + } + registryDeployment.Labels = r.getRegistryBSLLabels(bsl) registryDeployment.Spec = appsv1.DeploymentSpec{ @@ -208,7 +209,7 @@ func (r *VeleroReconciler) buildRegistryDeployment(registryDeployment *appsv1.De }, }, } - return registryDeployment + return nil } func (r *VeleroReconciler) getRegistryBSLLabels(bsl *velerov1.BackupStorageLocation) map[string]string { diff --git a/oadp-operator/controllers/registry_test.go b/oadp-operator/controllers/registry_test.go new file mode 100644 index 00000000000..6ce190606dc --- /dev/null +++ b/oadp-operator/controllers/registry_test.go @@ -0,0 +1,119 @@ +package controllers + +import ( + velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" + "reflect" + "testing" +) + +func TestVeleroReconciler_buildRegistryDeployment(t *testing.T) { + tests := []struct { + name string + registryDeployment *appsv1.Deployment + bsl *velerov1.BackupStorageLocation + wantErr bool + }{ + { + name: "registry without owner reference as well as labels", + registryDeployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-registry", + Namespace: "test-ns", + }, + }, + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + }, + }, + { + name: "registry without owner reference but has labels", + registryDeployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-registry", + Namespace: "test-ns", + Labels: map[string]string{ + "app.kubernetes.io/name": OADPOperatorVelero, + "app.kubernetes.io/instance": "oadp-test-bsl-test-ns-registry", + "app.kubernetes.io/managed-by": OADPOperator, + "app.kubernetes.io/component": Registry, + }, + }, + }, + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme, err := getSchemeForFakeClient() + if err != nil { + t.Errorf("error getting scheme for the test: %#v", err) + } + r := &VeleroReconciler{ + Scheme: scheme, + } + wantRegistryDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-registry", + Namespace: "test-ns", + Labels: map[string]string{ + "app.kubernetes.io/name": OADPOperatorVelero, + "app.kubernetes.io/instance": "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry", + "app.kubernetes.io/managed-by": OADPOperator, + "app.kubernetes.io/component": Registry, + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "velero.io/v1", + Kind: "BackupStorageLocation", + Name: tt.bsl.Name, + UID: tt.bsl.UID, + Controller: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(true), + }, + }, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: pointer.Int32(1), + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "component": "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry", + }, + }, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyAlways, + //Containers: r.buildRegistryContainer(bsl), + }, + }, + }, + } + + err = r.buildRegistryDeployment(tt.registryDeployment, tt.bsl) + if (err != nil) != tt.wantErr { + t.Errorf("buildRegistryDeployment() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(wantRegistryDeployment.Labels, tt.registryDeployment.Labels) { + t.Errorf("expected registry deployment labels to be %#v, got %#v", wantRegistryDeployment.Labels, tt.registryDeployment.Labels) + } + if !reflect.DeepEqual(wantRegistryDeployment.OwnerReferences, tt.registryDeployment.OwnerReferences) { + t.Errorf("expected registry deployment owner references to be %#v, got %#v", wantRegistryDeployment.OwnerReferences, tt.registryDeployment.OwnerReferences) + } + if !reflect.DeepEqual(wantRegistryDeployment.Spec.Replicas, tt.registryDeployment.Spec.Replicas) { + t.Errorf("expected registry deployment replicas to be %#v, got %#v", wantRegistryDeployment.Spec.Replicas, tt.registryDeployment.Spec.Replicas) + } + }) + } +} From 291ba11da617e8a553d6bb661ba12ca526ac899e Mon Sep 17 00:00:00 2001 From: Shubham Dilip Pampattiwar Date: Thu, 5 Aug 2021 11:25:50 -0400 Subject: [PATCH 5/6] add provider specific unit tests --- oadp-operator/controllers/registry.go | 16 +- oadp-operator/controllers/registry_test.go | 295 ++++++++++++++++++++- 2 files changed, 302 insertions(+), 9 deletions(-) diff --git a/oadp-operator/controllers/registry.go b/oadp-operator/controllers/registry.go index a8e7f360ee4..08b495b1fe4 100644 --- a/oadp-operator/controllers/registry.go +++ b/oadp-operator/controllers/registry.go @@ -237,7 +237,7 @@ func (r *VeleroReconciler) buildRegistryContainer(bsl *velerov1.BackupStorageLoc Protocol: corev1.ProtocolTCP, }, }, - Env: getRegistryEnvVars(bsl), + Env: r.getRegistryEnvVars(bsl), LivenessProbe: &corev1.Probe{ Handler: corev1.Handler{ HTTPGet: &corev1.HTTPGetAction{ @@ -266,23 +266,23 @@ func (r *VeleroReconciler) buildRegistryContainer(bsl *velerov1.BackupStorageLoc return containers } -func getRegistryEnvVars(bsl *velerov1.BackupStorageLocation) []corev1.EnvVar { +func (r *VeleroReconciler) getRegistryEnvVars(bsl *velerov1.BackupStorageLocation) []corev1.EnvVar { envVar := []corev1.EnvVar{} provider := bsl.Spec.Provider switch provider { case AWSProvider: - envVar = getAWSRegistryEnvVars(bsl, cloudProviderEnvVarMap[AWSProvider]) + envVar = r.getAWSRegistryEnvVars(bsl, cloudProviderEnvVarMap[AWSProvider]) case AzureProvider: - envVar = getAzureRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider]) + envVar = r.getAzureRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider]) case GCPProvider: - envVar = getGCPRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider]) + envVar = r.getGCPRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider]) } return envVar } -func getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation, awsEnvVars []corev1.EnvVar) []corev1.EnvVar { +func (r *VeleroReconciler) getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation, awsEnvVars []corev1.EnvVar) []corev1.EnvVar { for i, _ := range awsEnvVars { //TODO: This needs to be fetched from the provider secret if awsEnvVars[i].Name == RegistryStorageS3AccesskeyEnvVarKey { @@ -316,7 +316,7 @@ func getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation, awsEnvVars []cor return awsEnvVars } -func getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars []corev1.EnvVar) []corev1.EnvVar { +func (r *VeleroReconciler) getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars []corev1.EnvVar) []corev1.EnvVar { for i, _ := range azureEnvVars { if azureEnvVars[i].Name == RegistryStorageAzureContainerEnvVarKey { azureEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket @@ -333,7 +333,7 @@ func getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars [ return azureEnvVars } -func getGCPRegistryEnvVars(bsl *velerov1.BackupStorageLocation, gcpEnvVars []corev1.EnvVar) []corev1.EnvVar { +func (r *VeleroReconciler) getGCPRegistryEnvVars(bsl *velerov1.BackupStorageLocation, gcpEnvVars []corev1.EnvVar) []corev1.EnvVar { for i, _ := range gcpEnvVars { if gcpEnvVars[i].Name == RegistryStorageGCSBucket { gcpEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket diff --git a/oadp-operator/controllers/registry_test.go b/oadp-operator/controllers/registry_test.go index 6ce190606dc..659a606864d 100644 --- a/oadp-operator/controllers/registry_test.go +++ b/oadp-operator/controllers/registry_test.go @@ -5,6 +5,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/pointer" "reflect" "testing" @@ -94,7 +95,6 @@ func TestVeleroReconciler_buildRegistryDeployment(t *testing.T) { }, Spec: corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyAlways, - //Containers: r.buildRegistryContainer(bsl), }, }, }, @@ -117,3 +117,296 @@ func TestVeleroReconciler_buildRegistryDeployment(t *testing.T) { }) } } + +func TestVeleroReconciler_buildRegistryContainer(t *testing.T) { + tests := []struct { + name string + bsl *velerov1.BackupStorageLocation + wantErr bool + }{ + { + name: "given bsl appropriate container is built or not", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme, err := getSchemeForFakeClient() + if err != nil { + t.Errorf("error getting scheme for the test: %#v", err) + } + r := &VeleroReconciler{ + Scheme: scheme, + } + wantRegistryContainer := &corev1.Container{ + Image: RegistryImage, + Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry" + "-container", + Ports: []corev1.ContainerPort{ + { + ContainerPort: 5000, + Protocol: corev1.ProtocolTCP, + }, + }, + LivenessProbe: &corev1.Probe{ + Handler: corev1.Handler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/v2/_catalog?n=5", + Port: intstr.IntOrString{IntVal: 5000}, + }, + }, + PeriodSeconds: 5, + TimeoutSeconds: 3, + InitialDelaySeconds: 15, + }, + ReadinessProbe: &corev1.Probe{ + Handler: corev1.Handler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/v2/_catalog?n=5", + Port: intstr.IntOrString{IntVal: 5000}, + }, + }, + PeriodSeconds: 5, + TimeoutSeconds: 3, + InitialDelaySeconds: 15, + }, + } + + gotRegistryContainer := r.buildRegistryContainer(tt.bsl) + + if !reflect.DeepEqual(wantRegistryContainer.Name, gotRegistryContainer[0].Name) { + t.Errorf("expected registry container name to be %#v, got %#v", wantRegistryContainer.Name, gotRegistryContainer[0].Name) + } + + if !reflect.DeepEqual(wantRegistryContainer.Ports, gotRegistryContainer[0].Ports) { + t.Errorf("expected registry container ports to be %#v, got %#v", wantRegistryContainer.Ports, gotRegistryContainer[0].Ports) + } + if !reflect.DeepEqual(wantRegistryContainer.ReadinessProbe, gotRegistryContainer[0].ReadinessProbe) { + t.Errorf("expected registry container readiness probe to be %#v, got %#v", wantRegistryContainer.ReadinessProbe, gotRegistryContainer[0].ReadinessProbe) + } + if !reflect.DeepEqual(wantRegistryContainer.LivenessProbe, gotRegistryContainer[0].LivenessProbe) { + t.Errorf("expected registry container liveness probe to be %#v, got %#v", wantRegistryContainer.LivenessProbe, gotRegistryContainer[0].LivenessProbe) + } + + }) + } +} + +var testAWSEnvVar = cloudProviderEnvVarMap["aws"] +var testAzureEnvVar = cloudProviderEnvVarMap["azure"] +var testGCPEnvVar = cloudProviderEnvVarMap["gcp"] + +func TestVeleroReconciler_getAWSRegistryEnvVars(t *testing.T) { + tests := []struct { + name string + bsl *velerov1.BackupStorageLocation + wantErr bool + }{ + { + name: "given aws bsl, appropriate env var for the container are returned", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: AWSProvider, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "aws-bucket", + }, + }, + Config: map[string]string{ + Region: "aws-region", + S3URL: "https://sr-url-aws-domain.com", + RootDirectory: "/velero-aws", + InsecureSkipTLSVerify: "false", + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme, err := getSchemeForFakeClient() + if err != nil { + t.Errorf("error getting scheme for the test: %#v", err) + } + r := &VeleroReconciler{ + Scheme: scheme, + } + wantRegistryContainerEnvVar := []corev1.EnvVar{ + { + Name: RegistryStorageEnvVarKey, + Value: S3, + }, + { + Name: RegistryStorageS3AccesskeyEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageS3BucketEnvVarKey, + Value: "aws-bucket", + }, + { + Name: RegistryStorageS3RegionEnvVarKey, + Value: "aws-region", + }, + { + Name: RegistryStorageS3SecretkeyEnvVarKey, + Value: "", + }, + { + Name: RegistryStorageS3RegionendpointEnvVarKey, + Value: "https://sr-url-aws-domain.com", + }, + { + Name: RegistryStorageS3RootdirectoryEnvVarKey, + Value: "/velero-aws", + }, + { + Name: RegistryStorageS3SkipverifyEnvVarKey, + Value: "false", + }, + } + + gotRegistryContainerEnvVar := r.getAWSRegistryEnvVars(tt.bsl, testAWSEnvVar) + + if !reflect.DeepEqual(wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { + t.Errorf("expected registry container env var length to be %#v, got %#v", wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) + } + }) + } +} + +func TestVeleroReconciler_getAzureRegistryEnvVars(t *testing.T) { + tests := []struct { + name string + bsl *velerov1.BackupStorageLocation + wantErr bool + }{ + { + name: "given azure bsl, appropriate env var for the container are returned", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: AzureProvider, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "azure-bucket", + }, + }, + Config: map[string]string{ + StorageAccount: "velero-azure-account", + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme, err := getSchemeForFakeClient() + if err != nil { + t.Errorf("error getting scheme for the test: %#v", err) + } + r := &VeleroReconciler{ + Scheme: scheme, + } + wantRegistryContainerEnvVar := []corev1.EnvVar{ + { + Name: RegistryStorageEnvVarKey, + Value: Azure, + }, + { + Name: RegistryStorageAzureContainerEnvVarKey, + Value: "azure-bucket", + }, + { + Name: RegistryStorageAzureAccountnameEnvVarKey, + Value: "velero-azure-account", + }, + { + Name: RegistryStorageAzureAccountkeyEnvVarKey, + Value: "", + }, + } + + gotRegistryContainerEnvVar := r.getAzureRegistryEnvVars(tt.bsl, testAzureEnvVar) + + if !reflect.DeepEqual(wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { + t.Errorf("expected registry container env var length to be %#v, got %#v", wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) + } + }) + } +} + +func TestVeleroReconciler_getGCPRegistryEnvVars(t *testing.T) { + tests := []struct { + name string + bsl *velerov1.BackupStorageLocation + wantErr bool + }{ + { + name: "given gcp bsl, appropriate env var for the container are returned", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: GCPProvider, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "gcp-bucket", + }, + }, + Config: map[string]string{ + RootDirectory: "/velero-gcp", + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme, err := getSchemeForFakeClient() + if err != nil { + t.Errorf("error getting scheme for the test: %#v", err) + } + r := &VeleroReconciler{ + Scheme: scheme, + } + wantRegistryContainerEnvVar := []corev1.EnvVar{ + { + Name: RegistryStorageEnvVarKey, + Value: GCS, + }, + { + Name: RegistryStorageGCSBucket, + Value: "gcp-bucket", + }, + { + Name: RegistryStorageGCSKeyfile, + Value: "", + }, + { + Name: RegistryStorageGCSRootdirectory, + Value: "/velero-gcp", + }, + } + + gotRegistryContainerEnvVar := r.getGCPRegistryEnvVars(tt.bsl, testGCPEnvVar) + + if !reflect.DeepEqual(wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { + t.Errorf("expected registry container env var length to be %#v, got %#v", wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) + } + }) + } +} From a94fac5ad9e67f180a8da862f71edf355205ba6a Mon Sep 17 00:00:00 2001 From: Shubham Dilip Pampattiwar Date: Thu, 5 Aug 2021 14:11:32 -0400 Subject: [PATCH 6/6] minor updates to unit tests --- oadp-operator/controllers/registry.go | 12 ++--- oadp-operator/controllers/registry_test.go | 60 +++++++++++----------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/oadp-operator/controllers/registry.go b/oadp-operator/controllers/registry.go index 08b495b1fe4..a86a276ecd0 100644 --- a/oadp-operator/controllers/registry.go +++ b/oadp-operator/controllers/registry.go @@ -143,7 +143,7 @@ func (r *VeleroReconciler) ReconcileRegistries(log logr.Logger) (bool, error) { for _, bsl := range backupStorageLocationList.Items { registryDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ - Name: constructBSLRegistryName(&bsl), + Name: registryName(&bsl), Namespace: VeleoNamespace, }, } @@ -154,7 +154,7 @@ func (r *VeleroReconciler) ReconcileRegistries(log logr.Logger) (bool, error) { if registryDeployment.ObjectMeta.CreationTimestamp.IsZero() { registryDeployment.Spec.Selector = &metav1.LabelSelector{ MatchLabels: map[string]string{ - "component": constructBSLRegistryName(&bsl), + "component": registryName(&bsl), }, } } @@ -200,7 +200,7 @@ func (r *VeleroReconciler) buildRegistryDeployment(registryDeployment *appsv1.De Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "component": constructBSLRegistryName(bsl), + "component": registryName(bsl), }, }, Spec: corev1.PodSpec{ @@ -215,14 +215,14 @@ func (r *VeleroReconciler) buildRegistryDeployment(registryDeployment *appsv1.De func (r *VeleroReconciler) getRegistryBSLLabels(bsl *velerov1.BackupStorageLocation) map[string]string { labels := map[string]string{ "app.kubernetes.io/name": OADPOperatorVelero, - "app.kubernetes.io/instance": constructBSLRegistryName(bsl), + "app.kubernetes.io/instance": registryName(bsl), "app.kubernetes.io/managed-by": OADPOperator, "app.kubernetes.io/component": Registry, } return labels } -func constructBSLRegistryName(bsl *velerov1.BackupStorageLocation) string { +func registryName(bsl *velerov1.BackupStorageLocation) string { return "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry" } @@ -230,7 +230,7 @@ func (r *VeleroReconciler) buildRegistryContainer(bsl *velerov1.BackupStorageLoc containers := []corev1.Container{ { Image: RegistryImage, - Name: constructBSLRegistryName(bsl) + "-container", + Name: registryName(bsl) + "-container", Ports: []corev1.ContainerPort{ { ContainerPort: 5000, diff --git a/oadp-operator/controllers/registry_test.go b/oadp-operator/controllers/registry_test.go index 659a606864d..40328b983d3 100644 --- a/oadp-operator/controllers/registry_test.go +++ b/oadp-operator/controllers/registry_test.go @@ -120,9 +120,9 @@ func TestVeleroReconciler_buildRegistryDeployment(t *testing.T) { func TestVeleroReconciler_buildRegistryContainer(t *testing.T) { tests := []struct { - name string - bsl *velerov1.BackupStorageLocation - wantErr bool + name string + bsl *velerov1.BackupStorageLocation + wantRegistryContainer *corev1.Container }{ { name: "given bsl appropriate container is built or not", @@ -143,7 +143,7 @@ func TestVeleroReconciler_buildRegistryContainer(t *testing.T) { r := &VeleroReconciler{ Scheme: scheme, } - wantRegistryContainer := &corev1.Container{ + tt.wantRegistryContainer = &corev1.Container{ Image: RegistryImage, Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry" + "-container", Ports: []corev1.ContainerPort{ @@ -178,18 +178,18 @@ func TestVeleroReconciler_buildRegistryContainer(t *testing.T) { gotRegistryContainer := r.buildRegistryContainer(tt.bsl) - if !reflect.DeepEqual(wantRegistryContainer.Name, gotRegistryContainer[0].Name) { - t.Errorf("expected registry container name to be %#v, got %#v", wantRegistryContainer.Name, gotRegistryContainer[0].Name) + if !reflect.DeepEqual(tt.wantRegistryContainer.Name, gotRegistryContainer[0].Name) { + t.Errorf("expected registry container name to be %#v, got %#v", tt.wantRegistryContainer.Name, gotRegistryContainer[0].Name) } - if !reflect.DeepEqual(wantRegistryContainer.Ports, gotRegistryContainer[0].Ports) { - t.Errorf("expected registry container ports to be %#v, got %#v", wantRegistryContainer.Ports, gotRegistryContainer[0].Ports) + if !reflect.DeepEqual(tt.wantRegistryContainer.Ports, gotRegistryContainer[0].Ports) { + t.Errorf("expected registry container ports to be %#v, got %#v", tt.wantRegistryContainer.Ports, gotRegistryContainer[0].Ports) } - if !reflect.DeepEqual(wantRegistryContainer.ReadinessProbe, gotRegistryContainer[0].ReadinessProbe) { - t.Errorf("expected registry container readiness probe to be %#v, got %#v", wantRegistryContainer.ReadinessProbe, gotRegistryContainer[0].ReadinessProbe) + if !reflect.DeepEqual(tt.wantRegistryContainer.ReadinessProbe, gotRegistryContainer[0].ReadinessProbe) { + t.Errorf("expected registry container readiness probe to be %#v, got %#v", tt.wantRegistryContainer.ReadinessProbe, gotRegistryContainer[0].ReadinessProbe) } - if !reflect.DeepEqual(wantRegistryContainer.LivenessProbe, gotRegistryContainer[0].LivenessProbe) { - t.Errorf("expected registry container liveness probe to be %#v, got %#v", wantRegistryContainer.LivenessProbe, gotRegistryContainer[0].LivenessProbe) + if !reflect.DeepEqual(tt.wantRegistryContainer.LivenessProbe, gotRegistryContainer[0].LivenessProbe) { + t.Errorf("expected registry container liveness probe to be %#v, got %#v", tt.wantRegistryContainer.LivenessProbe, gotRegistryContainer[0].LivenessProbe) } }) @@ -202,9 +202,9 @@ var testGCPEnvVar = cloudProviderEnvVarMap["gcp"] func TestVeleroReconciler_getAWSRegistryEnvVars(t *testing.T) { tests := []struct { - name string - bsl *velerov1.BackupStorageLocation - wantErr bool + name string + bsl *velerov1.BackupStorageLocation + wantRegistryContainerEnvVar []corev1.EnvVar }{ { name: "given aws bsl, appropriate env var for the container are returned", @@ -239,7 +239,7 @@ func TestVeleroReconciler_getAWSRegistryEnvVars(t *testing.T) { r := &VeleroReconciler{ Scheme: scheme, } - wantRegistryContainerEnvVar := []corev1.EnvVar{ + tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ { Name: RegistryStorageEnvVarKey, Value: S3, @@ -276,8 +276,8 @@ func TestVeleroReconciler_getAWSRegistryEnvVars(t *testing.T) { gotRegistryContainerEnvVar := r.getAWSRegistryEnvVars(tt.bsl, testAWSEnvVar) - if !reflect.DeepEqual(wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { - t.Errorf("expected registry container env var length to be %#v, got %#v", wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) + if !reflect.DeepEqual(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { + t.Errorf("expected registry container env var to be %#v, got %#v", tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) } }) } @@ -285,9 +285,9 @@ func TestVeleroReconciler_getAWSRegistryEnvVars(t *testing.T) { func TestVeleroReconciler_getAzureRegistryEnvVars(t *testing.T) { tests := []struct { - name string - bsl *velerov1.BackupStorageLocation - wantErr bool + name string + bsl *velerov1.BackupStorageLocation + wantRegistryContainerEnvVar []corev1.EnvVar }{ { name: "given azure bsl, appropriate env var for the container are returned", @@ -319,7 +319,7 @@ func TestVeleroReconciler_getAzureRegistryEnvVars(t *testing.T) { r := &VeleroReconciler{ Scheme: scheme, } - wantRegistryContainerEnvVar := []corev1.EnvVar{ + tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ { Name: RegistryStorageEnvVarKey, Value: Azure, @@ -340,8 +340,8 @@ func TestVeleroReconciler_getAzureRegistryEnvVars(t *testing.T) { gotRegistryContainerEnvVar := r.getAzureRegistryEnvVars(tt.bsl, testAzureEnvVar) - if !reflect.DeepEqual(wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { - t.Errorf("expected registry container env var length to be %#v, got %#v", wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) + if !reflect.DeepEqual(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { + t.Errorf("expected registry container env var to be %#v, got %#v", tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) } }) } @@ -349,9 +349,9 @@ func TestVeleroReconciler_getAzureRegistryEnvVars(t *testing.T) { func TestVeleroReconciler_getGCPRegistryEnvVars(t *testing.T) { tests := []struct { - name string - bsl *velerov1.BackupStorageLocation - wantErr bool + name string + bsl *velerov1.BackupStorageLocation + wantRegistryContainerEnvVar []corev1.EnvVar }{ { name: "given gcp bsl, appropriate env var for the container are returned", @@ -383,7 +383,7 @@ func TestVeleroReconciler_getGCPRegistryEnvVars(t *testing.T) { r := &VeleroReconciler{ Scheme: scheme, } - wantRegistryContainerEnvVar := []corev1.EnvVar{ + tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ { Name: RegistryStorageEnvVarKey, Value: GCS, @@ -404,8 +404,8 @@ func TestVeleroReconciler_getGCPRegistryEnvVars(t *testing.T) { gotRegistryContainerEnvVar := r.getGCPRegistryEnvVars(tt.bsl, testGCPEnvVar) - if !reflect.DeepEqual(wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { - t.Errorf("expected registry container env var length to be %#v, got %#v", wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) + if !reflect.DeepEqual(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { + t.Errorf("expected registry container env var to be %#v, got %#v", tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) } }) }