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

Migrate backup sync controller from code-generator to kubebuilder #5218

Merged
merged 5 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions changelogs/unreleased/5218-jxun
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Migrate backup sync controller from code-generator to kubebuilder.
83 changes: 42 additions & 41 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import (
snapshotv1listers "github.com/kubernetes-csi/external-snapshotter/client/v4/listers/volumesnapshot/v1"

"github.com/vmware-tanzu/velero/internal/credentials"
"github.com/vmware-tanzu/velero/internal/storage"
"github.com/vmware-tanzu/velero/pkg/backup"
"github.com/vmware-tanzu/velero/pkg/buildinfo"
"github.com/vmware-tanzu/velero/pkg/client"
Expand All @@ -79,7 +80,6 @@ import (
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager"

"github.com/vmware-tanzu/velero/internal/storage"
"github.com/vmware-tanzu/velero/internal/util/managercontroller"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/podvolume"
Expand Down Expand Up @@ -604,29 +604,6 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string

csiVSLister, csiVSCLister, csiVSClassLister := s.getCSISnapshotListers()

backupSyncControllerRunInfo := func() controllerRunInfo {
backupSyncContoller := controller.NewBackupSyncController(
s.veleroClient.VeleroV1(),
s.mgr.GetClient(),
s.veleroClient.VeleroV1(),
s.sharedInformerFactory.Velero().V1().Backups().Lister(),
csiVSLister,
s.config.backupSyncPeriod,
s.namespace,
s.csiSnapshotClient,
s.kubeClient,
s.config.defaultBackupLocation,
newPluginManager,
backupStoreGetter,
s.logger,
)

return controllerRunInfo{
controller: backupSyncContoller,
numWorkers: defaultControllerWorkers,
}
}

backupTracker := controller.NewBackupTracker()

backupControllerRunInfo := func() controllerRunInfo {
Expand Down Expand Up @@ -714,16 +691,21 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
}
}

// By far, PodVolumeBackup, PodVolumeRestore, BackupStorageLocation controllers
// are not included in --disable-controllers list.
// This is because of PVB and PVR are used by Restic DaemonSet,
// and BSL controller is mandatory for Velero to work.
enabledControllers := map[string]func() controllerRunInfo{
controller.BackupSync: backupSyncControllerRunInfo,
controller.Backup: backupControllerRunInfo,
controller.Restore: restoreControllerRunInfo,
controller.Backup: backupControllerRunInfo,
controller.Restore: restoreControllerRunInfo,
}
// Note: all runtime type controllers that can be disabled are grouped separately, below:
enabledRuntimeControllers := make(map[string]struct{})
enabledRuntimeControllers[controller.ServerStatusRequest] = struct{}{}
enabledRuntimeControllers[controller.DownloadRequest] = struct{}{}
enabledRuntimeControllers[controller.GarbageCollection] = struct{}{}
enabledRuntimeControllers := map[string]struct{}{
controller.ServerStatusRequest: {},
controller.DownloadRequest: {},
controller.GarbageCollection: {},
controller.BackupSync: {},
}

if s.config.restoreOnly {
s.logger.Info("Restore only mode - not starting the backup, schedule, delete-backup, or GC controllers")
Expand All @@ -736,7 +718,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
}

// Remove disabled controllers so they are not initialized. If a match is not found we want
// to hault the system so the user knows this operation was not possible.
// to halt the system so the user knows this operation was not possible.
if err := removeControllers(s.config.disabledControllers, enabledControllers, enabledRuntimeControllers, s.logger); err != nil {
log.Fatal(err, "unable to disable a controller")
}
Expand Down Expand Up @@ -770,18 +752,18 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
s.logger.WithField("informer", informer).Info("Informer cache synced")
}

bslr := controller.BackupStorageLocationReconciler{
Ctx: s.ctx,
Client: s.mgr.GetClient(),
Scheme: s.mgr.GetScheme(),
DefaultBackupLocationInfo: storage.DefaultBackupLocationInfo{
bslr := controller.NewBackupStorageLocationReconciler(
s.ctx,
s.mgr.GetClient(),
s.mgr.GetScheme(),
storage.DefaultBackupLocationInfo{
StorageLocation: s.config.defaultBackupLocation,
ServerValidationFrequency: s.config.storeValidationFrequency,
},
NewPluginManager: newPluginManager,
BackupStoreGetter: backupStoreGetter,
Log: s.logger,
}
newPluginManager,
backupStoreGetter,
s.logger,
)
if err := bslr.SetupWithManager(s.mgr); err != nil {
s.logger.Fatal(err, "unable to create controller", "controller", controller.BackupStorageLocation)
}
Expand Down Expand Up @@ -835,6 +817,25 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
}
}

if _, ok := enabledRuntimeControllers[controller.BackupSync]; ok {
syncPeriod := s.config.backupSyncPeriod
if syncPeriod <= 0 {
syncPeriod = time.Minute
}

backupSyncReconciler := controller.NewBackupSyncReconciler(
s.mgr.GetClient(),
s.namespace,
syncPeriod,
newPluginManager,
backupStoreGetter,
s.logger,
)
if err := backupSyncReconciler.SetupWithManager(s.mgr); err != nil {
s.logger.Fatal(err, " unable to create controller ", "controller ", controller.BackupSync)
}
}

if _, ok := enabledRuntimeControllers[controller.GarbageCollection]; ok {
r := controller.NewGCReconciler(s.logger, s.mgr.GetClient())
if err := r.SetupWithManager(s.mgr); err != nil {
Expand Down
60 changes: 40 additions & 20 deletions pkg/controller/backup_storage_location_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,35 +44,55 @@ const (
)

// BackupStorageLocationReconciler reconciles a BackupStorageLocation object
type BackupStorageLocationReconciler struct {
Ctx context.Context
Client client.Client
Scheme *runtime.Scheme
DefaultBackupLocationInfo storage.DefaultBackupLocationInfo
type backupStorageLocationReconciler struct {
ctx context.Context
client client.Client
scheme *runtime.Scheme
defaultBackupLocationInfo storage.DefaultBackupLocationInfo
// use variables to refer to these functions so they can be
// replaced with fakes for testing.
NewPluginManager func(logrus.FieldLogger) clientmgmt.Manager
BackupStoreGetter persistence.ObjectBackupStoreGetter
newPluginManager func(logrus.FieldLogger) clientmgmt.Manager
backupStoreGetter persistence.ObjectBackupStoreGetter

Log logrus.FieldLogger
log logrus.FieldLogger
}

// NewBackupStorageLocationReconciler initialize and return a backupStorageLocationReconciler struct
func NewBackupStorageLocationReconciler(
ctx context.Context,
client client.Client,
scheme *runtime.Scheme,
defaultBackupLocationInfo storage.DefaultBackupLocationInfo,
newPluginManager func(logrus.FieldLogger) clientmgmt.Manager,
backupStoreGetter persistence.ObjectBackupStoreGetter,
log logrus.FieldLogger) *backupStorageLocationReconciler {
return &backupStorageLocationReconciler{
ctx: ctx,
client: client,
scheme: scheme,
defaultBackupLocationInfo: defaultBackupLocationInfo,
newPluginManager: newPluginManager,
backupStoreGetter: backupStoreGetter,
log: log,
}
}

// +kubebuilder:rbac:groups=velero.io,resources=backupstoragelocations,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=velero.io,resources=backupstoragelocations/status,verbs=get;update;patch
func (r *BackupStorageLocationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
func (r *backupStorageLocationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
var unavailableErrors []string
var location velerov1api.BackupStorageLocation

log := r.Log.WithField("controller", BackupStorageLocation).WithField(BackupStorageLocation, req.NamespacedName.String())
log := r.log.WithField("controller", BackupStorageLocation).WithField(BackupStorageLocation, req.NamespacedName.String())
log.Debug("Validating availability of BackupStorageLocation")

locationList, err := storage.ListBackupStorageLocations(r.Ctx, r.Client, req.Namespace)
locationList, err := storage.ListBackupStorageLocations(r.ctx, r.client, req.Namespace)
if err != nil {
log.WithError(err).Error("No BackupStorageLocations found, at least one is required")
return ctrl.Result{}, nil
}

pluginManager := r.NewPluginManager(log)
pluginManager := r.newPluginManager(log)
defer pluginManager.CleanupClients()

var defaultFound bool
Expand All @@ -93,7 +113,7 @@ func (r *BackupStorageLocationReconciler) Reconcile(ctx context.Context, req ctr
isDefault := location.Spec.Default

// TODO(2.0) remove this check since the server default will be deprecated
if !defaultFound && location.Name == r.DefaultBackupLocationInfo.StorageLocation {
if !defaultFound && location.Name == r.defaultBackupLocationInfo.StorageLocation {
// For backward-compatible, to configure the backup storage location as the default if
// none of the BSLs be marked as the default and the BSL name matches against the
// "velero server --default-backup-storage-location".
Expand All @@ -117,12 +137,12 @@ func (r *BackupStorageLocationReconciler) Reconcile(ctx context.Context, req ctr
location.Status.Phase = velerov1api.BackupStorageLocationPhaseAvailable
location.Status.Message = ""
}
if err := r.Client.Patch(r.Ctx, &location, client.MergeFrom(original)); err != nil {
if err := r.client.Patch(r.ctx, &location, client.MergeFrom(original)); err != nil {
log.WithError(err).Error("Error updating BackupStorageLocation phase")
}
}()

backupStore, err := r.BackupStoreGetter.Get(&location, pluginManager, log)
backupStore, err := r.backupStoreGetter.Get(&location, pluginManager, log)
if err != nil {
log.WithError(err).Error("Error getting a backup store")
return
Expand All @@ -144,11 +164,11 @@ func (r *BackupStorageLocationReconciler) Reconcile(ctx context.Context, req ctr
return ctrl.Result{}, nil
}

func (r *BackupStorageLocationReconciler) logReconciledPhase(defaultFound bool, locationList velerov1api.BackupStorageLocationList, errs []string) {
func (r *backupStorageLocationReconciler) logReconciledPhase(defaultFound bool, locationList velerov1api.BackupStorageLocationList, errs []string) {
var availableBSLs []*velerov1api.BackupStorageLocation
var unAvailableBSLs []*velerov1api.BackupStorageLocation
var unknownBSLs []*velerov1api.BackupStorageLocation
log := r.Log.WithField("controller", BackupStorageLocation)
log := r.log.WithField("controller", BackupStorageLocation)

for i, location := range locationList.Items {
phase := location.Status.Phase
Expand Down Expand Up @@ -181,16 +201,16 @@ func (r *BackupStorageLocationReconciler) logReconciledPhase(defaultFound bool,
}
}

func (r *BackupStorageLocationReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *backupStorageLocationReconciler) SetupWithManager(mgr ctrl.Manager) error {
g := kube.NewPeriodicalEnqueueSource(
r.Log,
r.log,
mgr.GetClient(),
&velerov1api.BackupStorageLocationList{},
bslValidationEnqueuePeriod,
// Add filter function to enqueue BSL per ValidationFrequency setting.
func(object client.Object) bool {
location := object.(*velerov1api.BackupStorageLocation)
return storage.IsReadyToValidate(location.Spec.ValidationFrequency, location.Status.LastValidationTime, r.DefaultBackupLocationInfo.ServerValidationFrequency, r.Log.WithField("controller", BackupStorageLocation))
return storage.IsReadyToValidate(location.Spec.ValidationFrequency, location.Status.LastValidationTime, r.defaultBackupLocationInfo.ServerValidationFrequency, r.log.WithField("controller", BackupStorageLocation))
},
)
return ctrl.NewControllerManagedBy(mgr).
Expand Down
32 changes: 16 additions & 16 deletions pkg/controller/backup_storage_location_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,16 @@ var _ = Describe("Backup Storage Location Reconciler", func() {

// Setup reconciler
Expect(velerov1api.AddToScheme(scheme.Scheme)).To(Succeed())
r := BackupStorageLocationReconciler{
Ctx: ctx,
Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(locations).Build(),
DefaultBackupLocationInfo: storage.DefaultBackupLocationInfo{
r := backupStorageLocationReconciler{
ctx: ctx,
client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(locations).Build(),
defaultBackupLocationInfo: storage.DefaultBackupLocationInfo{
StorageLocation: "location-1",
ServerValidationFrequency: 0,
},
NewPluginManager: func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
BackupStoreGetter: NewFakeObjectBackupStoreGetter(backupStores),
Log: velerotest.NewLogger(),
newPluginManager: func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
backupStoreGetter: NewFakeObjectBackupStoreGetter(backupStores),
log: velerotest.NewLogger(),
}

// Assertions
Expand All @@ -101,7 +101,7 @@ var _ = Describe("Backup Storage Location Reconciler", func() {

key := client.ObjectKey{Name: location.Name, Namespace: location.Namespace}
instance := &velerov1api.BackupStorageLocation{}
err = r.Client.Get(ctx, key, instance)
err = r.client.Get(ctx, key, instance)
Expect(err).To(BeNil())
Expect(instance.Spec.Default).To(BeIdenticalTo(tests[i].expectedIsDefault))
Expect(instance.Status.Phase).To(BeIdenticalTo(tests[i].expectedPhase))
Expand Down Expand Up @@ -144,16 +144,16 @@ var _ = Describe("Backup Storage Location Reconciler", func() {

// Setup reconciler
Expect(velerov1api.AddToScheme(scheme.Scheme)).To(Succeed())
r := BackupStorageLocationReconciler{
Ctx: ctx,
Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(locations).Build(),
DefaultBackupLocationInfo: storage.DefaultBackupLocationInfo{
r := backupStorageLocationReconciler{
ctx: ctx,
client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(locations).Build(),
defaultBackupLocationInfo: storage.DefaultBackupLocationInfo{
StorageLocation: "default",
ServerValidationFrequency: 0,
},
NewPluginManager: func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
BackupStoreGetter: NewFakeObjectBackupStoreGetter(backupStores),
Log: velerotest.NewLogger(),
newPluginManager: func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
backupStoreGetter: NewFakeObjectBackupStoreGetter(backupStores),
log: velerotest.NewLogger(),
}

// Assertions
Expand All @@ -166,7 +166,7 @@ var _ = Describe("Backup Storage Location Reconciler", func() {

key := client.ObjectKey{Name: location.Name, Namespace: location.Namespace}
instance := &velerov1api.BackupStorageLocation{}
err = r.Client.Get(ctx, key, instance)
err = r.client.Get(ctx, key, instance)
Expect(err).To(BeNil())
Expect(instance.Spec.Default).To(BeIdenticalTo(tests[i].expectedIsDefault))
}
Expand Down
Loading