Skip to content

Commit

Permalink
Remove the Velero generated client from pkg/podvolume directory.
Browse files Browse the repository at this point in the history
Signed-off-by: Xun Jiang <[email protected]>
  • Loading branch information
Xun Jiang committed Nov 1, 2023
1 parent 3e88469 commit d7ad365
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 148 deletions.
10 changes: 2 additions & 8 deletions pkg/client/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,14 @@ import (
)

func CreateRetryGenerateName(client kbclient.Client, ctx context.Context, obj kbclient.Object) error {
return CreateRetryGenerateNameWithFunc(obj, func() error {
return client.Create(ctx, obj, &kbclient.CreateOptions{})
})
}

func CreateRetryGenerateNameWithFunc(obj kbclient.Object, createFn func() error) error {
retryCreateFn := func() error {
// needed to ensure that the name from the failed create isn't left on the object between retries
obj.SetName("")
return createFn()
return client.Create(ctx, obj, &kbclient.CreateOptions{})

Check warning on line 31 in pkg/client/retry.go

View check run for this annotation

Codecov / codecov/patch

pkg/client/retry.go#L31

Added line #L31 was not covered by tests
}
if obj.GetGenerateName() != "" && obj.GetName() == "" {
return retry.OnError(retry.DefaultRetry, apierrors.IsAlreadyExists, retryCreateFn)
} else {
return createFn()
return client.Create(ctx, obj, &kbclient.CreateOptions{})

Check warning on line 36 in pkg/client/retry.go

View check run for this annotation

Codecov / codecov/patch

pkg/client/retry.go#L36

Added line #L36 was not covered by tests
}
}
16 changes: 10 additions & 6 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ import (
"github.com/vmware-tanzu/velero/pkg/controller"
velerodiscovery "github.com/vmware-tanzu/velero/pkg/discovery"
"github.com/vmware-tanzu/velero/pkg/features"
clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned"
"github.com/vmware-tanzu/velero/pkg/itemoperationmap"
"github.com/vmware-tanzu/velero/pkg/metrics"
"github.com/vmware-tanzu/velero/pkg/nodeagent"
Expand Down Expand Up @@ -250,12 +249,12 @@ type server struct {
metricsAddress string
kubeClientConfig *rest.Config
kubeClient kubernetes.Interface
veleroClient clientset.Interface
discoveryClient discovery.DiscoveryInterface
discoveryHelper velerodiscovery.Helper
dynamicClient dynamic.Interface
csiSnapshotClient *snapshotv1client.Clientset
csiSnapshotLister snapshotv1listers.VolumeSnapshotLister
crClient ctrlclient.Client
crWatchClient ctrlclient.WithWatch
ctx context.Context
cancelFunc context.CancelFunc
Expand Down Expand Up @@ -306,6 +305,11 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s
return nil, err
}

crClient, err := f.KubebuilderClient()
if err != nil {
return nil, err
}

Check warning on line 311 in pkg/cmd/server/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/server/server.go#L308-L311

Added lines #L308 - L311 were not covered by tests

crWatchClient, err := f.KubebuilderWatchClient()
if err != nil {
return nil, err
Expand Down Expand Up @@ -386,9 +390,9 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s
metricsAddress: config.metricsAddress,
kubeClientConfig: clientConfig,
kubeClient: kubeClient,
veleroClient: veleroClient,
discoveryClient: veleroClient.Discovery(),
dynamicClient: dynamicClient,
crClient: crClient,
crWatchClient: crWatchClient,

Check warning on line 396 in pkg/cmd/server/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/server/server.go#L395-L396

Added lines #L395 - L396 were not covered by tests
ctx: ctx,
cancelFunc: cancelFunc,
Expand Down Expand Up @@ -743,10 +747,10 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
podvolume.NewBackupperFactory(
s.repoLocker,
s.repoEnsurer,
s.veleroClient,
s.kubeClient.CoreV1(),
s.kubeClient.CoreV1(),
s.kubeClient.CoreV1(),
s.crClient,
s.crWatchClient,

Check warning on line 754 in pkg/cmd/server/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/server/server.go#L753-L754

Added lines #L753 - L754 were not covered by tests
s.logger,
),
Expand Down Expand Up @@ -826,10 +830,10 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
podvolume.NewBackupperFactory(
s.repoLocker,
s.repoEnsurer,
s.veleroClient,
s.kubeClient.CoreV1(),
s.kubeClient.CoreV1(),
s.kubeClient.CoreV1(),
s.crClient,
s.crWatchClient,

Check warning on line 837 in pkg/cmd/server/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/server/server.go#L836-L837

Added lines #L836 - L837 were not covered by tests
s.logger,
),
Expand Down Expand Up @@ -927,10 +931,10 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
podvolume.NewRestorerFactory(
s.repoLocker,
s.repoEnsurer,
s.veleroClient,
s.kubeClient.CoreV1(),
s.kubeClient.CoreV1(),
s.kubeClient,
s.crClient,
s.crWatchClient,

Check warning on line 938 in pkg/cmd/server/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/server/server.go#L937-L938

Added lines #L937 - L938 were not covered by tests
s.logger,
),
Expand Down
18 changes: 5 additions & 13 deletions pkg/podvolume/backupper.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/tools/cache"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/vmware-tanzu/velero/internal/resourcepolicies"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
veleroclient "github.com/vmware-tanzu/velero/pkg/client"
clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned"
"github.com/vmware-tanzu/velero/pkg/label"
"github.com/vmware-tanzu/velero/pkg/nodeagent"
"github.com/vmware-tanzu/velero/pkg/repository"
Expand All @@ -50,10 +50,10 @@ type backupper struct {
ctx context.Context
repoLocker *repository.RepoLocker
repoEnsurer *repository.Ensurer
veleroClient clientset.Interface
pvcClient corev1client.PersistentVolumeClaimsGetter
pvClient corev1client.PersistentVolumesGetter
podClient corev1client.PodsGetter
crClient ctrlclient.Client
uploaderType string

results map[string]chan *velerov1api.PodVolumeBackup
Expand Down Expand Up @@ -104,10 +104,10 @@ func newBackupper(
repoLocker *repository.RepoLocker,
repoEnsurer *repository.Ensurer,
pvbInformer cache.SharedInformer,
veleroClient clientset.Interface,
pvcClient corev1client.PersistentVolumeClaimsGetter,
pvClient corev1client.PersistentVolumesGetter,
podClient corev1client.PodsGetter,
crClient ctrlclient.Client,
uploaderType string,
backup *velerov1api.Backup,
log logrus.FieldLogger,
Expand All @@ -116,10 +116,10 @@ func newBackupper(
ctx: ctx,
repoLocker: repoLocker,
repoEnsurer: repoEnsurer,
veleroClient: veleroClient,
pvcClient: pvcClient,
pvClient: pvClient,
podClient: podClient,
crClient: crClient,
uploaderType: uploaderType,

results: make(map[string]chan *velerov1api.PodVolumeBackup),
Expand Down Expand Up @@ -308,11 +308,7 @@ func (b *backupper) BackupPodVolumes(backup *velerov1api.Backup, pod *corev1api.
}

volumeBackup := newPodVolumeBackup(backup, pod, volume, repoIdentifier, b.uploaderType, pvc)
// TODO: once backupper is refactored to use controller-runtime, just pass client instead of anonymous func
if err := veleroclient.CreateRetryGenerateNameWithFunc(volumeBackup, func() error {
_, err := b.veleroClient.VeleroV1().PodVolumeBackups(volumeBackup.Namespace).Create(context.TODO(), volumeBackup, metav1.CreateOptions{})
return err
}); err != nil {
if err := veleroclient.CreateRetryGenerateName(b.crClient, b.ctx, volumeBackup); err != nil {
errs = append(errs, err)
continue
}
Expand Down Expand Up @@ -426,7 +422,3 @@ func newPodVolumeBackup(backup *velerov1api.Backup, pod *corev1api.Pod, volume c

return pvb
}

func errorOnly(_ interface{}, err error) error {
return err
}
9 changes: 4 additions & 5 deletions pkg/podvolume/backupper_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned"
"github.com/vmware-tanzu/velero/pkg/repository"
"github.com/vmware-tanzu/velero/pkg/util/kube"
)
Expand All @@ -41,20 +40,20 @@ type BackupperFactory interface {
func NewBackupperFactory(
repoLocker *repository.RepoLocker,
repoEnsurer *repository.Ensurer,
veleroClient clientset.Interface,
pvcClient corev1client.PersistentVolumeClaimsGetter,
pvClient corev1client.PersistentVolumesGetter,
podClient corev1client.PodsGetter,
crClient ctrlclient.Client,
crWatchClient ctrlclient.WithWatch,
log logrus.FieldLogger,
) BackupperFactory {
return &backupperFactory{
repoLocker: repoLocker,
repoEnsurer: repoEnsurer,
veleroClient: veleroClient,
pvcClient: pvcClient,
pvClient: pvClient,
podClient: podClient,
crClient: crClient,
crWatchClient: crWatchClient,
log: log,
}
Expand All @@ -63,10 +62,10 @@ func NewBackupperFactory(
type backupperFactory struct {
repoLocker *repository.RepoLocker
repoEnsurer *repository.Ensurer
veleroClient clientset.Interface
pvcClient corev1client.PersistentVolumeClaimsGetter
pvClient corev1client.PersistentVolumesGetter
podClient corev1client.PodsGetter
crClient ctrlclient.Client
crWatchClient ctrlclient.WithWatch
log logrus.FieldLogger
}
Expand All @@ -80,7 +79,7 @@ func (bf *backupperFactory) NewBackupper(ctx context.Context, backup *velerov1ap

informer := cache.NewSharedInformer(&lw, &velerov1api.PodVolumeBackup{}, time.Second)

b := newBackupper(ctx, bf.repoLocker, bf.repoEnsurer, informer, bf.veleroClient, bf.pvcClient, bf.pvClient, bf.podClient, uploaderType, backup, bf.log)
b := newBackupper(ctx, bf.repoLocker, bf.repoEnsurer, informer, bf.pvcClient, bf.pvClient, bf.podClient, bf.crClient, uploaderType, backup, bf.log)

go informer.Run(ctx.Done())
if !cache.WaitForCacheSync(ctx.Done(), informer.HasSynced) {
Expand Down
47 changes: 3 additions & 44 deletions pkg/podvolume/backupper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ import (
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned"
velerofake "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/fake"
"github.com/vmware-tanzu/velero/pkg/repository"
velerotest "github.com/vmware-tanzu/velero/pkg/test"
)
Expand Down Expand Up @@ -509,40 +507,6 @@ func TestBackupPodVolumes(t *testing.T) {
uploaderType: "kopia",
bsl: "fake-bsl",
},
{
name: "create PVB fail",
volumes: []string{
"fake-volume-1",
"fake-volume-2",
},
sourcePod: createPodObj(true, true, true, 2),
kubeClientObj: []runtime.Object{
createNodeAgentPodObj(true),
createPVCObj(1),
createPVCObj(2),
createPVObj(1, false),
createPVObj(2, false),
},
ctlClientObj: []runtime.Object{
createBackupRepoObj(),
},
veleroReactors: []reactor{
{
verb: "create",
resource: "podvolumebackups",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, errors.New("fake-create-error")
},
},
},
runtimeScheme: scheme,
uploaderType: "kopia",
bsl: "fake-bsl",
errs: []string{
"fake-create-error",
"fake-create-error",
},
},
{
name: "context cancelled",
ctx: ctxWithCancel,
Expand Down Expand Up @@ -630,25 +594,20 @@ func TestBackupPodVolumes(t *testing.T) {
fakeClientBuilder = fakeClientBuilder.WithScheme(test.runtimeScheme)
}

fakeCtlClient := fakeClientBuilder.WithRuntimeObjects(test.ctlClientObj...).Build()
objList := append(test.ctlClientObj, test.veleroClientObj...)
fakeCtlClient := fakeClientBuilder.WithRuntimeObjects(objList...).Build()

fakeKubeClient := kubefake.NewSimpleClientset(test.kubeClientObj...)
var kubeClient kubernetes.Interface = fakeKubeClient

fakeVeleroClient := velerofake.NewSimpleClientset(test.veleroClientObj...)
for _, reactor := range test.veleroReactors {
fakeVeleroClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc)
}
var veleroClient versioned.Interface = fakeVeleroClient

fakeCRWatchClient := velerotest.NewFakeControllerRuntimeWatchClient(t, test.kubeClientObj...)

ensurer := repository.NewEnsurer(fakeCtlClient, velerotest.NewLogger(), time.Millisecond)

backupObj := builder.ForBackup(velerov1api.DefaultNamespace, "fake-backup").Result()
backupObj.Spec.StorageLocation = test.bsl

factory := NewBackupperFactory(repository.NewRepoLocker(), ensurer, veleroClient, kubeClient.CoreV1(), kubeClient.CoreV1(), kubeClient.CoreV1(), fakeCRWatchClient, velerotest.NewLogger())
factory := NewBackupperFactory(repository.NewRepoLocker(), ensurer, kubeClient.CoreV1(), kubeClient.CoreV1(), kubeClient.CoreV1(), fakeCtlClient, fakeCRWatchClient, velerotest.NewLogger())
bp, err := factory.NewBackupper(ctx, backupObj, test.uploaderType)

require.NoError(t, err)
Expand Down
37 changes: 17 additions & 20 deletions pkg/podvolume/restorer.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ import (
"k8s.io/client-go/kubernetes"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/tools/cache"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
veleroclient "github.com/vmware-tanzu/velero/pkg/client"
clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned"
"github.com/vmware-tanzu/velero/pkg/label"
"github.com/vmware-tanzu/velero/pkg/nodeagent"
"github.com/vmware-tanzu/velero/pkg/repository"
Expand All @@ -54,13 +54,13 @@ type Restorer interface {
}

type restorer struct {
ctx context.Context
repoLocker *repository.RepoLocker
repoEnsurer *repository.Ensurer
veleroClient clientset.Interface
pvcClient corev1client.PersistentVolumeClaimsGetter
podClient corev1client.PodsGetter
kubeClient kubernetes.Interface
ctx context.Context
repoLocker *repository.RepoLocker
repoEnsurer *repository.Ensurer
pvcClient corev1client.PersistentVolumeClaimsGetter
podClient corev1client.PodsGetter
kubeClient kubernetes.Interface
crClient ctrlclient.Client

resultsLock sync.Mutex
results map[string]chan *velerov1api.PodVolumeRestore
Expand All @@ -73,21 +73,21 @@ func newRestorer(
repoLocker *repository.RepoLocker,
repoEnsurer *repository.Ensurer,
pvrInformer cache.SharedInformer,
veleroClient clientset.Interface,
pvcClient corev1client.PersistentVolumeClaimsGetter,
podClient corev1client.PodsGetter,
kubeClient kubernetes.Interface,
crClient ctrlclient.Client,
restore *velerov1api.Restore,
log logrus.FieldLogger,
) *restorer {
r := &restorer{
ctx: ctx,
repoLocker: repoLocker,
repoEnsurer: repoEnsurer,
veleroClient: veleroClient,
pvcClient: pvcClient,
podClient: podClient,
kubeClient: kubeClient,
ctx: ctx,
repoLocker: repoLocker,
repoEnsurer: repoEnsurer,
pvcClient: pvcClient,
podClient: podClient,
kubeClient: kubeClient,
crClient: crClient,

results: make(map[string]chan *velerov1api.PodVolumeRestore),
log: log,
Expand Down Expand Up @@ -183,10 +183,7 @@ func (r *restorer) RestorePodVolumes(data RestoreData) []error {

volumeRestore := newPodVolumeRestore(data.Restore, data.Pod, data.BackupLocation, volume, backupInfo.snapshotID, repoIdentifier, backupInfo.uploaderType, data.SourceNamespace, pvc)

// TODO: once restorer is refactored to use controller-runtime, just pass client instead of anonymous func
if err := veleroclient.CreateRetryGenerateNameWithFunc(volumeRestore, func() error {
return errorOnly(r.veleroClient.VeleroV1().PodVolumeRestores(volumeRestore.Namespace).Create(context.TODO(), volumeRestore, metav1.CreateOptions{}))
}); err != nil {
if err := veleroclient.CreateRetryGenerateName(r.crClient, r.ctx, volumeRestore); err != nil {
errs = append(errs, errors.WithStack(err))
continue
}
Expand Down
Loading

0 comments on commit d7ad365

Please sign in to comment.