From 0fc7e2f98a4d0bd14eefe0b9ef5bea72ced14c71 Mon Sep 17 00:00:00 2001 From: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com> Date: Thu, 24 Jul 2025 11:28:28 +0800 Subject: [PATCH] Add imagePullSecrets inheritance for VGDP pod and maintenance job. (#9102) Signed-off-by: Xun Jiang --- changelogs/unreleased/9102-blackpiglet | 1 + pkg/exposer/csi_snapshot.go | 1 + pkg/exposer/generic_restore.go | 1 + pkg/exposer/image.go | 19 +++--- pkg/exposer/image_test.go | 14 ++++- pkg/repository/maintenance/maintenance.go | 15 ++++- .../maintenance/maintenance_test.go | 51 ++++++++++++---- pkg/uploader/provider/kopia_test.go | 24 ++++---- pkg/util/velero/velero.go | 5 ++ pkg/util/velero/velero_test.go | 58 +++++++++++++++++++ 10 files changed, 153 insertions(+), 36 deletions(-) create mode 100644 changelogs/unreleased/9102-blackpiglet diff --git a/changelogs/unreleased/9102-blackpiglet b/changelogs/unreleased/9102-blackpiglet new file mode 100644 index 000000000..87a714b55 --- /dev/null +++ b/changelogs/unreleased/9102-blackpiglet @@ -0,0 +1 @@ +Add imagePullSecrets inheritance for VGDP pod and maintenance job. diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index df5936c3b..030965f3f 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -681,6 +681,7 @@ func (e *csiSnapshotExposer) createBackupPod( RestartPolicy: corev1.RestartPolicyNever, SecurityContext: securityCtx, Tolerations: toleration, + ImagePullSecrets: podInfo.imagePullSecrets, }, } diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index b299f954f..eefbaace1 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -510,6 +510,7 @@ func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObjec RestartPolicy: corev1.RestartPolicyNever, SecurityContext: securityCtx, Tolerations: toleration, + ImagePullSecrets: podInfo.imagePullSecrets, }, } diff --git a/pkg/exposer/image.go b/pkg/exposer/image.go index da399cce5..70ab9b00c 100644 --- a/pkg/exposer/image.go +++ b/pkg/exposer/image.go @@ -28,14 +28,15 @@ import ( ) type inheritedPodInfo struct { - image string - serviceAccount string - env []v1.EnvVar - envFrom []v1.EnvFromSource - volumeMounts []v1.VolumeMount - volumes []v1.Volume - logLevelArgs []string - logFormatArgs []string + image string + serviceAccount string + env []v1.EnvVar + envFrom []v1.EnvFromSource + volumeMounts []v1.VolumeMount + volumes []v1.Volume + logLevelArgs []string + logFormatArgs []string + imagePullSecrets []v1.LocalObjectReference } func getInheritedPodInfo(ctx context.Context, client kubernetes.Interface, veleroNamespace string, osType string) (inheritedPodInfo, error) { @@ -71,5 +72,7 @@ func getInheritedPodInfo(ctx context.Context, client kubernetes.Interface, veler } } + podInfo.imagePullSecrets = podSpec.ImagePullSecrets + return podInfo, nil } diff --git a/pkg/exposer/image_test.go b/pkg/exposer/image_test.go index 18626174b..2e25e389e 100644 --- a/pkg/exposer/image_test.go +++ b/pkg/exposer/image_test.go @@ -26,11 +26,11 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" - "github.com/vmware-tanzu/velero/pkg/util/kube" - appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes/fake" + + "github.com/vmware-tanzu/velero/pkg/util/kube" ) func TestGetInheritedPodInfo(t *testing.T) { @@ -177,6 +177,11 @@ func TestGetInheritedPodInfo(t *testing.T) { }, }, ServiceAccountName: "sa-1", + ImagePullSecrets: []v1.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, }, }, @@ -317,6 +322,11 @@ func TestGetInheritedPodInfo(t *testing.T) { "--log-level", "debug", }, + imagePullSecrets: []v1.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, }, } diff --git a/pkg/repository/maintenance/maintenance.go b/pkg/repository/maintenance/maintenance.go index b5d489484..3d369936b 100644 --- a/pkg/repository/maintenance/maintenance.go +++ b/pkg/repository/maintenance/maintenance.go @@ -407,8 +407,16 @@ func StartNewJob(cli client.Client, ctx context.Context, repo *velerov1api.Backu return maintenanceJob.Name, nil } -func buildJob(cli client.Client, ctx context.Context, repo *velerov1api.BackupRepository, bslName string, config *JobConfigs, - podResources kube.PodResources, logLevel logrus.Level, logFormat *logging.FormatFlag) (*batchv1.Job, error) { +func buildJob( + cli client.Client, + ctx context.Context, + repo *velerov1api.BackupRepository, + bslName string, + config *JobConfigs, + podResources kube.PodResources, + logLevel logrus.Level, + logFormat *logging.FormatFlag, +) (*batchv1.Job, error) { // Get the Velero server deployment deployment := &appsv1.Deployment{} err := cli.Get(ctx, types.NamespacedName{Name: "velero", Namespace: repo.Namespace}, deployment) @@ -431,6 +439,8 @@ func buildJob(cli client.Client, ctx context.Context, repo *velerov1api.BackupRe // Get the service account from the Velero server deployment serviceAccount := veleroutil.GetServiceAccountFromVeleroServer(deployment) + imagePullSecrets := veleroutil.GetImagePullSecretsFromVeleroServer(deployment) + // Get image image := veleroutil.GetVeleroServerImage(deployment) @@ -520,6 +530,7 @@ func buildJob(cli client.Client, ctx context.Context, repo *velerov1api.BackupRe Value: "windows", }, }, + ImagePullSecrets: imagePullSecrets, }, }, }, diff --git a/pkg/repository/maintenance/maintenance_test.go b/pkg/repository/maintenance/maintenance_test.go index 5ee7a92d1..be7af2a8e 100644 --- a/pkg/repository/maintenance/maintenance_test.go +++ b/pkg/repository/maintenance/maintenance_test.go @@ -903,6 +903,11 @@ func TestBuildJob(t *testing.T) { }, }, }, + ImagePullSecrets: []v1.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, }, }, @@ -912,17 +917,18 @@ func TestBuildJob(t *testing.T) { deploy2.Spec.Template.Labels = map[string]string{"azure.workload.identity/use": "fake-label-value"} testCases := []struct { - name string - m *JobConfigs - deploy *appsv1.Deployment - logLevel logrus.Level - logFormat *logging.FormatFlag - thirdPartyLabel map[string]string - expectedJobName string - expectedError bool - expectedEnv []v1.EnvVar - expectedEnvFrom []v1.EnvFromSource - expectedPodLabel map[string]string + name string + m *JobConfigs + deploy *appsv1.Deployment + logLevel logrus.Level + logFormat *logging.FormatFlag + thirdPartyLabel map[string]string + expectedJobName string + expectedError bool + expectedEnv []v1.EnvVar + expectedEnvFrom []v1.EnvFromSource + expectedPodLabel map[string]string + expectedImagePullSecrets []v1.LocalObjectReference }{ { name: "Valid maintenance job without third party labels", @@ -964,6 +970,11 @@ func TestBuildJob(t *testing.T) { expectedPodLabel: map[string]string{ RepositoryNameLabel: "test-123", }, + expectedImagePullSecrets: []v1.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, { name: "Valid maintenance job with third party labels", @@ -1006,6 +1017,11 @@ func TestBuildJob(t *testing.T) { RepositoryNameLabel: "test-123", "azure.workload.identity/use": "fake-label-value", }, + expectedImagePullSecrets: []v1.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, { name: "Error getting Velero server deployment", @@ -1057,7 +1073,16 @@ func TestBuildJob(t *testing.T) { cli := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objs...).Build() // Call the function to test - job, err := buildJob(cli, context.TODO(), param.BackupRepo, param.BackupLocation.Name, tc.m, *tc.m.PodResources, tc.logLevel, tc.logFormat) + job, err := buildJob( + cli, + context.TODO(), + param.BackupRepo, + param.BackupLocation.Name, + tc.m, + *tc.m.PodResources, + tc.logLevel, + tc.logFormat, + ) // Check the error if tc.expectedError { @@ -1108,6 +1133,8 @@ func TestBuildJob(t *testing.T) { assert.Equal(t, expectedArgs, container.Args) assert.Equal(t, tc.expectedPodLabel, job.Spec.Template.Labels) + + assert.Equal(t, tc.expectedImagePullSecrets, job.Spec.Template.Spec.ImagePullSecrets) } }) } diff --git a/pkg/uploader/provider/kopia_test.go b/pkg/uploader/provider/kopia_test.go index 8391d1c7d..2ecdddfad 100644 --- a/pkg/uploader/provider/kopia_test.go +++ b/pkg/uploader/provider/kopia_test.go @@ -63,14 +63,6 @@ type FakeRestoreProgressUpdater struct { func (f *FakeRestoreProgressUpdater) UpdateProgress(p *uploader.Progress) {} func TestRunBackup(t *testing.T) { - mockBRepo := udmrepomocks.NewBackupRepo(t) - mockBRepo.On("GetAdvancedFeatures").Return(udmrepo.AdvancedFeatureInfo{}) - - var kp kopiaProvider - kp.log = logrus.New() - kp.bkRepo = mockBRepo - updater := FakeBackupProgressUpdater{PodVolumeBackup: &velerov1api.PodVolumeBackup{}, Log: kp.log, Ctx: context.Background(), Cli: fake.NewClientBuilder().WithScheme(util.VeleroScheme).Build()} - testCases := []struct { name string hookBackupFunc func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, uploaderCfg map[string]string, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) @@ -102,6 +94,14 @@ func TestRunBackup(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + mockBRepo := udmrepomocks.NewBackupRepo(t) + mockBRepo.On("GetAdvancedFeatures").Return(udmrepo.AdvancedFeatureInfo{}) + + var kp kopiaProvider + kp.log = logrus.New() + kp.bkRepo = mockBRepo + updater := FakeBackupProgressUpdater{PodVolumeBackup: &velerov1api.PodVolumeBackup{}, Log: kp.log, Ctx: context.Background(), Cli: fake.NewClientBuilder().WithScheme(util.VeleroScheme).Build()} + if tc.volMode == "" { tc.volMode = uploader.PersistentVolumeFilesystem } @@ -117,10 +117,6 @@ func TestRunBackup(t *testing.T) { } func TestRunRestore(t *testing.T) { - var kp kopiaProvider - kp.log = logrus.New() - updater := FakeRestoreProgressUpdater{PodVolumeRestore: &velerov1api.PodVolumeRestore{}, Log: kp.log, Ctx: context.Background(), Cli: fake.NewClientBuilder().WithScheme(util.VeleroScheme).Build()} - testCases := []struct { name string hookRestoreFunc func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, volMode uploader.PersistentVolumeMode, uploaderCfg map[string]string, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) @@ -153,6 +149,10 @@ func TestRunRestore(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + var kp kopiaProvider + kp.log = logrus.New() + updater := FakeRestoreProgressUpdater{PodVolumeRestore: &velerov1api.PodVolumeRestore{}, Log: kp.log, Ctx: context.Background(), Cli: fake.NewClientBuilder().WithScheme(util.VeleroScheme).Build()} + if tc.volMode == "" { tc.volMode = uploader.PersistentVolumeFilesystem } diff --git a/pkg/util/velero/velero.go b/pkg/util/velero/velero.go index a4596584e..2af82cae3 100644 --- a/pkg/util/velero/velero.go +++ b/pkg/util/velero/velero.go @@ -75,6 +75,11 @@ func GetServiceAccountFromVeleroServer(deployment *appsv1.Deployment) string { return deployment.Spec.Template.Spec.ServiceAccountName } +// GetImagePullSecretsFromVeleroServer get the image pull secrets from the Velero server deployment +func GetImagePullSecretsFromVeleroServer(deployment *appsv1.Deployment) []v1.LocalObjectReference { + return deployment.Spec.Template.Spec.ImagePullSecrets +} + // getVeleroServerImage get the image of the Velero server deployment func GetVeleroServerImage(deployment *appsv1.Deployment) string { return deployment.Spec.Template.Spec.Containers[0].Image diff --git a/pkg/util/velero/velero_test.go b/pkg/util/velero/velero_test.go index 1fd9246a9..c17f61173 100644 --- a/pkg/util/velero/velero_test.go +++ b/pkg/util/velero/velero_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -582,6 +583,63 @@ func TestGetServiceAccountFromVeleroServer(t *testing.T) { } } +func TestGetImagePullSecretsFromVeleroServer(t *testing.T) { + tests := []struct { + name string + deploy *appsv1.Deployment + want []v1.LocalObjectReference + }{ + { + name: "no image pull secrets", + deploy: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + ServiceAccountName: "", + }, + }, + }, + }, + want: nil, + }, + { + name: "image pull secrets", + deploy: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + ImagePullSecrets: []v1.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + { + Name: "imagePullSecret2", + }, + }, + }, + }, + }, + }, + want: []v1.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + { + Name: "imagePullSecret2", + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := GetImagePullSecretsFromVeleroServer(test.deploy) + + require.Equal(t, test.want, got) + }) + } +} + func TestGetVeleroServerImage(t *testing.T) { tests := []struct { name string