From 1862cebaad24b83dcf23a9d6bebcee4c7e58ee96 Mon Sep 17 00:00:00 2001 From: shkumari-px <141733960+shkumari-px@users.noreply.github.com> Date: Mon, 12 Aug 2024 22:35:30 +0530 Subject: [PATCH] Revert "PB-7476: Adding node affinity to kopia backup and restore jobs" --- pkg/controllers/dataexport/reconcile.go | 13 ------- pkg/controllers/resourceexport/reconcile.go | 8 ---- pkg/drivers/drivers.go | 3 +- pkg/drivers/kopiabackup/kopiabackup.go | 34 ++++++++++------- pkg/drivers/kopiarestore/kopiarestore.go | 6 --- pkg/drivers/nfsbackup/nfsbackup.go | 6 --- pkg/drivers/nfscsirestore/nfscsirestore.go | 7 ---- pkg/drivers/nfsrestore/nfsrestore.go | 7 ---- pkg/drivers/utils/utils.go | 41 --------------------- 9 files changed, 21 insertions(+), 104 deletions(-) diff --git a/pkg/controllers/dataexport/reconcile.go b/pkg/controllers/dataexport/reconcile.go index 3b6bc6128..7e8630537 100644 --- a/pkg/controllers/dataexport/reconcile.go +++ b/pkg/controllers/dataexport/reconcile.go @@ -1903,11 +1903,6 @@ func startTransferJob( psaJobUid = getAnnotationValue(dataExport, utils.PsaUIDKey) psaJobGid = getAnnotationValue(dataExport, utils.PsaGIDKey) } - nodeLabel, err := utils.GetNodeLabelFromDeployment(jobConfigMap, jobConfigMapNs, drivers.PxbJobNodeLabelKey) - if err != nil { - return "", err - } - switch drv.Name() { case drivers.Rsync: return drv.StartJob( @@ -1952,7 +1947,6 @@ func startTransferJob( drivers.WithExcludeFileList(excludeFileList), drivers.WithPodDatapathType(podDataPath), drivers.WithJobConfigMap(jobConfigMap), - drivers.WithNodeAffinity(nodeLabel), drivers.WithJobConfigMapNs(jobConfigMapNs), drivers.WithNfsServer(nfsServerAddr), drivers.WithNfsExportDir(nfsExportPath), @@ -1975,7 +1969,6 @@ func startTransferJob( drivers.WithCertSecretNamespace(dataExport.Spec.Destination.Namespace), drivers.WithJobConfigMap(jobConfigMap), drivers.WithJobConfigMapNs(jobConfigMapNs), - drivers.WithNodeAffinity(nodeLabel), drivers.WithNfsServer(nfsServerAddr), drivers.WithNfsExportDir(nfsExportPath), drivers.WithPodUserId(psaJobUid), @@ -2403,11 +2396,6 @@ func startNfsCSIRestoreVolumeJob( logrus.Errorf("failed to create NFS cred secret: %v", err) return "", fmt.Errorf("failed to create NFS cred secret: %v", err) } - nodeLabel, err := utils.GetNodeLabelFromDeployment(jobConfigMap, jobConfigMapNs, drivers.PxbJobNodeLabelKey) - if err != nil { - return "", err - } - switch drv.Name() { case drivers.NFSCSIRestore: return drv.StartJob( @@ -2422,7 +2410,6 @@ func startNfsCSIRestoreVolumeJob( drivers.WithNfsSubPath(bl.Location.Path), drivers.WithPodUserId(psaJobUid), drivers.WithPodGroupId(psaJobGid), - drivers.WithNodeAffinity(nodeLabel), ) } return "", fmt.Errorf("unknown driver for nfs csi volume restore: %s", drv.Name()) diff --git a/pkg/controllers/resourceexport/reconcile.go b/pkg/controllers/resourceexport/reconcile.go index f81bacfc2..9edff22a9 100644 --- a/pkg/controllers/resourceexport/reconcile.go +++ b/pkg/controllers/resourceexport/reconcile.go @@ -412,12 +412,6 @@ func startNfsResourceJob( logrus.Errorf("failed to create NFS cred secret: %v", err) return "", fmt.Errorf("failed to create NFS cred secret: %v", err) } - - nodeLabel, err := utils.GetNodeLabelFromDeployment(jobConfigMap, jobConfigMapNs, drivers.PxbJobNodeLabelKey) - if err != nil { - return "", err - } - switch drv.Name() { case drivers.NFSBackup: return drv.StartJob( @@ -433,7 +427,6 @@ func startNfsResourceJob( drivers.WithAppCRNamespace(re.Spec.Source.Namespace), drivers.WithNamespace(re.Namespace), drivers.WithResoureBackupName(re.Name), - drivers.WithNodeAffinity(nodeLabel), drivers.WithResoureBackupNamespace(re.Namespace), drivers.WithNfsMountOption(bl.Location.NFSConfig.MountOptions), drivers.WithNfsExportDir(bl.Location.NFSConfig.SubPath), @@ -452,7 +445,6 @@ func startNfsResourceJob( drivers.WithAppCRNamespace(re.Spec.Source.Namespace), drivers.WithNamespace(re.Namespace), drivers.WithResoureBackupName(re.Name), - drivers.WithNodeAffinity(nodeLabel), drivers.WithResoureBackupNamespace(re.Namespace), drivers.WithNfsMountOption(bl.Location.NFSConfig.MountOptions), drivers.WithNfsExportDir(bl.Location.NFSConfig.SubPath), diff --git a/pkg/drivers/drivers.go b/pkg/drivers/drivers.go index 2faaa2be5..921718a49 100644 --- a/pkg/drivers/drivers.go +++ b/pkg/drivers/drivers.go @@ -123,8 +123,7 @@ const ( var ( // ErrJobFailed is a know error for a data transfer job failure. - ErrJobFailed = fmt.Errorf("data transfer job failed") - PxbJobNodeLabelKey = "PXB_JOB_NODE_AFFINITY_LABEL" + ErrJobFailed = fmt.Errorf("data transfer job failed") ) // Interface defines a data export driver behaviour. diff --git a/pkg/drivers/kopiabackup/kopiabackup.go b/pkg/drivers/kopiabackup/kopiabackup.go index d50220e0f..2fd6f5e32 100644 --- a/pkg/drivers/kopiabackup/kopiabackup.go +++ b/pkg/drivers/kopiabackup/kopiabackup.go @@ -273,7 +273,6 @@ func jobFor( jobName string, resources corev1.ResourceRequirements, nodeName string, - live bool, ) (*batchv1.Job, error) { backupName := jobName @@ -411,14 +410,6 @@ func jobFor( job.Spec.Template.Spec.ImagePullSecrets = utils.ToImagePullSecret(utils.GetImageSecretName(jobName)) } - // Add node affinity to the job spec - if !live { - job, err = utils.AddNodeAffinityToJob(job, jobOption) - if err != nil { - return nil, err - } - } - if len(jobOption.NfsServer) != 0 { volumeMount := corev1.VolumeMount{ Name: utils.NfsVolumeName, @@ -502,8 +493,8 @@ func buildJob(jobName string, jobOptions drivers.JobOpts) (*batchv1.Job, error) return nil, fmt.Errorf(errMsg) } var resourceNamespace string - var nodeName string var live bool + var nodeName string // filter out the pods that are create by us for _, pod := range pods { labels := pod.ObjectMeta.Labels @@ -514,12 +505,11 @@ func buildJob(jobName string, jobOptions drivers.JobOpts) (*batchv1.Job, error) // get the nodeName, if the pods is in Running state, So that we can schedule // kopia job on the same node. nodeName = pod.Spec.NodeName - live = true break } } resourceNamespace = jobOptions.Namespace - if err := utils.SetupServiceAccount(jobName, resourceNamespace, roleFor()); err != nil { + if err := utils.SetupServiceAccount(jobName, resourceNamespace, roleFor(live)); err != nil { errMsg := fmt.Sprintf("error creating service account %s/%s: %v", resourceNamespace, jobName, err) logrus.Errorf("%s: %v", fn, errMsg) return nil, fmt.Errorf(errMsg) @@ -529,11 +519,10 @@ func buildJob(jobName string, jobOptions drivers.JobOpts) (*batchv1.Job, error) jobName, resources, nodeName, - live, ) } -func roleFor() *rbacv1.Role { +func roleFor(live bool) *rbacv1.Role { role := &rbacv1.Role{ Rules: []rbacv1.PolicyRule{ { @@ -543,5 +532,22 @@ func roleFor() *rbacv1.Role { }, }, } + // Only live backup, we will add the hostaccess and privilege option. + if live { + hostAccessRule := rbacv1.PolicyRule{ + APIGroups: []string{"security.openshift.io"}, + Resources: []string{"securitycontextconstraints"}, + ResourceNames: []string{"hostaccess"}, + Verbs: []string{"use"}, + } + role.Rules = append(role.Rules, hostAccessRule) + PrivilegedRule := rbacv1.PolicyRule{ + APIGroups: []string{"security.openshift.io"}, + Resources: []string{"securitycontextconstraints"}, + ResourceNames: []string{"privileged"}, + Verbs: []string{"use"}, + } + role.Rules = append(role.Rules, PrivilegedRule) + } return role } diff --git a/pkg/drivers/kopiarestore/kopiarestore.go b/pkg/drivers/kopiarestore/kopiarestore.go index 3544c632d..5e700428c 100644 --- a/pkg/drivers/kopiarestore/kopiarestore.go +++ b/pkg/drivers/kopiarestore/kopiarestore.go @@ -307,12 +307,6 @@ func jobFor( job.Spec.Template.Spec.ImagePullSecrets = utils.ToImagePullSecret(utils.GetImageSecretName(jobName)) } - // Add node affinity to the job spec - job, err = utils.AddNodeAffinityToJob(job, jobOption) - if err != nil { - return nil, err - } - if drivers.CertFilePath != "" { volumeMount := corev1.VolumeMount{ Name: utils.TLSCertMountVol, diff --git a/pkg/drivers/nfsbackup/nfsbackup.go b/pkg/drivers/nfsbackup/nfsbackup.go index 0fb6d7679..b64454a26 100644 --- a/pkg/drivers/nfsbackup/nfsbackup.go +++ b/pkg/drivers/nfsbackup/nfsbackup.go @@ -306,12 +306,6 @@ func jobForBackupResource( } } - // Add node affinity to the job spec - job, err = utils.AddNodeAffinityToJob(job, jobOption) - if err != nil { - return nil, err - } - // Add the image secret in job spec only if it is present in the stork deployment. if len(imageRegistrySecret) != 0 { job.Spec.Template.Spec.ImagePullSecrets = utils.ToImagePullSecret(utils.GetImageSecretName(jobOption.RestoreExportName)) diff --git a/pkg/drivers/nfscsirestore/nfscsirestore.go b/pkg/drivers/nfscsirestore/nfscsirestore.go index 60d84d62f..2cc6024db 100644 --- a/pkg/drivers/nfscsirestore/nfscsirestore.go +++ b/pkg/drivers/nfscsirestore/nfscsirestore.go @@ -286,13 +286,6 @@ func jobForRestoreCSISnapshot( if len(imageRegistrySecret) != 0 { job.Spec.Template.Spec.ImagePullSecrets = utils.ToImagePullSecret(utils.GetImageSecretName(jobName)) } - - // Add node affinity to the job spec - job, err = utils.AddNodeAffinityToJob(job, jobOption) - if err != nil { - return nil, err - } - if len(jobOption.NfsServer) != 0 { volumeMount := corev1.VolumeMount{ Name: utils.NfsVolumeName, diff --git a/pkg/drivers/nfsrestore/nfsrestore.go b/pkg/drivers/nfsrestore/nfsrestore.go index efc45ac58..881ef2721 100644 --- a/pkg/drivers/nfsrestore/nfsrestore.go +++ b/pkg/drivers/nfsrestore/nfsrestore.go @@ -327,13 +327,6 @@ func jobForRestoreResource( if err != nil { return nil, err } - - // Add node affinity to the job spec - job, err = utils.AddNodeAffinityToJob(job, jobOption) - if err != nil { - return nil, err - } - // Add the image secret in job spec only if it is present in the stork deployment. if len(imageRegistrySecret) != 0 { job.Spec.Template.Spec.ImagePullSecrets = utils.ToImagePullSecret(utils.GetImageSecretName(jobOption.RestoreExportName)) diff --git a/pkg/drivers/utils/utils.go b/pkg/drivers/utils/utils.go index 2341d6e70..ab566c84b 100644 --- a/pkg/drivers/utils/utils.go +++ b/pkg/drivers/utils/utils.go @@ -859,20 +859,6 @@ func GetNodeAffinityFromDeployment(name, namespace string) (*corev1.NodeAffinity return deploy.Spec.Template.Spec.Affinity.NodeAffinity, nil } -// GetNodeLabelFromDeployment gets node label from deployment -func GetNodeLabelFromDeployment(name, namespace, key string) (map[string]string, error) { - nodeLabel := make(map[string]string) - deploy, err := core.Instance().GetConfigMap(name, namespace) - if err != nil { - return nil, err - } - value, ok := deploy.Data[key] - if ok && value != "" { - nodeLabel[key] = value - } - return nodeLabel, nil -} - // IsJobPodMountFailed - checks for mount failure in a Job pod func IsJobPodMountFailed(job *batchv1.Job, namespace string) bool { fn := "IsJobPodMountFailed" @@ -1141,30 +1127,3 @@ func PauseCleanupResource() (time.Duration, error) { } return pauseCleanupVal, nil } - -// AddNodeAffinityToJob adds node affinity to the job spec -func AddNodeAffinityToJob(job *batchv1.Job, jobOption drivers.JobOpts) (*batchv1.Job, error) { - if len(jobOption.NodeAffinity) > 0 { - matchExpressions := []corev1.NodeSelectorRequirement{} - for key, val := range jobOption.NodeAffinity { - expression := corev1.NodeSelectorRequirement{ - Key: key, - Operator: corev1.NodeSelectorOpIn, - Values: []string{val}, - } - matchExpressions = append(matchExpressions, expression) - } - job.Spec.Template.Spec.Affinity = &corev1.Affinity{ - NodeAffinity: &corev1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ - NodeSelectorTerms: []corev1.NodeSelectorTerm{ - { - MatchExpressions: matchExpressions, - }, - }, - }, - }, - } - } - return job, nil -}