Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OADP-5158: Do not allow restic config use in OADP 1.5+; Deprecate restic for file system backups #1235

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions api/v1alpha1/dataprotectionapplication_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/field/spec at both places

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sold on this. Changing to spec would not pass sense that restic is an CR instead of part of DPA spec?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷 I like field. alternatively spec field.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or better yet..

Suggested change
// (do not use warning) restic field is for backwards compatibility and
// (do not use warning) restic config is for backwards compatibility and

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// will be removed in the future. Use nodeAgent field instead
// +optional
Restic *ResticConfig `json:"restic,omitempty"`

Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions bundle/manifests/oadp-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ metadata:
"configuration": {
"nodeAgent": {
"enable": true,
"uploaderType": "restic"
"uploaderType": "kopia"
},
"velero": {
"defaultPlugins": [
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: |-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: |-
Expand Down
2 changes: 0 additions & 2 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ spec:
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: RESTIC_PV_HOSTPATH
value: ""
- name: FS_PV_HOSTPATH
value: ""
- name: PLUGINS_HOSTPATH
Expand Down
1 change: 0 additions & 1 deletion config/samples/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ resources:
- downloadrequest.yaml
- podvolumebackup.yaml
- podvolumerestore.yaml
- resticrepository.yaml
- backup.yaml
- restore.yaml
- schedule.yaml
Expand Down
4 changes: 2 additions & 2 deletions config/samples/oadp_v1alpha1_dataprotectionapplication.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ spec:
- kubevirt
nodeAgent:
enable: true
uploaderType: restic
uploaderType: kopia
backupLocations:
- velero:
provider: aws
Expand All @@ -26,7 +26,7 @@ spec:
name: cloud-credentials
key: cloud
snapshotLocations:
- velero:
- velero:
provider: aws
config:
region: us-west-2
Expand Down
6 changes: 0 additions & 6 deletions config/samples/resticrepository.yaml

This file was deleted.

115 changes: 23 additions & 92 deletions internal/controller/nodeagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -27,7 +26,6 @@ import (
)

const (
ResticRestoreHelperCM = "restic-restore-action-config"
FsRestoreHelperCM = "fs-restore-action-config"
HostPods = "host-pods"
HostPlugins = "host-plugins"
Expand All @@ -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"
)
Expand All @@ -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:
Expand Down Expand Up @@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading