OADP-5158: Do not allow restic config use in OADP 1.5+; Deprecate res…

…tic for file system backups (#1235) * fix: Remove restic config from master branch Signed-off-by: Mateus Oliveira <[email protected]> * fixup! fix: Remove restic config from master branch Signed-off-by: Mateus Oliveira <[email protected]> * fixup! fix: Remove restic config from master branch Signed-off-by: Mateus Oliveira <[email protected]> --------- Signed-off-by: Mateus Oliveira <[email protected]>
openshift · Mar 7, 2025 · 9ce4670 · 9ce4670
1 parent da7cbd2
commit 9ce4670
Show file tree

Hide file tree

Showing 15 changed files with 89 additions and 196 deletions.
diff --git a/api/v1alpha1/dataprotectionapplication_types.go b/api/v1alpha1/dataprotectionapplication_types.go
@@ -374,10 +374,8 @@ type ResticConfig struct {
 // ApplicationConfig defines the configuration for the Data Protection Application
 type ApplicationConfig struct {
 	Velero *VeleroConfig `json:"velero,omitempty"`
-	// (deprecation warning) ResticConfig is the configuration for restic DaemonSet.
-	// restic is for backwards compatibility and is replaced by the nodeAgent
-	// restic will be removed in the future
-	// +kubebuilder:deprecatedversion:warning=1.3
+	// (do not use warning) restic field is for backwards compatibility and
+	// will be removed in the future. Use nodeAgent field instead
 	// +optional
 	Restic *ResticConfig `json:"restic,omitempty"`
 
@@ -761,8 +759,6 @@ func (dpa *DataProtectionApplication) AutoCorrect() {
 		if dpa.Spec.Configuration != nil {
 			if dpa.Spec.Configuration.NodeAgent != nil && len(dpa.Spec.Configuration.NodeAgent.Timeout) > 0 {
 				fsBackupTimeout = dpa.Spec.Configuration.NodeAgent.Timeout
-			} else if dpa.Spec.Configuration.Restic != nil && len(dpa.Spec.Configuration.Restic.Timeout) > 0 {
-				fsBackupTimeout = dpa.Spec.Configuration.Restic.Timeout
 			}
 		}
 		if pvOperationTimeout, err := time.ParseDuration(fsBackupTimeout); err == nil && dpa.Spec.Configuration.Velero.Args.PodVolumeOperationTimeout == nil {

diff --git a/bundle/manifests/oadp-operator.clusterserviceversion.yaml b/bundle/manifests/oadp-operator.clusterserviceversion.yaml
@@ -34,7 +34,7 @@ metadata:
             "configuration": {
               "nodeAgent": {
                 "enable": true,
-                "uploaderType": "restic"
+                "uploaderType": "kopia"
               },
               "velero": {
                 "defaultPlugins": [
@@ -1245,7 +1245,6 @@ spec:
                   valueFrom:
                     fieldRef:
                       fieldPath: metadata.annotations['olm.targetNamespaces']
-                - name: RESTIC_PV_HOSTPATH
                 - name: FS_PV_HOSTPATH
                 - name: PLUGINS_HOSTPATH
                 - name: RELATED_IMAGE_VELERO

diff --git a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
@@ -457,9 +457,8 @@ spec:
                       type: object
                     restic:
                       description: |-
-                        (deprecation warning) ResticConfig is the configuration for restic DaemonSet.
-                        restic is for backwards compatibility and is replaced by the nodeAgent
-                        restic will be removed in the future
+                        (do not use warning) restic field is for backwards compatibility and
+                        will be removed in the future. Use nodeAgent field instead
                       properties:
                         enable:
                           description: |-

diff --git a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
@@ -457,9 +457,8 @@ spec:
                       type: object
                     restic:
                       description: |-
-                        (deprecation warning) ResticConfig is the configuration for restic DaemonSet.
-                        restic is for backwards compatibility and is replaced by the nodeAgent
-                        restic will be removed in the future
+                        (do not use warning) restic field is for backwards compatibility and
+                        will be removed in the future. Use nodeAgent field instead
                       properties:
                         enable:
                           description: |-

diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -56,8 +56,6 @@ spec:
               valueFrom:
                 fieldRef:
                   fieldPath: metadata.namespace
-            - name: RESTIC_PV_HOSTPATH
-              value: ""
             - name: FS_PV_HOSTPATH
               value: ""
             - name: PLUGINS_HOSTPATH

diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml
@@ -8,7 +8,6 @@ resources:
 - downloadrequest.yaml
 - podvolumebackup.yaml
 - podvolumerestore.yaml
-- resticrepository.yaml
 - backup.yaml
 - restore.yaml
 - schedule.yaml

diff --git a/config/samples/oadp_v1alpha1_dataprotectionapplication.yaml b/config/samples/oadp_v1alpha1_dataprotectionapplication.yaml
@@ -11,7 +11,7 @@ spec:
       - kubevirt
     nodeAgent:
       enable: true
-      uploaderType: restic
+      uploaderType: kopia
   backupLocations:
     - velero:
         provider: aws
@@ -26,7 +26,7 @@ spec:
           name: cloud-credentials
           key: cloud
   snapshotLocations:
-    - velero:  
+    - velero:
         provider: aws
         config:
           region: us-west-2

diff --git a/config/samples/resticrepository.yaml b/config/samples/resticrepository.yaml
diff --git a/internal/controller/nodeagent.go b/internal/controller/nodeagent.go
@@ -16,7 +16,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
-	"k8s.io/utils/pointer"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -27,7 +26,6 @@ import (
 )
 
 const (
-	ResticRestoreHelperCM   = "restic-restore-action-config"
 	FsRestoreHelperCM       = "fs-restore-action-config"
 	HostPods                = "host-pods"
 	HostPlugins             = "host-plugins"
@@ -37,7 +35,6 @@ const (
 	IBMCloudPVHostPath      = "/var/data/kubelet/pods"
 	GenericPluginsHostPath  = "/var/lib/kubelet/plugins"
 	IBMCloudPluginsHostPath = "/var/data/kubelet/plugins"
-	ResticPVHostPathEnvVar  = "RESTIC_PV_HOSTPATH"
 	FSPVHostPathEnvVar      = "FS_PV_HOSTPATH"
 	PluginsHostPathEnvVar   = "PLUGINS_HOSTPATH"
 )
@@ -63,10 +60,6 @@ func getFsPvHostPath(platformType string) string {
 		return envFs
 	}
 
-	if env := os.Getenv(ResticPVHostPathEnvVar); env != "" {
-		return env
-	}
-
 	// Return platform-specific host paths
 	switch platformType {
 	case IBMCloudPlatform:
@@ -108,16 +101,7 @@ func (r *DataProtectionApplicationReconciler) ReconcileNodeAgentDaemonset(log lo
 		ObjectMeta: getNodeAgentObjectMeta(r),
 	}
 
-	if dpa.Spec.Configuration.Restic != nil {
-		// V(-1) corresponds to the warn level
-		var deprecationMsg string = "(Deprecation Warning) Use nodeAgent instead of restic, which is deprecated and will be removed in the future"
-		log.V(-1).Info(deprecationMsg)
-		r.EventRecorder.Event(dpa, corev1.EventTypeWarning, "DeprecationResticConfig", deprecationMsg)
-	}
-
-	if dpa.Spec.Configuration.Restic != nil && dpa.Spec.Configuration.Restic.Enable != nil && *dpa.Spec.Configuration.Restic.Enable {
-		deleteDaemonSet = false
-	} else if dpa.Spec.Configuration.NodeAgent != nil && dpa.Spec.Configuration.NodeAgent.Enable != nil && *dpa.Spec.Configuration.NodeAgent.Enable {
+	if dpa.Spec.Configuration.NodeAgent != nil && dpa.Spec.Configuration.NodeAgent.Enable != nil && *dpa.Spec.Configuration.NodeAgent.Enable {
 		deleteDaemonSet = false
 	}
 
@@ -132,11 +116,9 @@ func (r *DataProtectionApplicationReconciler) ReconcileNodeAgentDaemonset(log lo
 			}
 			return false, err
 		}
-		// no errors means there already is an existing DaeMonset.
+		// no errors means there is already an existing DaemonSet.
 		// TODO: Check if NodeAgent is in use, a backup is running, so don't blindly delete NodeAgent.
-		// If dpa.Spec.Configuration.NodeAgent enable exists and is false, attempt to delete.
-		deleteOptionPropagationForeground := metav1.DeletePropagationForeground
-		if err := r.Delete(deleteContext, ds, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil {
+		if err := r.Delete(deleteContext, ds, &client.DeleteOptions{PropagationPolicy: ptr.To(metav1.DeletePropagationForeground)}); err != nil {
 			// TODO: Come back and fix event recording to be consistent
 			r.EventRecorder.Event(ds, corev1.EventTypeNormal, "DeleteDaemonSetFailed", "Got DaemonSet to delete but could not delete err:"+err.Error())
 			return false, err
@@ -214,24 +196,16 @@ func (r *DataProtectionApplicationReconciler) ReconcileNodeAgentDaemonset(log lo
  */
 func (r *DataProtectionApplicationReconciler) buildNodeAgentDaemonset(ds *appsv1.DaemonSet) (*appsv1.DaemonSet, error) {
 	dpa := r.dpa
-	if dpa == nil {
-		return nil, fmt.Errorf("dpa cannot be nil")
-	}
+
 	if ds == nil {
-		return nil, fmt.Errorf("ds cannot be nil")
+		return nil, fmt.Errorf("DaemonSet cannot be nil")
 	}
 
 	var nodeAgentResourceReqs corev1.ResourceRequirements
 
 	// get resource requirements for nodeAgent ds
 	// ignoring err here as it is checked in validator.go
-	if dpa.Spec.Configuration.Restic != nil {
-		nodeAgentResourceReqs, _ = getResticResourceReqs(dpa)
-	} else if dpa.Spec.Configuration.NodeAgent != nil {
-		nodeAgentResourceReqs, _ = getNodeAgentResourceReqs(dpa)
-	} else {
-		return nil, fmt.Errorf("NodeAgent or Restic configuration cannot be nil")
-	}
+	nodeAgentResourceReqs, _ = getNodeAgentResourceReqs(dpa)
 
 	installDs := install.DaemonSet(ds.Namespace,
 		install.WithResources(nodeAgentResourceReqs),
@@ -257,29 +231,6 @@ func (r *DataProtectionApplicationReconciler) buildNodeAgentDaemonset(ds *appsv1
 
 func (r *DataProtectionApplicationReconciler) customizeNodeAgentDaemonset(ds *appsv1.DaemonSet) (*appsv1.DaemonSet, error) {
 	dpa := r.dpa
-	if dpa.Spec.Configuration == nil || (dpa.Spec.Configuration.Restic == nil && dpa.Spec.Configuration.NodeAgent == nil) {
-		// if restic and nodeAgent are not configured, therefore not enabled, return early.
-		return nil, nil
-	}
-
-	var useResticConf bool = true
-
-	if dpa.Spec.Configuration.NodeAgent != nil {
-		useResticConf = false
-	}
-
-	// add custom pod labels
-	var err error
-	if useResticConf {
-		if dpa.Spec.Configuration.Restic.PodConfig != nil && dpa.Spec.Configuration.Restic.PodConfig.Labels != nil {
-			ds.Spec.Template.Labels, err = common.AppendUniqueKeyTOfTMaps(ds.Spec.Template.Labels, dpa.Spec.Configuration.Restic.PodConfig.Labels)
-		}
-	} else if dpa.Spec.Configuration.NodeAgent.PodConfig != nil && dpa.Spec.Configuration.NodeAgent.PodConfig.Labels != nil {
-		ds.Spec.Template.Labels, err = common.AppendUniqueKeyTOfTMaps(ds.Spec.Template.Labels, dpa.Spec.Configuration.NodeAgent.PodConfig.Labels)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("NodeAgent daemonset template custom label: %s", err)
-	}
 
 	// customize specs
 	ds.Spec.Selector = nodeAgentLabelSelector
@@ -288,16 +239,9 @@ func (r *DataProtectionApplicationReconciler) customizeNodeAgentDaemonset(ds *ap
 	}
 
 	// customize template specs
-	if useResticConf {
-		ds.Spec.Template.Spec.SecurityContext = &corev1.PodSecurityContext{
-			RunAsUser:          pointer.Int64(0),
-			SupplementalGroups: dpa.Spec.Configuration.Restic.SupplementalGroups,
-		}
-	} else {
-		ds.Spec.Template.Spec.SecurityContext = &corev1.PodSecurityContext{
-			RunAsUser:          pointer.Int64(0),
-			SupplementalGroups: dpa.Spec.Configuration.NodeAgent.SupplementalGroups,
-		}
+	ds.Spec.Template.Spec.SecurityContext = &corev1.PodSecurityContext{
+		RunAsUser:          ptr.To(int64(0)),
+		SupplementalGroups: dpa.Spec.Configuration.NodeAgent.SupplementalGroups,
 	}
 	ds.Spec.Template.Spec.Volumes = append(ds.Spec.Template.Spec.Volumes,
 		// append certs volume
@@ -344,14 +288,17 @@ func (r *DataProtectionApplicationReconciler) customizeNodeAgentDaemonset(ds *ap
 	}
 
 	// Update with any pod config values
-	if useResticConf {
-		if dpa.Spec.Configuration.Restic.PodConfig != nil {
-			ds.Spec.Template.Spec.Tolerations = dpa.Spec.Configuration.Restic.PodConfig.Tolerations
-			ds.Spec.Template.Spec.NodeSelector = dpa.Spec.Configuration.Restic.PodConfig.NodeSelector
-		}
-	} else if dpa.Spec.Configuration.NodeAgent.PodConfig != nil {
+	if dpa.Spec.Configuration.NodeAgent.PodConfig != nil {
 		ds.Spec.Template.Spec.Tolerations = dpa.Spec.Configuration.NodeAgent.PodConfig.Tolerations
 		ds.Spec.Template.Spec.NodeSelector = dpa.Spec.Configuration.NodeAgent.PodConfig.NodeSelector
+		// add custom pod labels
+		if dpa.Spec.Configuration.NodeAgent.PodConfig.Labels != nil {
+			var err error
+			ds.Spec.Template.Labels, err = common.AppendUniqueKeyTOfTMaps(ds.Spec.Template.Labels, dpa.Spec.Configuration.NodeAgent.PodConfig.Labels)
+			if err != nil {
+				return nil, fmt.Errorf("NodeAgent daemonset template custom label: %s", err)
+			}
+		}
 	}
 
 	// fetch nodeAgent container in order to customize it
@@ -380,19 +327,15 @@ func (r *DataProtectionApplicationReconciler) customizeNodeAgentDaemonset(ds *ap
 				}
 			}
 			// append PodConfig envs to nodeAgent container
-			if useResticConf {
-				if dpa.Spec.Configuration.Restic.PodConfig != nil && dpa.Spec.Configuration.Restic.PodConfig.Env != nil {
-					nodeAgentContainer.Env = common.AppendUniqueEnvVars(nodeAgentContainer.Env, dpa.Spec.Configuration.Restic.PodConfig.Env)
-				}
-			} else if dpa.Spec.Configuration.NodeAgent.PodConfig != nil && dpa.Spec.Configuration.NodeAgent.PodConfig.Env != nil {
+			if dpa.Spec.Configuration.NodeAgent.PodConfig != nil && dpa.Spec.Configuration.NodeAgent.PodConfig.Env != nil {
 				nodeAgentContainer.Env = common.AppendUniqueEnvVars(nodeAgentContainer.Env, dpa.Spec.Configuration.NodeAgent.PodConfig.Env)
 			}
 
 			// append env vars to the nodeAgent container
 			nodeAgentContainer.Env = common.AppendUniqueEnvVars(nodeAgentContainer.Env, proxy.ReadProxyVarsFromEnv())
 
 			nodeAgentContainer.SecurityContext = &corev1.SecurityContext{
-				Privileged: pointer.Bool(true),
+				Privileged: ptr.To(true),
 			}
 
 			imagePullPolicy, err := common.GetImagePullPolicy(dpa.Spec.ImagePullPolicy, getVeleroImage(dpa))
@@ -428,9 +371,8 @@ func (r *DataProtectionApplicationReconciler) customizeNodeAgentDaemonset(ds *ap
 		return nil, err
 	}
 
-	if err := credentials.AppendCloudProviderVolumes(dpa, ds, providerNeedsDefaultCreds, hasCloudStorage); err != nil {
-		return nil, err
-	}
+	credentials.AppendCloudProviderVolumes(dpa, ds, providerNeedsDefaultCreds, hasCloudStorage)
+
 	setPodTemplateSpecDefaults(&ds.Spec.Template)
 	if ds.Spec.UpdateStrategy.Type == appsv1.RollingUpdateDaemonSetStrategyType {
 		ds.Spec.UpdateStrategy.RollingUpdate = &appsv1.RollingUpdateDaemonSet{
@@ -445,7 +387,7 @@ func (r *DataProtectionApplicationReconciler) customizeNodeAgentDaemonset(ds *ap
 		}
 	}
 	if ds.Spec.RevisionHistoryLimit == nil {
-		ds.Spec.RevisionHistoryLimit = pointer.Int32(10)
+		ds.Spec.RevisionHistoryLimit = ptr.To(int32(10))
 	}
 
 	return ds, nil
@@ -459,17 +401,6 @@ func (r *DataProtectionApplicationReconciler) ReconcileFsRestoreHelperConfig(log
 		},
 	}
 
-	// Delete renamed CM restic-restore-action-config
-	// Velero uses labels to identify the CM. For consistency we have the
-	// same name as upstream, whch is `fs-restore-action-config`
-	resticRestoreHelperCM := corev1.ConfigMap{}
-	if err := r.Get(r.Context, types.NamespacedName{Namespace: r.NamespacedName.Namespace, Name: ResticRestoreHelperCM}, &resticRestoreHelperCM); err == nil {
-		r.Log.Info("Deleting deprecated ConfigMap restic-restore-action-config.")
-		if err := r.Delete(r.Context, &resticRestoreHelperCM); err != nil {
-			return false, err
-		}
-	}
-
 	op, err := controllerutil.CreateOrPatch(r.Context, r.Client, &fsRestoreHelperCM, func() error {
 
 		// update the Config Map