From 2665b29e24ff2d9a1b3be746285ab4a37e91429e Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Tue, 12 Jul 2022 09:43:47 -0400 Subject: [PATCH] OADP-205: Remove registry deployment in favor of plugin enhancement (#743) * wip * add OPENSHIFT_IMAGESTREAM_BACKUP env to velero container * remove registry deployment waits, add TODO for s3 check * comment out empty func * remove commented code * Remove func and tests for code deprecated or moved to plugin repo Removed buildRegistryDeployment, buildRegistryContainer, getAWS/Azure/GCPRegistryEnvVars, populateAWSRegistrySecret, updateRegistrySVC/Route/RouteCM * remove registry from CSV * remove registry unsupportedOverride key * add name/ns to deletion event message * force velero loglevel to debug to expose registry logs * Print velero container logs after each entry * ignore debug logs we don't care about * remove FEntry * add "blob unknown" to failure log ignore * disable backupImages for GCP provider * add import b/r entry name clarification update --- api/v1alpha1/oadp_types.go | 1 - .../oadp-operator.clusterserviceversion.yaml | 6 - config/manager/manager.yaml | 4 - .../oadp-operator.clusterserviceversion.yaml | 2 - controllers/registry.go | 607 +------ controllers/registry_test.go | 1530 +---------------- controllers/velero.go | 6 + controllers/velero_test.go | 68 + tests/e2e/backup_restore_suite_test.go | 26 +- tests/e2e/dpa_deployment_suite_test.go | 10 - tests/e2e/lib/dpa_helpers.go | 5 +- tests/e2e/lib/velero_helpers.go | 32 +- tests/e2e/subscription_suite_test.go | 8 +- 13 files changed, 232 insertions(+), 2073 deletions(-) diff --git a/api/v1alpha1/oadp_types.go b/api/v1alpha1/oadp_types.go index 2b15841730..fd6cdc206f 100644 --- a/api/v1alpha1/oadp_types.go +++ b/api/v1alpha1/oadp_types.go @@ -56,7 +56,6 @@ const GCPPluginImageKey UnsupportedImageKey = "gcpPluginImageFqin" const CSIPluginImageKey UnsupportedImageKey = "csiPluginImageFqin" const DataMoverImageKey UnsupportedImageKey = "dataMoverImageFqin" const ResticRestoreImageKey UnsupportedImageKey = "resticRestoreImageFqin" -const RegistryImageKey UnsupportedImageKey = "registryImageFqin" const KubeVirtPluginImageKey UnsupportedImageKey = "kubevirtPluginImageFqin" const OperatorTypeKey UnsupportedImageKey = "operator-type" diff --git a/bundle/manifests/oadp-operator.clusterserviceversion.yaml b/bundle/manifests/oadp-operator.clusterserviceversion.yaml index 0b0bd46b3f..6937fdf103 100644 --- a/bundle/manifests/oadp-operator.clusterserviceversion.yaml +++ b/bundle/manifests/oadp-operator.clusterserviceversion.yaml @@ -708,10 +708,6 @@ spec: value: quay.io - name: PROJECT value: konveyor - - name: VELERO_REGISTRY_REPO - value: registry - - name: VELERO_REGISTRY_TAG - value: latest - name: VELERO_REPO value: velero - name: VELERO_OPENSHIFT_PLUGIN_REPO @@ -864,8 +860,6 @@ spec: name: gcp_plugin - image: quay.io/konveyor/velero-plugin-for-csi:konveyor-0.3 name: csi_plugin - - image: quay.io/konveyor/registry:latest - name: registry - image: quay.io/konveyor/kubevirt-velero-plugin:v0.2.0 name: kubevirt_plugin version: 99.0.0 diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index d6b6c774d9..b4b8a06432 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -41,10 +41,6 @@ spec: value: quay.io - name: PROJECT value: konveyor - - name: VELERO_REGISTRY_REPO - value: registry - - name: VELERO_REGISTRY_TAG - value: latest - name: VELERO_REPO value: velero - name: VELERO_OPENSHIFT_PLUGIN_REPO diff --git a/config/manifests/bases/oadp-operator.clusterserviceversion.yaml b/config/manifests/bases/oadp-operator.clusterserviceversion.yaml index 35a27076e4..5d01f8cd1a 100644 --- a/config/manifests/bases/oadp-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/oadp-operator.clusterserviceversion.yaml @@ -355,8 +355,6 @@ spec: name: gcp_plugin - image: quay.io/konveyor/velero-plugin-for-csi:konveyor-0.3 name: csi_plugin - - image: quay.io/konveyor/registry:latest - name: registry - image: quay.io/konveyor/kubevirt-velero-plugin:v0.2.0 name: kubevirt_plugin version: 99.0.0 diff --git a/controllers/registry.go b/controllers/registry.go index ec638cc54b..a1d1e705dc 100644 --- a/controllers/registry.go +++ b/controllers/registry.go @@ -4,8 +4,6 @@ import ( "context" "errors" "fmt" - "os" - "reflect" "regexp" "strings" @@ -23,10 +21,7 @@ import ( corev1 "k8s.io/api/core/v1" k8serror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/utils/pointer" - "github.com/operator-framework/operator-lib/proxy" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) @@ -192,361 +187,32 @@ func (r *DPAReconciler) ReconcileRegistries(log logr.Logger) (bool, error) { Namespace: bsl.Namespace, }, } - - if !dpa.BackupImages() { - deleteContext := context.Background() - if err := r.Get(deleteContext, types.NamespacedName{ - Name: registryDeployment.Name, - Namespace: r.NamespacedName.Namespace, - }, registryDeployment); err != nil { - if k8serror.IsNotFound(err) { - return true, nil - } - return false, err - } - - deleteOptionPropagationForeground := metav1.DeletePropagationForeground - if err := r.Delete(deleteContext, registryDeployment, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { - r.EventRecorder.Event(registryDeployment, corev1.EventTypeNormal, "DeleteRegistryDeploymentFailed", "Could not delete registry deployment:"+err.Error()) - return false, err - } - r.EventRecorder.Event(registryDeployment, corev1.EventTypeNormal, "DeletedRegistryDeployment", "Registry Deployment deleted") - - return true, nil - } - - 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": registryName(&bsl), - }, - } - } - - err := controllerutil.SetControllerReference(&dpa, registryDeployment, r.Scheme) - if err != nil { - return err + deleteContext := context.Background() + if err := r.Get(deleteContext, types.NamespacedName{ + Name: registryDeployment.Name, + Namespace: r.NamespacedName.Namespace, + }, registryDeployment); err != nil { + if k8serror.IsNotFound(err) { + return true, nil } - // update the Registry Deployment template - err = r.buildRegistryDeployment(registryDeployment, &bsl, &dpa) - return err - }) - - 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), - ) + deleteOptionPropagationForeground := metav1.DeletePropagationForeground + if err := r.Delete(deleteContext, registryDeployment, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { + r.EventRecorder.Event(registryDeployment, corev1.EventTypeNormal, "DeleteRegistryDeploymentFailed", fmt.Sprint("Could not delete registry deployment %s from %s:"+err.Error(), registryDeployment.Name, registryDeployment.Namespace)) + return false, err } - + r.EventRecorder.Event(registryDeployment, corev1.EventTypeNormal, "DeletedRegistryDeployment", fmt.Sprintf("Registry Deployment %s deleted from %s", registryDeployment.Name, registryDeployment.Namespace)) } return true, nil } -// Construct and update the registry deployment for a bsl -func (r *DPAReconciler) buildRegistryDeployment(registryDeployment *appsv1.Deployment, bsl *velerov1.BackupStorageLocation, dpa *oadpv1alpha1.DataProtectionApplication) error { - - // Build registry container - registryContainer, err := r.buildRegistryContainer(bsl, dpa) - if err != nil { - return err - } - // Setting controller owner reference on the registry deployment - registryDeployment.Labels = r.getRegistryBSLLabels(bsl) - - registryDeployment.Spec = appsv1.DeploymentSpec{ - Selector: registryDeployment.Spec.Selector, - Replicas: pointer.Int32(1), - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "component": registryName(bsl), - }, - Annotations: dpa.Spec.PodAnnotations, - }, - Spec: corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyAlways, - Containers: registryContainer, - }, - }, - } - - // attach secret volume for cloud providers - if _, ok := bsl.Spec.Config["credentialsFile"]; ok { - if secretName, err := credentials.GetSecretNameFromCredentialsFileConfigString(bsl.Spec.Config["credentialsFile"]); err == nil { - registryDeployment.Spec.Template.Spec.Volumes = append( - registryDeployment.Spec.Template.Spec.Volumes, - corev1.Volume{ - Name: secretName, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: secretName, - }, - }, - }, - ) - } - } else if bsl.Spec.Provider == GCPProvider { - // check for secret name - secretName, _ := r.getSecretNameAndKey(&bsl.Spec, oadpv1alpha1.DefaultPluginGCP) - registryDeployment.Spec.Template.Spec.Volumes = []corev1.Volume{ - { - Name: secretName, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: secretName, - }, - }, - }, - } - } - - // attach DNS policy and config if enabled - registryDeployment.Spec.Template.Spec.DNSPolicy = dpa.Spec.PodDnsPolicy - if !reflect.DeepEqual(dpa.Spec.PodDnsConfig, corev1.PodDNSConfig{}) { - registryDeployment.Spec.Template.Spec.DNSConfig = &dpa.Spec.PodDnsConfig - } - - return nil -} - -func (r *DPAReconciler) getRegistryBSLLabels(bsl *velerov1.BackupStorageLocation) map[string]string { - labels := getAppLabels(registryName(bsl)) - labels["app.kubernetes.io/name"] = common.OADPOperatorVelero - labels["app.kubernetes.io/component"] = Registry - labels[oadpv1alpha1.RegistryDeploymentLabel] = "True" - return labels -} - func registryName(bsl *velerov1.BackupStorageLocation) string { return "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry" } -func getRegistryImage(dpa *oadpv1alpha1.DataProtectionApplication) string { - if dpa.Spec.UnsupportedOverrides[oadpv1alpha1.RegistryImageKey] != "" { - return dpa.Spec.UnsupportedOverrides[oadpv1alpha1.RegistryImageKey] - } - if os.Getenv("VELERO_REGISTRY_REPO") == "" { - return common.RegistryImage - } - return fmt.Sprintf("%v/%v/%v:%v", os.Getenv("REGISTRY"), os.Getenv("PROJECT"), os.Getenv("VELERO_REGISTRY_REPO"), os.Getenv("VELERO_REGISTRY_TAG")) -} - -func (r *DPAReconciler) buildRegistryContainer(bsl *velerov1.BackupStorageLocation, dpa *oadpv1alpha1.DataProtectionApplication) ([]corev1.Container, error) { - envVars, err := r.getRegistryEnvVars(bsl) - if err != nil { - r.Log.Info(fmt.Sprintf("Error building registry container for backupstoragelocation %s/%s, could not fetch registry env vars", bsl.Namespace, bsl.Name)) - return nil, err - } - containers := []corev1.Container{ - { - Image: getRegistryImage(dpa), - Name: registryName(bsl) + "-container", - Ports: []corev1.ContainerPort{ - { - ContainerPort: 5000, - Protocol: corev1.ProtocolTCP, - }, - }, - Env: envVars, - LivenessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/v2/_catalog?n=5", - Port: intstr.IntOrString{IntVal: 5000}, - }, - }, - PeriodSeconds: 10, - TimeoutSeconds: 10, - InitialDelaySeconds: 15, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/v2/_catalog?n=5", - Port: intstr.IntOrString{IntVal: 5000}, - }, - }, - PeriodSeconds: 10, - TimeoutSeconds: 10, - InitialDelaySeconds: 15, - }, - }, - } - - // check for secret name - if _, ok := bsl.Spec.Config["credentialsFile"]; ok { // If credentialsFile config is used, then mount the bsl secret - if secretName, err := credentials.GetSecretNameFromCredentialsFileConfigString(bsl.Spec.Config["credentialsFile"]); err == nil { - if _, bslCredOk := credentials.PluginSpecificFields[oadpv1alpha1.DefaultPlugin(bsl.Spec.Provider)]; bslCredOk { - containers[0].VolumeMounts = []corev1.VolumeMount{ - { - Name: secretName, - MountPath: credentials.PluginSpecificFields[oadpv1alpha1.DefaultPlugin(bsl.Spec.Provider)].BslMountPath, - }, - } - } - } - } else if bsl.Spec.Provider == GCPProvider { // append secret volumes if the BSL provider is GCP - secretName, _ := r.getSecretNameAndKey(&bsl.Spec, oadpv1alpha1.DefaultPluginGCP) - containers[0].VolumeMounts = []corev1.VolumeMount{ - { - Name: secretName, - MountPath: credentials.PluginSpecificFields[oadpv1alpha1.DefaultPluginGCP].MountPath, - }, - } - } - - return containers, nil -} - -func (r *DPAReconciler) getRegistryEnvVars(bsl *velerov1.BackupStorageLocation) ([]corev1.EnvVar, error) { - envVar := []corev1.EnvVar{} - provider := bsl.Spec.Provider - var err error - switch provider { - case AWSProvider: - envVar, err = r.getAWSRegistryEnvVars(bsl, cloudProviderEnvVarMap[AWSProvider]) - - case AzureProvider: - envVar, err = r.getAzureRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider]) - - case GCPProvider: - envVar, err = r.getGCPRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider]) - } - if err != nil { - return nil, err - } - // Appending proxy env vars - envVar = append(envVar, proxy.ReadProxyVarsFromEnv()...) - return envVar, nil -} - -func (r *DPAReconciler) getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation, awsEnvVars []corev1.EnvVar) ([]corev1.EnvVar, error) { - - // create secret data and fill up the values and return from here - for i := range awsEnvVars { - if awsEnvVars[i].Name == RegistryStorageS3AccesskeyEnvVarKey { - awsEnvVars[i].ValueFrom = &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, - Key: "access_key", - }, - } - } - - if awsEnvVars[i].Name == RegistryStorageS3BucketEnvVarKey { - awsEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket - } - - if awsEnvVars[i].Name == RegistryStorageS3RegionEnvVarKey { - bslSpecRegion, regionInConfig := bsl.Spec.Config[Region] - if regionInConfig { - awsEnvVars[i].Value = bslSpecRegion - } else { - r.Log.Info("region not found in backupstoragelocation spec") - } - } - - if awsEnvVars[i].Name == RegistryStorageS3SecretkeyEnvVarKey { - awsEnvVars[i].ValueFrom = &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, - Key: "secret_key", - }, - } - } - - if awsEnvVars[i].Name == RegistryStorageS3RegionendpointEnvVarKey { - awsEnvVars[i].Value = bsl.Spec.Config[S3URL] - } - - if awsEnvVars[i].Name == RegistryStorageS3SkipverifyEnvVarKey { - awsEnvVars[i].Value = bsl.Spec.Config[InsecureSkipTLSVerify] - } - - } - return awsEnvVars, nil -} - -func (r *DPAReconciler) getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars []corev1.EnvVar) ([]corev1.EnvVar, error) { - - for i := range azureEnvVars { - if azureEnvVars[i].Name == RegistryStorageAzureContainerEnvVarKey { - azureEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket - } - - if azureEnvVars[i].Name == RegistryStorageAzureAccountnameEnvVarKey { - azureEnvVars[i].Value = bsl.Spec.Config[StorageAccount] - } - - if azureEnvVars[i].Name == RegistryStorageAzureAccountkeyEnvVarKey { - azureEnvVars[i].ValueFrom = &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, - Key: "storage_account_key", - }, - } - } - if azureEnvVars[i].Name == RegistryStorageAzureSPNClientIDEnvVarKey { - azureEnvVars[i].ValueFrom = &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, - Key: "client_id_key", - }, - } - } - - if azureEnvVars[i].Name == RegistryStorageAzureSPNClientSecretEnvVarKey { - azureEnvVars[i].ValueFrom = &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, - Key: "client_secret_key", - }, - } - } - if azureEnvVars[i].Name == RegistryStorageAzureSPNTenantIDEnvVarKey { - azureEnvVars[i].ValueFrom = &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, - Key: "tenant_id_key", - }, - } - } - } - return azureEnvVars, nil -} - -func (r *DPAReconciler) getGCPRegistryEnvVars(bsl *velerov1.BackupStorageLocation, gcpEnvVars []corev1.EnvVar) ([]corev1.EnvVar, error) { - for i := range gcpEnvVars { - if gcpEnvVars[i].Name == RegistryStorageGCSBucket { - gcpEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket - } - - if gcpEnvVars[i].Name == RegistryStorageGCSKeyfile { - // check for secret name - _, secretKey := r.getSecretNameAndKey(&bsl.Spec, oadpv1alpha1.DefaultPluginGCP) - if _, ok := bsl.Spec.Config["credentialsFile"]; ok { - gcpEnvVars[i].Value = credentials.PluginSpecificFields[oadpv1alpha1.DefaultPluginGCP].BslMountPath + "/" + secretKey - } else { - gcpEnvVars[i].Value = credentials.PluginSpecificFields[oadpv1alpha1.DefaultPluginGCP].MountPath + "/" + secretKey - } - } - } - return gcpEnvVars, nil -} - func (r *DPAReconciler) getProviderSecret(secretName string) (corev1.Secret, error) { secret := corev1.Secret{} @@ -823,7 +489,7 @@ func (r *DPAReconciler) ReconcileRegistrySVCs(log logr.Logger) (bool, error) { return false, err } - // Now for each of these bsl instances, create a service + // Now for each of these bsl instances, delete any existing service if len(bslList.Items) > 0 { for _, bsl := range bslList.Items { svc := corev1.Service{ @@ -832,83 +498,28 @@ func (r *DPAReconciler) ReconcileRegistrySVCs(log logr.Logger) (bool, error) { Namespace: r.NamespacedName.Namespace, }, } - - if !dpa.BackupImages() { - deleteContext := context.Background() - if err := r.Get(deleteContext, types.NamespacedName{ - Name: svc.Name, - Namespace: r.NamespacedName.Namespace, - }, &svc); err != nil { - if k8serror.IsNotFound(err) { - return true, nil - } - return false, err - } - - deleteOptionPropagationForeground := metav1.DeletePropagationForeground - if err := r.Delete(deleteContext, &svc, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { - r.EventRecorder.Event(&svc, corev1.EventTypeNormal, "DeleteRegistryServiceFailed", "Could not delete registry service:"+err.Error()) - return false, err + deleteContext := context.Background() + if err := r.Get(deleteContext, types.NamespacedName{ + Name: svc.Name, + Namespace: r.NamespacedName.Namespace, + }, &svc); err != nil { + if k8serror.IsNotFound(err) { + return true, nil } - r.EventRecorder.Event(&svc, corev1.EventTypeNormal, "DeletedRegistryService", "Registry service deleted") - - return true, nil - } - - // Create SVC - op, err := controllerutil.CreateOrUpdate(r.Context, r.Client, &svc, func() error { - // TODO: check for svc status condition errors and respond here - err := r.updateRegistrySVC(&svc, &bsl, &dpa) - - return err - }) - if err != nil { return false, err } - if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - // Trigger event to indicate SVC was created or updated - r.EventRecorder.Event(&bsl, - corev1.EventTypeNormal, - "RegistryServicesReconciled", - fmt.Sprintf("performed %s on service %s/%s", op, svc.Namespace, svc.Name), - ) + deleteOptionPropagationForeground := metav1.DeletePropagationForeground + if err := r.Delete(deleteContext, &svc, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { + r.EventRecorder.Event(&svc, corev1.EventTypeNormal, "DeleteRegistryServiceFailed", fmt.Sprintf("Could not delete registry service %s from %s:"+err.Error(), svc.Name, svc.Namespace)) + return false, err } + r.EventRecorder.Event(&svc, corev1.EventTypeNormal, "DeletedRegistryService", fmt.Sprintf("Registry service %s deleted from %s", svc.Name, svc.Namespace)) } } return true, nil } -func (r *DPAReconciler) updateRegistrySVC(svc *corev1.Service, bsl *velerov1.BackupStorageLocation, dpa *oadpv1alpha1.DataProtectionApplication) error { - // Setting controller owner reference on the registry svc - err := controllerutil.SetControllerReference(dpa, svc, r.Scheme) - if err != nil { - return err - } - - // when updating the spec fields we update each field individually - // to get around the immutable fields - svc.Spec.Selector = map[string]string{ - "component": "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry", - } - svc.Spec.Type = corev1.ServiceTypeClusterIP - svc.Spec.Ports = []corev1.ServicePort{ - { - Name: "5000-tcp", - Port: int32(5000), - TargetPort: intstr.IntOrString{ - IntVal: int32(5000), - }, - Protocol: corev1.ProtocolTCP, - }, - } - svc.Labels = map[string]string{ - oadpv1alpha1.OadpOperatorLabel: "True", - "component": "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry", - } - return nil -} - func (r *DPAReconciler) ReconcileRegistryRoutes(log logr.Logger) (bool, error) { dpa := oadpv1alpha1.DataProtectionApplication{} if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil { @@ -942,161 +553,61 @@ func (r *DPAReconciler) ReconcileRegistryRoutes(log logr.Logger) (bool, error) { }, } - if !dpa.BackupImages() { - deleteContext := context.Background() - if err := r.Get(deleteContext, types.NamespacedName{ - Name: route.Name, - Namespace: r.NamespacedName.Namespace, - }, &route); err != nil { - if k8serror.IsNotFound(err) { - return true, nil - } - return false, err - } - - deleteOptionPropagationForeground := metav1.DeletePropagationForeground - if err := r.Delete(deleteContext, &route, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { - r.EventRecorder.Event(&route, corev1.EventTypeNormal, "DeleteRegistryRouteFailed", "Could not delete registry route:"+err.Error()) - return false, err + deleteContext := context.Background() + if err := r.Get(deleteContext, types.NamespacedName{ + Name: route.Name, + Namespace: r.NamespacedName.Namespace, + }, &route); err != nil { + if k8serror.IsNotFound(err) { + return true, nil } - r.EventRecorder.Event(&route, corev1.EventTypeNormal, "DeletedRegistryRoute", "Registry route deleted") - - return true, nil + return false, err } - // Create Route - op, err := controllerutil.CreateOrUpdate(r.Context, r.Client, &route, func() error { - - // TODO: check for svc status condition errors and respond here - - err := r.updateRegistryRoute(&route, &bsl, &dpa) - - return err - }) - if err != nil { + deleteOptionPropagationForeground := metav1.DeletePropagationForeground + if err := r.Delete(deleteContext, &route, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { + r.EventRecorder.Event(&route, corev1.EventTypeNormal, "DeleteRegistryRouteFailed", fmt.Sprintf("Could not delete registry route %s from %s:"+err.Error(), route.Name, route.Namespace)) return false, err } - if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - // Trigger event to indicate route was created or updated - r.EventRecorder.Event(&bsl, - corev1.EventTypeNormal, - "RegistryroutesReconciled", - fmt.Sprintf("performed %s on route %s/%s", op, route.Namespace, route.Name), - ) - } + r.EventRecorder.Event(&route, corev1.EventTypeNormal, "DeletedRegistryRoute", fmt.Sprintf("Registry route %s deleted from %s", route.Name, route.Namespace)) } } return true, nil } -func (r *DPAReconciler) updateRegistryRoute(route *routev1.Route, bsl *velerov1.BackupStorageLocation, dpa *oadpv1alpha1.DataProtectionApplication) error { - // Setting controller owner reference on the registry route - err := controllerutil.SetControllerReference(dpa, route, r.Scheme) - if err != nil { - return err - } +func (r *DPAReconciler) ReconcileRegistryRouteConfigs(log logr.Logger) (bool, error) { - route.Labels = map[string]string{ - oadpv1alpha1.OadpOperatorLabel: "True", - "component": "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry", - "service": "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-svc", - "track": "registry-routes", + // Now for each of these bsl instances, create a registry route cm for each of them + registryRouteCM := corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-registry-config", + Namespace: r.NamespacedName.Namespace, + }, } - return nil -} - -func (r *DPAReconciler) ReconcileRegistryRouteConfigs(log logr.Logger) (bool, error) { - dpa := oadpv1alpha1.DataProtectionApplication{} - if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil { + deleteContext := context.Background() + if err := r.Get(deleteContext, types.NamespacedName{ + Name: registryRouteCM.Name, + Namespace: r.NamespacedName.Namespace, + }, ®istryRouteCM); err != nil { + if k8serror.IsNotFound(err) { + return true, nil + } return false, err } - // fetch the bsl instances - bslList := velerov1.BackupStorageLocationList{} - if err := r.List(r.Context, &bslList, &client.ListOptions{ - Namespace: r.NamespacedName.Namespace, - LabelSelector: labels.SelectorFromSet(map[string]string{ - "app.kubernetes.io/component": "bsl", - }), - }); err != nil { + deleteOptionPropagationForeground := metav1.DeletePropagationForeground + if err := r.Delete(deleteContext, ®istryRouteCM, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { + r.EventRecorder.Event(®istryRouteCM, corev1.EventTypeNormal, "DeleteRegistryConfigMapFailed", fmt.Sprintf("Could not delete registry configmap %s from %s:"+err.Error(), registryRouteCM.Name, registryRouteCM.Namespace)) return false, err } + r.EventRecorder.Event(®istryRouteCM, corev1.EventTypeNormal, "DeletedRegistryConfigMap", fmt.Sprintf("Registry configmap %s deleted from %s", registryRouteCM.Name, registryRouteCM.Namespace)) - // Now for each of these bsl instances, create a registry route cm for each of them - if len(bslList.Items) > 0 { - for _, bsl := range bslList.Items { - registryRouteCM := corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-registry-config", - Namespace: r.NamespacedName.Namespace, - }, - } - - if !dpa.BackupImages() { - deleteContext := context.Background() - if err := r.Get(deleteContext, types.NamespacedName{ - Name: registryRouteCM.Name, - Namespace: r.NamespacedName.Namespace, - }, ®istryRouteCM); err != nil { - if k8serror.IsNotFound(err) { - return true, nil - } - return false, err - } - - deleteOptionPropagationForeground := metav1.DeletePropagationForeground - if err := r.Delete(deleteContext, ®istryRouteCM, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { - r.EventRecorder.Event(®istryRouteCM, corev1.EventTypeNormal, "DeleteRegistryConfigMapFailed", "Could not delete registry configmap:"+err.Error()) - return false, err - } - r.EventRecorder.Event(®istryRouteCM, corev1.EventTypeNormal, "DeletedRegistryConfigMap", "Registry configmap deleted") - - return true, nil - } - - op, err := controllerutil.CreateOrUpdate(r.Context, r.Client, ®istryRouteCM, func() error { - - // update the Config Map - err := r.updateRegistryConfigMap(®istryRouteCM, &bsl, &dpa) - return err - }) - - if err != nil { - return false, err - } - - //TODO: Review Registry Route CM status and report errors and conditions - - if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - // Trigger event to indicate Registry Route CM was created or updated - r.EventRecorder.Event(®istryRouteCM, - corev1.EventTypeNormal, - "ReconcileRegistryRouteConfigReconciled", - fmt.Sprintf("performed %s on registry route config map %s/%s", op, registryRouteCM.Namespace, registryRouteCM.Name), - ) - } - } - } return true, nil } -func (r *DPAReconciler) updateRegistryConfigMap(registryRouteCM *corev1.ConfigMap, bsl *velerov1.BackupStorageLocation, dpa *oadpv1alpha1.DataProtectionApplication) error { - - // Setting controller owner reference on the restic restore helper CM - err := controllerutil.SetControllerReference(dpa, registryRouteCM, r.Scheme) - if err != nil { - return err - } - - registryRouteCM.Data = map[string]string{ - bsl.Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-route", - } - - return nil -} - +// Create secret for registry to be parsed by openshift-velero-plugin func (r *DPAReconciler) ReconcileRegistrySecrets(log logr.Logger) (bool, error) { dpa := oadpv1alpha1.DataProtectionApplication{} if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil { @@ -1144,10 +655,10 @@ func (r *DPAReconciler) ReconcileRegistrySecrets(log logr.Logger) (bool, error) deleteOptionPropagationForeground := metav1.DeletePropagationForeground if err := r.Delete(deleteContext, &secret, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { - r.EventRecorder.Event(&secret, corev1.EventTypeNormal, "DeleteRegistrySecretFailed", "Could not delete registry secret:"+err.Error()) + r.EventRecorder.Event(&secret, corev1.EventTypeNormal, "DeleteRegistrySecretFailed", fmt.Sprintf("Could not delete registry secret %s from %s:"+err.Error(), secret.Name, secret.Namespace)) return false, err } - r.EventRecorder.Event(&secret, corev1.EventTypeNormal, "DeletedRegistrySecret", "Registry secret deleted") + r.EventRecorder.Event(&secret, corev1.EventTypeNormal, "DeletedRegistrySecret", fmt.Sprintf("Registry secret %s deleted from %s", secret.Name, secret.Namespace)) return true, nil } diff --git a/controllers/registry_test.go b/controllers/registry_test.go index c18db824ed..a3767e7b7b 100644 --- a/controllers/registry_test.go +++ b/controllers/registry_test.go @@ -6,21 +6,14 @@ import ( "testing" "github.com/go-logr/logr" - routev1 "github.com/openshift/api/route/v1" oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1" - "github.com/openshift/oadp-operator/pkg/common" 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/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" - "k8s.io/utils/pointer" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func getSchemeForFakeClientForRegistry() (*runtime.Scheme, error) { @@ -37,15 +30,6 @@ func getSchemeForFakeClientForRegistry() (*runtime.Scheme, error) { return scheme.Scheme, nil } -func getFakeClientFromObjectsForRegistry(objs ...client.Object) (client.WithWatch, error) { - schemeForFakeClient, err := getSchemeForFakeClientForRegistry() - if err != nil { - return nil, err - } - - return fake.NewClientBuilder().WithScheme(schemeForFakeClient).WithObjects(objs...).Build(), nil -} - const ( testProfile = "someProfile" testAccessKey = "someAccessKey" @@ -162,42 +146,29 @@ var ( } ) -func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { +var testAWSEnvVar = cloudProviderEnvVarMap["aws"] +var testAzureEnvVar = cloudProviderEnvVarMap["azure"] +var testGCPEnvVar = cloudProviderEnvVarMap["gcp"] + +func TestDPAReconciler_getSecretNameAndKeyforBackupLocation(t *testing.T) { tests := []struct { - name string - registryDeployment *appsv1.Deployment - bsl *velerov1.BackupStorageLocation - secret *corev1.Secret - registrySecret *corev1.Secret - dpa *oadpv1alpha1.DataProtectionApplication - wantErr bool + name string + bsl *oadpv1alpha1.BackupLocation + secret *corev1.Secret + wantProfile string + wantSecretName string + wantSecretKey string }{ { - name: "given a valid bsl get appropriate registry deployment", - registryDeployment: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-ns", - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "aws" + "-registry", - }, - }, - }, - }, - bsl: &velerov1.BackupStorageLocation{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-bsl", - Namespace: "test-ns", - }, - Spec: velerov1.BackupStorageLocationSpec{ + name: "given provider secret, appropriate secret name and key are returned", + bsl: &oadpv1alpha1.BackupLocation{ + Velero: &velerov1.BackupStorageLocationSpec{ Provider: AWSProvider, - StorageType: velerov1.StorageType{ - ObjectStorage: &velerov1.ObjectStorageLocation{ - Bucket: "aws-bucket", + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials-aws", }, + Key: "cloud", }, Config: map[string]string{ Region: "aws-region", @@ -208,147 +179,19 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { }, secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", + Name: "cloud-credentials-aws", Namespace: "test-ns", }, Data: secretData, }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: awsRegistrySecretData, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{}, - }, - { - name: "given a valid bsl with equal in secret val get appropriate registry deployment", - registryDeployment: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-ns", - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "aws" + "-registry", - }, - }, - }, - }, - 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", - InsecureSkipTLSVerify: "false", - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", - Namespace: "test-ns", - }, - Data: secretDataWithEqualInSecret, - }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: awsRegistrySecretData, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{}, - }, - { - name: "given a valid bsl with carriageReturn in secret val get appropriate registry deployment", - registryDeployment: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-ns", - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "aws" + "-registry", - }, - }, - }, - }, - 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", - InsecureSkipTLSVerify: "false", - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", - Namespace: "test-ns", - }, - Data: secretDataWithCarriageReturnInSecret, - }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: secretDataWithCarriageReturnInSecret, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{}, + + wantProfile: "aws-provider", }, { - name: "given a valid bsl with use of quotes in secret val get appropriate registry deployment", - registryDeployment: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-ns", - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "aws" + "-registry", - }, - }, - }, - }, - bsl: &velerov1.BackupStorageLocation{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-bsl", - Namespace: "test-ns", - }, - Spec: velerov1.BackupStorageLocationSpec{ + name: "given no provider secret, appropriate secret name and key are returned", + bsl: &oadpv1alpha1.BackupLocation{ + Velero: &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", @@ -361,503 +204,59 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { Name: "cloud-credentials", Namespace: "test-ns", }, - Data: secretDataWithMixedQuotesAndSpacesInSecret, - }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: awsRegistrySecretData, + Data: secretData, }, - dpa: &oadpv1alpha1.DataProtectionApplication{}, + + wantProfile: "aws-provider-no-cred", }, { - name: "given a valid bsl with velero annotation get appropriate registry deployment", - registryDeployment: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-ns", - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "aws" + "-registry", - }, - }, - }, - }, - 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", + name: "given cloud storage secret, appropriate secret name and key are returned", + bsl: &oadpv1alpha1.BackupLocation{ + CloudStorage: &oadpv1alpha1.CloudStorageLocation{ + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials-aws", }, + Key: "cloud", }, - Config: map[string]string{ - Region: "aws-region", - S3URL: "https://sr-url-aws-domain.com", - InsecureSkipTLSVerify: "false", + CloudStorageRef: corev1.LocalObjectReference{ + Name: "example", }, }, }, secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", + Name: "cloud-credentials-aws", Namespace: "test-ns", }, Data: secretData, }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: awsRegistrySecretData, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{ - Spec: oadpv1alpha1.DataProtectionApplicationSpec{ - PodAnnotations: map[string]string{ - "test-annotation": "awesome-annotation", - }, - }, - }, + + wantProfile: "aws-cloud-cred", }, { - name: "given a valid bsl with velero annotation and DNS Policy/Config get appropriate registry deployment", - registryDeployment: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-ns", - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "aws" + "-registry", - }, - }, - }, - }, - 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", - InsecureSkipTLSVerify: "false", + name: "given no cloud storage secret, appropriate secret name and key are returned", + bsl: &oadpv1alpha1.BackupLocation{ + CloudStorage: &oadpv1alpha1.CloudStorageLocation{ + CloudStorageRef: corev1.LocalObjectReference{ + Name: "example", }, }, }, secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", + Name: "", Namespace: "test-ns", }, Data: secretData, }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: awsRegistrySecretData, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{ - Spec: oadpv1alpha1.DataProtectionApplicationSpec{ - PodAnnotations: map[string]string{ - "test-annotation": "awesome-annotation", - }, - PodDnsPolicy: "None", - PodDnsConfig: corev1.PodDNSConfig{ - Nameservers: []string{ - "1.1.1.1", - "8.8.8.8", - }, - Options: []corev1.PodDNSConfigOption{ - { - Name: "ndots", - Value: pointer.String("2"), - }, - { - Name: "edns0", - }, - }, - }, - }, - }, + + wantProfile: "aws-cloud-no-cred", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.registryDeployment, tt.bsl, tt.registrySecret) - if err != nil { - t.Errorf("error in creating fake client, likely programmer error") - } - r := &DPAReconciler{ - Client: fakeClient, - Scheme: fakeClient.Scheme(), - Log: logr.Discard(), - Context: newContextForTest(tt.name), - NamespacedName: types.NamespacedName{ - Namespace: tt.registryDeployment.Namespace, - Name: tt.registryDeployment.Name, - }, - EventRecorder: record.NewFakeRecorder(10), - } - wantRegistryDeployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-ns", - Labels: map[string]string{ - "app.kubernetes.io/name": common.OADPOperatorVelero, - "app.kubernetes.io/instance": "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry", - "app.kubernetes.io/managed-by": common.OADPOperator, - "app.kubernetes.io/component": Registry, - oadpv1alpha1.OadpOperatorLabel: "True", - oadpv1alpha1.RegistryDeploymentLabel: "True", - }, - 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), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "component": "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry", - }, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "component": "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry", - }, - Annotations: tt.dpa.Spec.PodAnnotations, - }, - Spec: corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyAlways, - DNSPolicy: tt.dpa.Spec.PodDnsPolicy, - DNSConfig: &tt.dpa.Spec.PodDnsConfig, - Containers: []corev1.Container{ - { - Image: getRegistryImage(tt.dpa), - Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry" + "-container", - Ports: []corev1.ContainerPort{ - { - ContainerPort: 5000, - Protocol: corev1.ProtocolTCP, - }, - }, - Env: []corev1.EnvVar{ - { - Name: RegistryStorageEnvVarKey, - Value: S3, - }, - { - Name: RegistryStorageS3AccesskeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "access_key", - }, - }, - }, - { - Name: RegistryStorageS3BucketEnvVarKey, - Value: "aws-bucket", - }, - { - Name: RegistryStorageS3RegionEnvVarKey, - Value: "aws-region", - }, - { - Name: RegistryStorageS3SecretkeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "secret_key", - }, - }, - }, - { - Name: RegistryStorageS3RegionendpointEnvVarKey, - Value: "https://sr-url-aws-domain.com", - }, - { - Name: RegistryStorageS3SkipverifyEnvVarKey, - Value: "false", - }, - }, - LivenessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/v2/_catalog?n=5", - Port: intstr.IntOrString{IntVal: 5000}, - }, - }, - PeriodSeconds: 10, - TimeoutSeconds: 10, - InitialDelaySeconds: 15, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/v2/_catalog?n=5", - Port: intstr.IntOrString{IntVal: 5000}, - }, - }, - PeriodSeconds: 10, - TimeoutSeconds: 10, - InitialDelaySeconds: 15, - }, - }, - }, - }, - }, - }, - } - if reflect.DeepEqual(tt.dpa.Spec.PodDnsConfig, corev1.PodDNSConfig{}) { - wantRegistryDeployment.Spec.Template.Spec.DNSConfig = nil - } - - err = r.buildRegistryDeployment(tt.registryDeployment, tt.bsl, tt.dpa) - 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.Spec, tt.registryDeployment.Spec) { - t.Errorf("expected registry deployment spec to be %#v, got %#v", wantRegistryDeployment, tt.registryDeployment) - } - }) - } -} - -func TestDPAReconciler_buildRegistryContainer(t *testing.T) { - tests := []struct { - name string - bsl *velerov1.BackupStorageLocation - dpa *oadpv1alpha1.DataProtectionApplication - wantRegistryContainer *corev1.Container - wantErr bool - }{ - { - name: "given bsl appropriate container is built or not", - bsl: &velerov1.BackupStorageLocation{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-bsl", - Namespace: "test-ns", - }, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "Velero-test-CR", - 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 := &DPAReconciler{ - Scheme: scheme, - } - tt.wantRegistryContainer = &corev1.Container{ - Image: getRegistryImage(tt.dpa), - 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: 10, - TimeoutSeconds: 10, - InitialDelaySeconds: 15, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/v2/_catalog?n=5", - Port: intstr.IntOrString{IntVal: 5000}, - }, - }, - PeriodSeconds: 10, - TimeoutSeconds: 10, - InitialDelaySeconds: 15, - }, - } - - gotRegistryContainer, gotErr := r.buildRegistryContainer(tt.bsl, tt.dpa) - - if (gotErr != nil) != tt.wantErr { - t.Errorf("ValidateBackupStorageLocations() gotErr = %v, wantErr %v", gotErr, tt.wantErr) - return - } - - 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(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(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(tt.wantRegistryContainer.LivenessProbe, gotRegistryContainer[0].LivenessProbe) { - t.Errorf("expected registry container liveness probe to be %#v, got %#v", tt.wantRegistryContainer.LivenessProbe, gotRegistryContainer[0].LivenessProbe) - } - - }) - } -} - -var testAWSEnvVar = cloudProviderEnvVarMap["aws"] -var testAzureEnvVar = cloudProviderEnvVarMap["azure"] -var testGCPEnvVar = cloudProviderEnvVarMap["gcp"] - -func TestDPAReconciler_getSecretNameAndKeyforBackupLocation(t *testing.T) { - tests := []struct { - name string - bsl *oadpv1alpha1.BackupLocation - secret *corev1.Secret - wantProfile string - wantSecretName string - wantSecretKey string - }{ - { - name: "given provider secret, appropriate secret name and key are returned", - bsl: &oadpv1alpha1.BackupLocation{ - Velero: &velerov1.BackupStorageLocationSpec{ - Provider: AWSProvider, - Credential: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "cloud-credentials-aws", - }, - Key: "cloud", - }, - Config: map[string]string{ - Region: "aws-region", - S3URL: "https://sr-url-aws-domain.com", - InsecureSkipTLSVerify: "false", - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials-aws", - Namespace: "test-ns", - }, - Data: secretData, - }, - - wantProfile: "aws-provider", - }, - { - name: "given no provider secret, appropriate secret name and key are returned", - bsl: &oadpv1alpha1.BackupLocation{ - Velero: &velerov1.BackupStorageLocationSpec{ - Provider: AWSProvider, - Config: map[string]string{ - Region: "aws-region", - S3URL: "https://sr-url-aws-domain.com", - InsecureSkipTLSVerify: "false", - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", - Namespace: "test-ns", - }, - Data: secretData, - }, - - wantProfile: "aws-provider-no-cred", - }, - { - name: "given cloud storage secret, appropriate secret name and key are returned", - bsl: &oadpv1alpha1.BackupLocation{ - CloudStorage: &oadpv1alpha1.CloudStorageLocation{ - Credential: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "cloud-credentials-aws", - }, - Key: "cloud", - }, - CloudStorageRef: corev1.LocalObjectReference{ - Name: "example", - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials-aws", - Namespace: "test-ns", - }, - Data: secretData, - }, - - wantProfile: "aws-cloud-cred", - }, - { - name: "given no cloud storage secret, appropriate secret name and key are returned", - bsl: &oadpv1alpha1.BackupLocation{ - CloudStorage: &oadpv1alpha1.CloudStorageLocation{ - CloudStorageRef: corev1.LocalObjectReference{ - Name: "example", - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "", - Namespace: "test-ns", - }, - Data: secretData, - }, - - wantProfile: "aws-cloud-no-cred", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjects(tt.secret) + fakeClient, err := getFakeClientFromObjects(tt.secret) if err != nil { t.Errorf("error in creating fake client, likely programmer error") } @@ -896,839 +295,6 @@ func TestDPAReconciler_getSecretNameAndKeyforBackupLocation(t *testing.T) { }) } } -func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { - tests := []struct { - name string - bsl *velerov1.BackupStorageLocation - wantRegistryContainerEnvVar []corev1.EnvVar - wantProfile string - secret *corev1.Secret - registrySecret *corev1.Secret - wantErr bool - matchProfile 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", - InsecureSkipTLSVerify: "false", - Profile: "test-profile", - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", - Namespace: "test-ns", - }, - Data: secretData, - }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: awsRegistrySecretData, - }, - wantProfile: "test-profile", - matchProfile: true, - }, { - name: "given aws profile in 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", - InsecureSkipTLSVerify: "false", - Profile: testBslProfile, - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", - Namespace: "test-ns", - }, - Data: secretData, - }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: awsRegistrySecretData, - }, - wantProfile: testBslProfile, - matchProfile: true, - }, { - name: "given missing aws profile in bsl, env var should not match", - 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", - InsecureSkipTLSVerify: "false", - Profile: testBslProfile, - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials", - Namespace: "test-ns", - }, - Data: awsSecretDataWithMissingProfile, - }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-aws-registry-secret", - Namespace: "test-ns", - }, - Data: awsRegistrySecretData, - }, - wantProfile: testBslProfile, - matchProfile: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.bsl, tt.registrySecret) - if err != nil { - t.Errorf("error in creating fake client, likely programmer error") - } - r := &DPAReconciler{ - Client: fakeClient, - Scheme: fakeClient.Scheme(), - Log: logr.Discard(), - Context: newContextForTest(tt.name), - NamespacedName: types.NamespacedName{ - Namespace: tt.bsl.Namespace, - Name: tt.bsl.Name, - }, - EventRecorder: record.NewFakeRecorder(10), - } - tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ - { - Name: RegistryStorageEnvVarKey, - Value: S3, - }, - { - Name: RegistryStorageS3AccesskeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "access_key", - }, - }, - }, - { - Name: RegistryStorageS3BucketEnvVarKey, - Value: "aws-bucket", - }, - { - Name: RegistryStorageS3RegionEnvVarKey, - Value: "aws-region", - }, - { - Name: RegistryStorageS3SecretkeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "secret_key", - }, - }, - }, - { - Name: RegistryStorageS3RegionendpointEnvVarKey, - Value: "https://sr-url-aws-domain.com", - }, - { - Name: RegistryStorageS3SkipverifyEnvVarKey, - Value: "false", - }, - } - if tt.wantProfile == testBslProfile { - tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ - { - Name: RegistryStorageEnvVarKey, - Value: S3, - }, - { - Name: RegistryStorageS3AccesskeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "access_key", - }, - }, - }, - { - Name: RegistryStorageS3BucketEnvVarKey, - Value: "aws-bucket", - }, - { - Name: RegistryStorageS3RegionEnvVarKey, - Value: "aws-region", - }, - { - Name: RegistryStorageS3SecretkeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "secret_key", - }, - }, - }, - { - Name: RegistryStorageS3RegionendpointEnvVarKey, - Value: "https://sr-url-aws-domain.com", - }, - { - Name: RegistryStorageS3SkipverifyEnvVarKey, - Value: "false", - }, - } - } - - gotRegistryContainerEnvVar, gotErr := r.getAWSRegistryEnvVars(tt.bsl, testAWSEnvVar) - - if tt.matchProfile && (gotErr != nil) != tt.wantErr { - t.Errorf("ValidateBackupStorageLocations() gotErr = %v, wantErr %v", gotErr, tt.wantErr) - return - } - - if tt.matchProfile && !reflect.DeepEqual(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { - t.Errorf("expected registry container env var to be %#v, got %#v", tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) - } - }) - } -} - -func TestDPAReconciler_getAzureRegistryEnvVars(t *testing.T) { - tests := []struct { - name string - bsl *velerov1.BackupStorageLocation - wantRegistryContainerEnvVar []corev1.EnvVar - secret *corev1.Secret - registrySecret *corev1.Secret - wantErr bool - wantProfile string - matchProfile 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", - ResourceGroup: testResourceGroup, - RegistryStorageAzureAccountnameEnvVarKey: "velero-azure-account", - "storageAccountKeyEnvVar": "AZURE_STORAGE_ACCOUNT_ACCESS_KEY", - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials-azure", - Namespace: "test-ns", - }, - Data: secretAzureData, - }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-azure-registry-secret", - Namespace: "test-ns", - }, - Data: azureRegistrySecretData, - }, - wantProfile: "test-profile", - matchProfile: true, - }, - { - name: "given azure bsl & SP credentials, 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", - ResourceGroup: testResourceGroup, - "subscriptionId": testSubscriptionID, - }, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials-azure", - Namespace: "test-ns", - }, - Data: secretAzureServicePrincipalData, - }, - registrySecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-test-bsl-azure-registry-secret", - Namespace: "test-ns", - }, - Data: azureRegistrySPSecretData, - }, - wantProfile: "test-sp-profile", - matchProfile: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.bsl, tt.registrySecret) - if err != nil { - t.Errorf("error in creating fake client, likely programmer error") - } - r := &DPAReconciler{ - Client: fakeClient, - Scheme: fakeClient.Scheme(), - Log: logr.Discard(), - Context: newContextForTest(tt.name), - NamespacedName: types.NamespacedName{ - Namespace: tt.bsl.Namespace, - Name: tt.bsl.Name, - }, - EventRecorder: record.NewFakeRecorder(10), - } - tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ - { - Name: RegistryStorageEnvVarKey, - Value: Azure, - }, - { - Name: RegistryStorageAzureContainerEnvVarKey, - Value: "azure-bucket", - }, - { - Name: RegistryStorageAzureAccountnameEnvVarKey, - Value: "velero-azure-account", - }, - { - Name: RegistryStorageAzureAccountkeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "storage_account_key", - }, - }, - }, - { - Name: RegistryStorageAzureAADEndpointEnvVarKey, - Value: "", - }, - { - Name: RegistryStorageAzureSPNClientIDEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "client_id_key", - }, - }, - }, - { - Name: RegistryStorageAzureSPNClientSecretEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "client_secret_key", - }, - }, - }, - { - Name: RegistryStorageAzureSPNTenantIDEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "tenant_id_key", - }, - }, - }, - } - if tt.wantProfile == "test-sp-profile" { - tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ - { - Name: RegistryStorageEnvVarKey, - Value: Azure, - }, - { - Name: RegistryStorageAzureContainerEnvVarKey, - Value: "azure-bucket", - }, - { - Name: RegistryStorageAzureAccountnameEnvVarKey, - Value: "velero-azure-account", - }, - { - Name: RegistryStorageAzureAccountkeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "storage_account_key", - }, - }, - }, - { - Name: RegistryStorageAzureAADEndpointEnvVarKey, - Value: "", - }, - { - Name: RegistryStorageAzureSPNClientIDEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "client_id_key", - }, - }, - }, - { - Name: RegistryStorageAzureSPNClientSecretEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "client_secret_key", - }, - }, - }, - { - Name: RegistryStorageAzureSPNTenantIDEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "tenant_id_key", - }, - }, - }, - } - } - - gotRegistryContainerEnvVar, gotErr := r.getAzureRegistryEnvVars(tt.bsl, testAzureEnvVar) - - if tt.matchProfile && (gotErr != nil) != tt.wantErr { - t.Errorf("ValidateBackupStorageLocations() gotErr = %v, wantErr %v", gotErr, tt.wantErr) - return - } - - if tt.matchProfile && !reflect.DeepEqual(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { - t.Errorf("expected registry container env var to be %#v, got %#v", tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) - } - }) - } -} - -func TestDPAReconciler_getGCPRegistryEnvVars(t *testing.T) { - tests := []struct { - name string - bsl *velerov1.BackupStorageLocation - wantRegistryContainerEnvVar []corev1.EnvVar - secret *corev1.Secret - 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{}, - }, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cloud-credentials-gcp", - Namespace: "test-ns", - }, - Data: secretData, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.bsl) - if err != nil { - t.Errorf("error in creating fake client, likely programmer error") - } - r := &DPAReconciler{ - Client: fakeClient, - Scheme: fakeClient.Scheme(), - Log: logr.Discard(), - Context: newContextForTest(tt.name), - NamespacedName: types.NamespacedName{ - Namespace: tt.bsl.Namespace, - Name: tt.bsl.Name, - }, - EventRecorder: record.NewFakeRecorder(10), - } - tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ - { - Name: RegistryStorageEnvVarKey, - Value: GCS, - }, - { - Name: RegistryStorageGCSBucket, - Value: "gcp-bucket", - }, - { - Name: RegistryStorageGCSKeyfile, - Value: "/credentials-gcp/cloud", - }, - } - - gotRegistryContainerEnvVar, gotErr := r.getGCPRegistryEnvVars(tt.bsl, testGCPEnvVar) - - if (gotErr != nil) != tt.wantErr { - t.Errorf("ValidateBackupStorageLocations() gotErr = %v, wantErr %v", gotErr, tt.wantErr) - return - } - - if !reflect.DeepEqual(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { - t.Errorf("expected registry container env var to be %#v, got %#v", tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) - } - }) - } -} - -func TestDPAReconciler_updateRegistrySVC(t *testing.T) { - tests := []struct { - name string - svc *corev1.Service - bsl *velerov1.BackupStorageLocation - dpa *oadpv1alpha1.DataProtectionApplication - wantErr bool - }{ - { - name: "Given Velero CR and BSL and SVC instance, appropriate registry svc gets updated", - wantErr: false, - bsl: &velerov1.BackupStorageLocation{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-bsl", - Namespace: "test-ns", - }, - Spec: velerov1.BackupStorageLocationSpec{ - Provider: "test-provider", - }, - }, - svc: &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry-svc", - Namespace: "test-ns", - }, - Spec: corev1.ServiceSpec{ - Selector: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry", - }, - Type: corev1.ServiceTypeClusterIP, - Ports: []corev1.ServicePort{ - { - Name: "5000-tcp", - Port: int32(5000), - TargetPort: intstr.IntOrString{ - IntVal: int32(5000), - }, - Protocol: corev1.ProtocolTCP, - }, - }, - }, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "Velero-test-CR", - Namespace: "test-ns", - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjects(tt.dpa) - if err != nil { - t.Errorf("error in creating fake client, likely programmer error") - } - r := &DPAReconciler{ - Client: fakeClient, - Scheme: fakeClient.Scheme(), - Log: logr.Discard(), - Context: newContextForTest(tt.name), - NamespacedName: types.NamespacedName{ - Namespace: tt.dpa.Namespace, - Name: tt.dpa.Name, - }, - EventRecorder: record.NewFakeRecorder(10), - } - wantSVC := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry-svc", - Namespace: "test-ns", - Labels: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry", - oadpv1alpha1.OadpOperatorLabel: "True", - }, - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: oadpv1alpha1.SchemeBuilder.GroupVersion.String(), - Kind: "DataProtectionApplication", - Name: tt.dpa.Name, - UID: tt.dpa.UID, - Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), - }}, - }, - Spec: corev1.ServiceSpec{ - Selector: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry", - }, - Ports: []corev1.ServicePort{ - { - Name: "5000-tcp", - Port: int32(5000), - TargetPort: intstr.IntOrString{ - IntVal: int32(5000), - }, - Protocol: corev1.ProtocolTCP, - }, - }, - Type: corev1.ServiceTypeClusterIP, - }, - } - if err := r.updateRegistrySVC(tt.svc, tt.bsl, tt.dpa); (err != nil) != tt.wantErr { - t.Errorf("updateRegistrySVC() error = %v, wantErr %v", err, tt.wantErr) - } - if !reflect.DeepEqual(tt.svc, wantSVC) { - t.Errorf("expected bsl labels to be %#v, got %#v", tt.svc, wantSVC) - } - }) - } -} - -func TestDPAReconciler_updateRegistryRoute(t *testing.T) { - tests := []struct { - name string - route *routev1.Route - bsl *velerov1.BackupStorageLocation - dpa *oadpv1alpha1.DataProtectionApplication - wantErr bool - }{ - { - name: "Given DPA CR and BSL and SVC instance, appropriate registry route gets updated", - wantErr: false, - bsl: &velerov1.BackupStorageLocation{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-bsl", - Namespace: "test-ns", - }, - Spec: velerov1.BackupStorageLocationSpec{ - Provider: "test-provider", - }, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "Velero-test-CR", - Namespace: "test-ns", - }, - }, - route: &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry-route", - Namespace: "test-ns", - }, - Spec: routev1.RouteSpec{ - To: routev1.RouteTargetReference{ - Kind: "Service", - Name: "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry-svc", - }, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjects() - if err != nil { - t.Errorf("error in creating fake client, likely programmer error") - } - r := &DPAReconciler{ - Client: fakeClient, - Scheme: fakeClient.Scheme(), - Log: logr.Discard(), - Context: newContextForTest(tt.name), - NamespacedName: types.NamespacedName{ - Namespace: tt.dpa.Namespace, - Name: tt.dpa.Name, - }, - EventRecorder: record.NewFakeRecorder(10), - } - wantRoute := &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry-route", - Namespace: "test-ns", - Labels: map[string]string{ - "component": "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry", - "service": "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry-svc", - "track": "registry-routes", - oadpv1alpha1.OadpOperatorLabel: "True", - }, - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: oadpv1alpha1.SchemeBuilder.GroupVersion.String(), - Kind: "DataProtectionApplication", - Name: tt.dpa.Name, - UID: tt.dpa.UID, - Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), - }}, - }, - Spec: routev1.RouteSpec{ - To: routev1.RouteTargetReference{ - Kind: "Service", - Name: "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry-svc", - }, - }, - } - if err := r.updateRegistryRoute(tt.route, tt.bsl, tt.dpa); (err != nil) != tt.wantErr { - t.Errorf("updateRegistryRoute() error = %v, wantErr %v", err, tt.wantErr) - } - if !reflect.DeepEqual(tt.route, wantRoute) { - t.Errorf("expected bsl labels to be %#v, got %#v", tt.route, wantRoute) - } - }) - } -} - -func TestDPAReconciler_updateRegistryRouteCM(t *testing.T) { - tests := []struct { - name string - registryRouteCM *corev1.ConfigMap - bsl *velerov1.BackupStorageLocation - dpa *oadpv1alpha1.DataProtectionApplication - wantErr bool - }{ - { - name: "Given Velero CR and bsl instance, appropriate registry cm is updated", - wantErr: false, - bsl: &velerov1.BackupStorageLocation{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-bsl", - Namespace: "test-ns", - }, - Spec: velerov1.BackupStorageLocationSpec{ - Provider: "test-provider", - }, - }, - dpa: &oadpv1alpha1.DataProtectionApplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "Velero-test-CR", - Namespace: "test-ns", - }, - }, - registryRouteCM: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-registry-config", - Namespace: "test-ns", - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjects() - if err != nil { - t.Errorf("error in creating fake client, likely programmer error") - } - r := &DPAReconciler{ - Client: fakeClient, - Scheme: fakeClient.Scheme(), - Log: logr.Discard(), - Context: newContextForTest(tt.name), - NamespacedName: types.NamespacedName{ - Namespace: tt.dpa.Namespace, - Name: tt.dpa.Name, - }, - EventRecorder: record.NewFakeRecorder(10), - } - wantRegitryRouteCM := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oadp-registry-config", - Namespace: "test-ns", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: oadpv1alpha1.SchemeBuilder.GroupVersion.String(), - Kind: "DataProtectionApplication", - Name: tt.dpa.Name, - UID: tt.dpa.UID, - Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), - }}, - }, - Data: map[string]string{ - "test-bsl": "oadp-" + "test-bsl" + "-" + "test-provider" + "-registry-route", - }, - } - if err := r.updateRegistryConfigMap(tt.registryRouteCM, tt.bsl, tt.dpa); (err != nil) != tt.wantErr { - t.Errorf("updateRegistryConfigMap() error = %v, wantErr %v", err, tt.wantErr) - } - if !reflect.DeepEqual(tt.registryRouteCM, wantRegitryRouteCM) { - t.Errorf("expected registry CM to be %#v, got %#v", tt.registryRouteCM, wantRegitryRouteCM) - } - }) - } -} func TestDPAReconciler_populateAWSRegistrySecret(t *testing.T) { diff --git a/controllers/velero.go b/controllers/velero.go index 144fad7a96..49db7087f0 100644 --- a/controllers/velero.go +++ b/controllers/velero.go @@ -584,6 +584,12 @@ func (r *DPAReconciler) customizeVeleroContainer(dpa *oadpv1alpha1.DataProtectio } // Append proxy settings to the container from environment variables veleroContainer.Env = append(veleroContainer.Env, proxy.ReadProxyVarsFromEnv()...) + if dpa.BackupImages() { + veleroContainer.Env = append(veleroContainer.Env, corev1.EnvVar{ + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }) + } // Check if data-mover is enabled and set the env var so that the csi data-mover code path is triggred if dpa.Spec.Features != nil && dpa.Spec.Features.EnableDataMover { diff --git a/controllers/velero_test.go b/controllers/velero_test.go index 2ebb54e69f..38e634549c 100644 --- a/controllers/velero_test.go +++ b/controllers/velero_test.go @@ -217,6 +217,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -405,6 +409,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, { Name: common.AWSSharedCredentialsFileEnvKey, Value: "/credentials/cloud", @@ -705,6 +713,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: strings.ToLower(proxyEnvKey), Value: proxyEnvValue, }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -881,6 +893,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -1095,6 +1111,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -1318,6 +1338,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -1499,6 +1523,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -1680,6 +1708,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -1861,6 +1893,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -2042,6 +2078,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -2244,6 +2284,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -2436,6 +2480,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, @@ -2613,6 +2661,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, { Name: common.AWSSharedCredentialsFileEnvKey, Value: "/credentials/cloud", @@ -2818,6 +2870,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, { Name: common.AWSSharedCredentialsFileEnvKey, Value: "/credentials/cloud", @@ -3040,6 +3096,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, { Name: common.AWSSharedCredentialsFileEnvKey, Value: "/credentials/cloud", @@ -3280,6 +3340,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, { Name: common.AWSSharedCredentialsFileEnvKey, Value: "/credentials/cloud", @@ -3503,6 +3567,10 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) { Name: common.LDLibraryPathEnvKey, Value: "/plugins", }, + { + Name: "OPENSHIFT_IMAGESTREAM_BACKUP", + Value: "true", + }, }, }, }, diff --git a/tests/e2e/backup_restore_suite_test.go b/tests/e2e/backup_restore_suite_test.go index 1a32dcc3e9..08a095948f 100755 --- a/tests/e2e/backup_restore_suite_test.go +++ b/tests/e2e/backup_restore_suite_test.go @@ -11,6 +11,7 @@ import ( . "github.com/onsi/gomega" . "github.com/openshift/oadp-operator/tests/e2e/lib" utils "github.com/openshift/oadp-operator/tests/e2e/utils" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -61,7 +62,12 @@ var _ = Describe("AWS backup restore tests", func() { }) var _ = AfterEach(func() { - err := dpaCR.Delete() + GinkgoWriter.Println("Printing velero deployment pod logs") + logs, err := GetVeleroContainerLogs(namespace) + Expect(err).NotTo(HaveOccurred()) + GinkgoWriter.Println(logs) + GinkgoWriter.Println("End of velero deployment pod logs") + err = dpaCR.Delete() Expect(err).ToNot(HaveOccurred()) }) @@ -102,6 +108,10 @@ var _ = Describe("AWS backup restore tests", func() { Expect(err).NotTo(HaveOccurred()) updateLastInstallingNamespace(dpaCR.Namespace) + if provider == "gcp" { + // disable image backup for GCP before https://github.com/openshift/openshift-velero-plugin/pull/151 is merged + dpaCR.CustomResource.Spec.BackupImages = pointer.Bool(false) + } err = dpaCR.CreateOrUpdate(&dpaCR.CustomResource.Spec) Expect(err).NotTo(HaveOccurred()) @@ -123,10 +133,8 @@ var _ = Describe("AWS backup restore tests", func() { } } - if dpaCR.CustomResource.BackupImages() { - log.Printf("Waiting for registry pods to be running") - Eventually(AreRegistryDeploymentsAvailable(namespace), timeoutMultiplier*time.Minute*3, time.Second*5).Should(BeTrue()) - } + // TODO: check registry deployments are deleted + // TODO: check S3 for images backupUid, _ := uuid.NewUUID() restoreUid, _ := uuid.NewUUID() @@ -255,7 +263,7 @@ var _ = Describe("AWS backup restore tests", func() { Entry("MySQL application CSI", Label("ibmcloud", "aws", "gcp"), BackupRestoreCase{ ApplicationTemplate: fmt.Sprintf("./sample-applications/mysql-persistent/mysql-persistent-csi-%s-template.yaml", provider), ApplicationNamespace: "mysql-persistent", - Name: "mysql-e2e", + Name: "mysql-csi-e2e", BackupRestoreType: CSI, PreBackupVerify: mysqlReady(true, CSI), PostRestoreVerify: mysqlReady(false, CSI), @@ -263,7 +271,7 @@ var _ = Describe("AWS backup restore tests", func() { Entry("Mongo application CSI", Label("ibmcloud", "aws", "gcp"), BackupRestoreCase{ ApplicationTemplate: fmt.Sprintf("./sample-applications/mongo-persistent/mongo-persistent-csi-%s-template.yaml", provider), ApplicationNamespace: "mongo-persistent", - Name: "mongo-e2e", + Name: "mongo-csi-e2e", BackupRestoreType: CSI, PreBackupVerify: mongoready(true, CSI), PostRestoreVerify: mongoready(false, CSI), @@ -271,7 +279,7 @@ var _ = Describe("AWS backup restore tests", func() { Entry("Mongo application RESTIC", BackupRestoreCase{ ApplicationTemplate: "./sample-applications/mongo-persistent/mongo-persistent.yaml", ApplicationNamespace: "mongo-persistent", - Name: "mongo-e2e", + Name: "mongo-restic-e2e", BackupRestoreType: RESTIC, PreBackupVerify: mongoready(false, RESTIC), PostRestoreVerify: mongoready(false, RESTIC), @@ -279,7 +287,7 @@ var _ = Describe("AWS backup restore tests", func() { Entry("MySQL application RESTIC", BackupRestoreCase{ ApplicationTemplate: "./sample-applications/mysql-persistent/mysql-persistent-template.yaml", ApplicationNamespace: "mysql-persistent", - Name: "mysql-e2e", + Name: "mysql-restic-e2e", BackupRestoreType: RESTIC, PreBackupVerify: mysqlReady(false, RESTIC), PostRestoreVerify: mysqlReady(false, RESTIC), diff --git a/tests/e2e/dpa_deployment_suite_test.go b/tests/e2e/dpa_deployment_suite_test.go index c68236e0ab..636c468a16 100644 --- a/tests/e2e/dpa_deployment_suite_test.go +++ b/tests/e2e/dpa_deployment_suite_test.go @@ -623,10 +623,6 @@ var _ = Describe("Configuration testing for DPA Custom Resource", func() { Eventually(ResticDaemonSetHasNodeSelector(namespace, key, value), timeoutMultiplier*time.Minute*3, time.Second*5).Should(BeTrue()) } } - if dpa.BackupImages() { - log.Printf("Waiting for registry pods to be running") - Eventually(AreRegistryDeploymentsAvailable(namespace), timeoutMultiplier*time.Minute*3, time.Second*5).Should(BeTrue()) - } }, genericTests, ) @@ -644,10 +640,6 @@ var _ = Describe("Configuration testing for DPA Custom Resource", func() { Expect(err).NotTo(HaveOccurred()) log.Printf("Waiting for velero pod to be running") Eventually(AreVeleroPodsRunning(namespace), timeoutMultiplier*time.Minute*3, time.Second*5).Should(BeTrue()) - if dpaCR.CustomResource.BackupImages() { - log.Printf("Waiting for registry pods to be running") - Eventually(AreRegistryDeploymentsAvailable(namespace), timeoutMultiplier*time.Minute*3, time.Second*5).Should(BeTrue()) - } log.Printf("Deleting dpa") err = dpaCR.Delete() if installCase.WantError { @@ -656,8 +648,6 @@ var _ = Describe("Configuration testing for DPA Custom Resource", func() { Expect(err).NotTo(HaveOccurred()) log.Printf("Checking no velero pods are running") Eventually(AreVeleroPodsRunning(namespace), timeoutMultiplier*time.Minute*3, time.Second*5).ShouldNot(BeTrue()) - log.Printf("Checking no registry deployment available") - Eventually(AreRegistryDeploymentsNotAvailable(namespace), timeoutMultiplier*time.Minute*3, time.Second*5).Should(BeTrue()) } }, Entry("Should succeed", deletionCase{WantError: false}), diff --git a/tests/e2e/lib/dpa_helpers.go b/tests/e2e/lib/dpa_helpers.go index 09e9800cef..cfcd1b78a8 100755 --- a/tests/e2e/lib/dpa_helpers.go +++ b/tests/e2e/lib/dpa_helpers.go @@ -66,6 +66,7 @@ func (v *DpaCustomResource) Build(backupRestoreType BackupRestoreType) error { Spec: oadpv1alpha1.DataProtectionApplicationSpec{ Configuration: &oadpv1alpha1.ApplicationConfig{ Velero: &oadpv1alpha1.VeleroConfig{ + LogLevel: "debug", DefaultPlugins: v.CustomResource.Spec.Configuration.Velero.DefaultPlugins, }, Restic: &oadpv1alpha1.ResticConfig{ @@ -250,7 +251,7 @@ func AreVeleroPodsRunning(namespace string) wait.ConditionFunc { } // Returns logs from velero container on velero pod -func getVeleroContainerLogs(namespace string) (string, error) { +func GetVeleroContainerLogs(namespace string) (string, error) { podList, err := GetVeleroPods(namespace) if err != nil { return "", err @@ -284,7 +285,7 @@ func getVeleroContainerLogs(namespace string) (string, error) { } func GetVeleroContainerFailureLogs(namespace string) []string { - containerLogs, err := getVeleroContainerLogs(namespace) + containerLogs, err := GetVeleroContainerLogs(namespace) if err != nil { log.Printf("cannot get velero container logs") return nil diff --git a/tests/e2e/lib/velero_helpers.go b/tests/e2e/lib/velero_helpers.go index bd9fd1b4f5..558902e389 100644 --- a/tests/e2e/lib/velero_helpers.go +++ b/tests/e2e/lib/velero_helpers.go @@ -128,6 +128,14 @@ func RestoreLogs(ocClient client.Client, restore velero.Restore) string { return logs.String() } +var errorIgnorePatterns = []string{ + "received EOF, stopping recv loop", + "Checking for AWS specific error information", + "awserr.Error contents", + "Error creating parent directories for blob-info-cache-v1.boltdb", + "blob unknown", +} + func BackupErrorLogs(ocClient client.Client, backup velero.Backup) []string { bl := BackupLogs(ocClient, backup) errorRegex, err := regexp.Compile("error|Error") @@ -137,7 +145,17 @@ func BackupErrorLogs(ocClient client.Client, backup velero.Backup) []string { logLines := []string{} for _, line := range strings.Split(bl, "\n") { if errorRegex.MatchString(line) { - logLines = append(logLines, line) + // ignore some expected errors + ignoreLine := false + for _, ignore := range errorIgnorePatterns { + ignoreLine, _ = regexp.MatchString(ignore, line) + if ignoreLine { + break + } + } + if !ignoreLine { + logLines = append(logLines, line) + } } } return logLines @@ -152,7 +170,17 @@ func RestoreErrorLogs(ocClient client.Client, restore velero.Restore) []string { logLines := []string{} for _, line := range strings.Split(rl, "\n") { if errorRegex.MatchString(line) { - logLines = append(logLines, line) + // ignore some expected errors + ignoreLine := false + for _, ignore := range errorIgnorePatterns { + ignoreLine, _ = regexp.MatchString(ignore, line) + if ignoreLine { + break + } + } + if !ignoreLine { + logLines = append(logLines, line) + } } } return logLines diff --git a/tests/e2e/subscription_suite_test.go b/tests/e2e/subscription_suite_test.go index 72c8e80a23..d71ef7b799 100644 --- a/tests/e2e/subscription_suite_test.go +++ b/tests/e2e/subscription_suite_test.go @@ -75,22 +75,16 @@ var _ = Describe("Subscription Config Suite Test", func() { log.Printf("Waiting for restic pods to be running") Eventually(AreResticPodsRunning(namespace), timeoutMultiplier*time.Minute*3, time.Second*5).Should(BeTrue()) } - if velero.BackupImages() { - log.Printf("Waiting for registry pods to be running") - Eventually(AreRegistryDeploymentsAvailable(namespace), timeoutMultiplier*time.Minute*3, time.Second*5).Should(BeTrue()) - } if s.Spec.Config != nil && s.Spec.Config.Env != nil { // get pod env vars log.Printf("Getting deployments") vd, err := GetVeleroDeploymentList(namespace) Expect(err).NotTo(HaveOccurred()) - rd, err := GetRegistryDeploymentList(namespace) - Expect(err).NotTo(HaveOccurred()) log.Printf("Getting daemonsets") rds, err := GetResticDaemonsetList(namespace) Expect(err).NotTo(HaveOccurred()) for _, env := range s.Spec.Config.Env { - for _, deployment := range append(vd.Items, rd.Items...) { + for _, deployment := range vd.Items { log.Printf("Checking env vars are passed to deployment " + deployment.Name) for _, container := range deployment.Spec.Template.Spec.Containers { log.Printf("Checking env vars are passed to container " + container.Name)