From 770ff142d7b563b63e20f728ec4849a4f549db1f Mon Sep 17 00:00:00 2001 From: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com> Date: Thu, 24 Jul 2025 01:55:16 +0800 Subject: [PATCH] Add imagePullSecrets inheritage for VGDP pod and maintenance job. (#9096) Signed-off-by: Xun Jiang --- changelogs/unreleased/9096-blackpiglet | 1 + pkg/exposer/csi_snapshot.go | 1 + pkg/exposer/generic_restore.go | 1 + pkg/exposer/image.go | 23 ++++---- pkg/exposer/image_test.go | 10 ++++ pkg/exposer/pod_volume.go | 3 + pkg/repository/maintenance/maintenance.go | 15 ++++- .../maintenance/maintenance_test.go | 29 +++++++++- pkg/uploader/provider/kopia_test.go | 24 ++++---- pkg/util/velero/velero.go | 5 ++ pkg/util/velero/velero_test.go | 57 +++++++++++++++++++ 11 files changed, 144 insertions(+), 25 deletions(-) create mode 100644 changelogs/unreleased/9096-blackpiglet diff --git a/changelogs/unreleased/9096-blackpiglet b/changelogs/unreleased/9096-blackpiglet new file mode 100644 index 000000000..87a714b55 --- /dev/null +++ b/changelogs/unreleased/9096-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 58748a105..bee42a61c 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -687,6 +687,7 @@ func (e *csiSnapshotExposer) createBackupPod( Tolerations: toleration, DNSPolicy: podInfo.dnsPolicy, DNSConfig: podInfo.dnsConfig, + ImagePullSecrets: podInfo.imagePullSecrets, }, } diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index fe658008b..42ba04c3e 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -558,6 +558,7 @@ func (e *genericRestoreExposer) createRestorePod( DNSPolicy: podInfo.dnsPolicy, DNSConfig: podInfo.dnsConfig, Affinity: podAffinity, + ImagePullSecrets: podInfo.imagePullSecrets, }, } diff --git a/pkg/exposer/image.go b/pkg/exposer/image.go index 11d23bf8c..58658e1b7 100644 --- a/pkg/exposer/image.go +++ b/pkg/exposer/image.go @@ -28,16 +28,17 @@ import ( ) type inheritedPodInfo struct { - image string - serviceAccount string - env []corev1api.EnvVar - envFrom []corev1api.EnvFromSource - volumeMounts []corev1api.VolumeMount - volumes []corev1api.Volume - logLevelArgs []string - logFormatArgs []string - dnsPolicy corev1api.DNSPolicy - dnsConfig *corev1api.PodDNSConfig + image string + serviceAccount string + env []corev1api.EnvVar + envFrom []corev1api.EnvFromSource + volumeMounts []corev1api.VolumeMount + volumes []corev1api.Volume + logLevelArgs []string + logFormatArgs []string + dnsPolicy corev1api.DNSPolicy + dnsConfig *corev1api.PodDNSConfig + imagePullSecrets []corev1api.LocalObjectReference } func getInheritedPodInfo(ctx context.Context, client kubernetes.Interface, veleroNamespace string, osType string) (inheritedPodInfo, error) { @@ -76,5 +77,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 e793f3c8d..5c47f5c04 100644 --- a/pkg/exposer/image_test.go +++ b/pkg/exposer/image_test.go @@ -177,6 +177,11 @@ func TestGetInheritedPodInfo(t *testing.T) { }, }, ServiceAccountName: "sa-1", + ImagePullSecrets: []corev1api.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, }, }, @@ -317,6 +322,11 @@ func TestGetInheritedPodInfo(t *testing.T) { "--log-level", "debug", }, + imagePullSecrets: []corev1api.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, }, } diff --git a/pkg/exposer/pod_volume.go b/pkg/exposer/pod_volume.go index d34972b9d..bdde55175 100644 --- a/pkg/exposer/pod_volume.go +++ b/pkg/exposer/pod_volume.go @@ -387,6 +387,9 @@ func (e *podVolumeExposer) createHostingPod(ctx context.Context, ownerObject cor RestartPolicy: corev1api.RestartPolicyNever, SecurityContext: securityCtx, Tolerations: toleration, + DNSPolicy: podInfo.dnsPolicy, + DNSConfig: podInfo.dnsConfig, + ImagePullSecrets: podInfo.imagePullSecrets, }, } diff --git a/pkg/repository/maintenance/maintenance.go b/pkg/repository/maintenance/maintenance.go index 35d7bd99e..8c6dc2cc3 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) (*batchv1api.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, +) (*batchv1api.Job, error) { // Get the Velero server deployment deployment := &appsv1api.Deployment{} err := cli.Get(ctx, types.NamespacedName{Name: "velero", Namespace: repo.Namespace}, deployment) @@ -437,6 +445,8 @@ func buildJob(cli client.Client, ctx context.Context, repo *velerov1api.BackupRe // Get the pod security context from the Velero server deployment podSecurityContext := veleroutil.GetPodSecurityContextsFromVeleroServer(deployment) + imagePullSecrets := veleroutil.GetImagePullSecretsFromVeleroServer(deployment) + // Get image image := veleroutil.GetVeleroServerImage(deployment) @@ -528,6 +538,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 33e4afd31..48fd58ed6 100644 --- a/pkg/repository/maintenance/maintenance_test.go +++ b/pkg/repository/maintenance/maintenance_test.go @@ -910,6 +910,11 @@ func TestBuildJob(t *testing.T) { }, }, }, + ImagePullSecrets: []corev1api.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, }, }, @@ -934,6 +939,7 @@ func TestBuildJob(t *testing.T) { expectedPodLabel map[string]string expectedSecurityContext *corev1api.SecurityContext expectedPodSecurityContext *corev1api.PodSecurityContext + expectedImagePullSecrets []corev1api.LocalObjectReference }{ { name: "Valid maintenance job without third party labels", @@ -981,6 +987,11 @@ func TestBuildJob(t *testing.T) { expectedPodSecurityContext: &corev1api.PodSecurityContext{ RunAsNonRoot: boolptr.True(), }, + expectedImagePullSecrets: []corev1api.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, { name: "Valid maintenance job with third party labels", @@ -1025,6 +1036,11 @@ func TestBuildJob(t *testing.T) { }, expectedSecurityContext: nil, expectedPodSecurityContext: nil, + expectedImagePullSecrets: []corev1api.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + }, }, { name: "Error getting Velero server deployment", @@ -1076,7 +1092,16 @@ func TestBuildJob(t *testing.T) { cli := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objs...).Build() // Call the function to test - job, err := buildJob(cli, t.Context(), param.BackupRepo, param.BackupLocation.Name, tc.m, *tc.m.PodResources, tc.logLevel, tc.logFormat) + job, err := buildJob( + cli, + t.Context(), + param.BackupRepo, + param.BackupLocation.Name, + tc.m, + *tc.m.PodResources, + tc.logLevel, + tc.logFormat, + ) // Check the error if tc.expectedError { @@ -1131,6 +1156,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 97f9eb98a..6231d5d62 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: t.Context(), 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: t.Context(), 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: t.Context(), 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: t.Context(), 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 d4e0f70cb..a58d6e76b 100644 --- a/pkg/util/velero/velero.go +++ b/pkg/util/velero/velero.go @@ -89,6 +89,11 @@ func GetServiceAccountFromVeleroServer(deployment *appsv1api.Deployment) string return deployment.Spec.Template.Spec.ServiceAccountName } +// GetImagePullSecretsFromVeleroServer get the image pull secrets from the Velero server deployment +func GetImagePullSecretsFromVeleroServer(deployment *appsv1api.Deployment) []corev1api.LocalObjectReference { + return deployment.Spec.Template.Spec.ImagePullSecrets +} + // getVeleroServerImage get the image of the Velero server deployment func GetVeleroServerImage(deployment *appsv1api.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 42948d890..c4e201780 100644 --- a/pkg/util/velero/velero_test.go +++ b/pkg/util/velero/velero_test.go @@ -679,6 +679,63 @@ func TestGetServiceAccountFromVeleroServer(t *testing.T) { } } +func TestGetImagePullSecretsFromVeleroServer(t *testing.T) { + tests := []struct { + name string + deploy *appsv1api.Deployment + want []corev1api.LocalObjectReference + }{ + { + name: "no image pull secrets", + deploy: &appsv1api.Deployment{ + Spec: appsv1api.DeploymentSpec{ + Template: corev1api.PodTemplateSpec{ + Spec: corev1api.PodSpec{ + ServiceAccountName: "", + }, + }, + }, + }, + want: nil, + }, + { + name: "image pull secrets", + deploy: &appsv1api.Deployment{ + Spec: appsv1api.DeploymentSpec{ + Template: corev1api.PodTemplateSpec{ + Spec: corev1api.PodSpec{ + ImagePullSecrets: []corev1api.LocalObjectReference{ + { + Name: "imagePullSecret1", + }, + { + Name: "imagePullSecret2", + }, + }, + }, + }, + }, + }, + want: []corev1api.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