Backup implementation: addressed comments

* remove velero watcher from mgmtbackup ctrl
* remove velero enabled flag
* remove mgmt.backup spec
* remove webhooks for mgmt/mgmtbackups
* indexer now caches schedules and incomplete backups
* mgmtbackup spec extended with schedule
* mgmtbackup ctrl now does not rely on mgmt object
* simplify mgmtbackup ctrl logic
* set error to mgmtbackup on meta API errors
* fix error with getting next attempt time

PROJECT

@@ -113,7 +113,4 @@ resources:
   kind: ManagementBackup
   path: github.com/K0rdent/kcm/api/v1alpha1
   version: v1alpha1
-  webhooks:
-      validation: true
-      webhookVersion: v1
 version: "3"

api/v1alpha1/indexers.go

@@ -35,7 +35,7 @@ func SetupIndexers(ctx context.Context, mgr ctrl.Manager) error {
 		setupClusterTemplateProvidersIndexer,
 		setupMultiClusterServiceServicesIndexer,
 		setupOwnerReferenceIndexers,
-		setupManagementBackupScheduledIndexer,
+		setupManagementBackupIndexer,
 	} {
 		merr = errors.Join(merr, f(ctx, mgr))
 	}
@@ -241,22 +241,26 @@ func extractOwnerReferences(rawObj client.Object) []string {
 
 // management backup indexers
 
-// ManagementBackupScheduledIndexKey indexer field name to extract only [ManagementBackup] objects
-// that meant to be scheduled.
-const ManagementBackupScheduledIndexKey = "k0rdent.scheduled-backup"
+// ManagementBackupIndexKey indexer field name to extract only [ManagementBackup] objects
+// that either has schedule or has NOT been completed yet.
+const ManagementBackupIndexKey = "k0rdent.management-backup"
 
-func setupManagementBackupScheduledIndexer(ctx context.Context, mgr ctrl.Manager) error {
-	return mgr.GetFieldIndexer().IndexField(ctx, &ManagementBackup{}, ManagementBackupScheduledIndexKey, func(o client.Object) []string {
+func setupManagementBackupIndexer(ctx context.Context, mgr ctrl.Manager) error {
+	return mgr.GetFieldIndexer().IndexField(ctx, &ManagementBackup{}, ManagementBackupIndexKey, func(o client.Object) []string {
 		mb, ok := o.(*ManagementBackup)
 		if !ok {
 			return nil
 		}
 
-		v, ok := mb.Annotations[ScheduleBackupAnnotation]
-		if !ok {
-			return nil
+		const trueVal = "true"
+		if mb.Spec.Schedule != "" {
+			return []string{trueVal}
 		}
 
-		return []string{v}
+		if mb.Status.LastBackup == nil || mb.Status.LastBackup.CompletionTimestamp.IsZero() {
+			return []string{trueVal}
+		}
+
+		return nil
 	})
 }

api/v1alpha1/management_backup_types.go

@@ -15,7 +15,6 @@
 package v1alpha1
 
 import (
-	"strconv"
 	"time"
 
 	velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
@@ -28,17 +27,16 @@ const (
 	GenericComponentNameLabel = "k0rdent.mirantis.com/component"
 	// Component label value for the KCM-related components.
 	GenericComponentLabelValueKCM = "kcm"
-
-	// ScheduleBackupAnnotation is an annotation for ease the listing
-	// of [ManagementBackup]. Indicates that the type of the object is schedule.
-	ScheduleBackupAnnotation = "k0rdent.mirantis.com/schedule"
 )
 
 // ManagementBackupSpec defines the desired state of ManagementBackup
 type ManagementBackupSpec struct {
 	// StorageLocation is the name of a [github.com/vmware-tanzu/velero/pkg/apis/velero/v1.StorageLocation]
 	// where the backup should be stored.
 	StorageLocation string `json:"storageLocation,omitempty"`
+	// Schedule is a Cron expression defining when to run the scheduled [ManagementBackup].
+	// If not set, the object is considered to be run only once.
+	Schedule string `json:"schedule,omitempty"`
 }
 
 // ManagementBackupStatus defines the observed state of ManagementBackup
@@ -52,17 +50,13 @@ type ManagementBackupStatus struct {
 	LastBackup *velerov1.BackupStatus `json:"lastBackup,omitempty"`
 	// Name of most recently created [github.com/vmware-tanzu/velero/pkg/apis/velero/v1.Backup].
 	LastBackupName string `json:"lastBackupName,omitempty"`
-	// Paused indicates if the schedule is currently paused.
-	Paused bool `json:"paused,omitempty"`
+	// Error stores messages in case of failed backup creation.
+	Error string `json:"error,omitempty"`
 }
 
 // IsSchedule checks if an instance of [ManagementBackup] is schedulable.
 func (s *ManagementBackup) IsSchedule() bool {
-	if _, err := strconv.ParseBool(s.Annotations[ScheduleBackupAnnotation]); err == nil {
-		return true
-	}
-
-	return false
+	return s.Spec.Schedule != ""
 }
 
 // TimestampedBackupName returns the backup name related to scheduled [ManagementBackup] based on the given timestamp.
@@ -77,7 +71,7 @@ func (s *ManagementBackup) TimestampedBackupName(timestamp time.Time) string {
 // +kubebuilder:printcolumn:name="NextBackup",type=string,JSONPath=`.status.nextAttempt`,description="Next scheduled attempt to back up",priority=0
 // +kubebuilder:printcolumn:name="SinceLastBackup",type=date,JSONPath=`.status.lastBackupTime`,description="Time elapsed since last backup run",priority=1
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,description="Time elapsed since object creation",priority=0
-// +kubebuilder:printcolumn:name="Paused",type=boolean,JSONPath=`.status.paused`,description="Schedule is on pause",priority=1
+// +kubebuilder:printcolumn:name="Error",type=string,JSONPath=`.status.error`,description="Error during creation",priority=1
 
 // ManagementBackup is the Schema for the managementbackups API
 type ManagementBackup struct {

api/v1alpha1/management_types.go

@@ -43,8 +43,6 @@ type ManagementSpec struct {
 
 	// Providers is the list of supported CAPI providers.
 	Providers []Provider `json:"providers,omitempty"`
-
-	Backup Backup `json:"backup,omitempty"`
 }
 
 // Core represents a structure describing core Management components.
@@ -55,24 +53,6 @@ type Core struct {
 	CAPI Component `json:"capi,omitempty"`
 }
 
-// Backup enables a feature to backup KCM objects into a cloud.
-type Backup struct {
-	// Schedule is a Cron expression defining when to run the scheduled [ManagementBackup].
-	// Default value is to backup at minute 0 past every 6th hour (0 */6 * * *).
-	Schedule string `json:"schedule,omitempty"`
-
-	// StorageLocation is the name of a [github.com/vmware-tanzu/velero/pkg/apis/velero/v1.StorageLocation]
-	// where the backup should be stored. It is propagated to the corresponding scheduled [ManagementBackup]
-	// unless set in the object.
-	StorageLocation string `json:"storageLocation,omitempty"`
-
-	// Flag to indicate whether the management cluster backup feature is enabled.
-	// The backup is done using [Velero].
-	//
-	// [Velero]: https://velero.io
-	Enabled bool `json:"enabled,omitempty"`
-}
-
 // Component represents KCM management component
 type Component struct {
 	// Config allows to provide parameters for management component customization.

cmd/main.go

@@ -90,7 +90,6 @@ func main() {
 		kcmTemplatesChartName     string
 		enableTelemetry           bool
 		enableWebhook             bool
-		enableVelero              bool
 		webhookPort               int
 		webhookCertDir            string
 	)
@@ -115,7 +114,6 @@ func main() {
 		"The name of the helm chart with KCM Templates.")
 	flag.BoolVar(&enableTelemetry, "enable-telemetry", true, "Collect and send telemetry data.")
 	flag.BoolVar(&enableWebhook, "enable-webhook", true, "Enable admission webhook.")
-	flag.BoolVar(&enableVelero, "enable-velero", true, "Enable Velero stack for management cluster backups.")
 	flag.IntVar(&webhookPort, "webhook-port", 9443, "Admission webhook port.")
 	flag.StringVar(&webhookCertDir, "webhook-cert-dir", "/tmp/k8s-webhook-server/serving-certs/",
 		"Webhook cert dir, only used when webhook-port is specified.")
@@ -293,14 +291,12 @@ func main() {
 		os.Exit(1)
 	}
 
-	if enableVelero {
-		if err = (&controller.ManagementBackupReconciler{
-			Client:          mgr.GetClient(),
-			SystemNamespace: currentNamespace,
-		}).SetupWithManager(mgr); err != nil {
-			setupLog.Error(err, "unable to create controller", "controller", "ManagementBackup")
-			os.Exit(1)
-		}
+	if err = (&controller.ManagementBackupReconciler{
+		Client:          mgr.GetClient(),
+		SystemNamespace: currentNamespace,
+	}).SetupWithManager(mgr); err != nil {
+		setupLog.Error(err, "unable to create controller", "controller", "ManagementBackup")
+		os.Exit(1)
 	}
 	// +kubebuilder:scaffold:builder
 
@@ -372,9 +368,5 @@ func setupWebhooks(mgr ctrl.Manager, currentNamespace string) error {
 		setupLog.Error(err, "unable to create webhook", "webhook", "Release")
 		return err
 	}
-	if err := (&kcmwebhook.ManagementBackupValidator{}).SetupWebhookWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create webhook", "webhook", "ManagementBackup")
-		return err
-	}
 	return nil
 }

internal/controller/backup/config.go

@@ -15,14 +15,8 @@
 package backup
 
 import (
-	"context"
-	"errors"
-	"fmt"
-
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	kcmv1alpha1 "github.com/K0rdent/kcm/api/v1alpha1"
 )
 
 // Reconciler has logic to create and reconcile [github.com/vmware-tanzu/velero/pkg/apis/velero/v1.Backup] objects.
@@ -41,20 +35,3 @@ func NewReconciler(cl client.Client, scheme *runtime.Scheme, systemNamespace str
 		systemNamespace: systemNamespace,
 	}
 }
-
-// ErrNoManagementExists is a sentinel error indicating no [github.com/K0rdent/kcm/api/v1alpha1.Management] object exists.
-var ErrNoManagementExists = errors.New("no Management object exists")
-
-// GetManagement fetches a [github.com/K0rdent/kcm/api/v1alpha1.Management] object.
-func (r *Reconciler) GetManagement(ctx context.Context) (*kcmv1alpha1.Management, error) {
-	mgmts := new(kcmv1alpha1.ManagementList)
-	if err := r.cl.List(ctx, mgmts, client.Limit(1)); err != nil {
-		return nil, fmt.Errorf("failed to list Management: %w", err)
-	}
-
-	if len(mgmts.Items) == 0 {
-		return nil, ErrNoManagementExists
-	}
-
-	return &mgmts.Items[0], nil
-}

internal/controller/backup/periodic.go

@@ -90,15 +90,15 @@ func (r *Runner) Start(ctx context.Context) error {
 
 	defer close(r.eventC)
 
-	l := ctrl.LoggerFrom(ctx).WithName("mgmtbackup_schedule_runner")
+	l := ctrl.LoggerFrom(ctx).WithName("mgmtbackup_runner")
 	ctx = ctrl.LoggerInto(ctx, l)
 
-	l.Info("Starting schedule runner")
+	l.Info("Starting backups runner")
 
 	wait.Until(func() {
-		if err := r.enqueueScheduledBackups(ctx); err != nil {
+		if err := r.enqueueSchedulesOrIncompleteBackups(ctx); err != nil {
 			if errors.Is(err, errEmptyList) {
-				l.V(1).Info("No management backups with schedule to enqueue")
+				l.V(1).Info("No management backups to enqueue")
 				return
 			}
 
@@ -111,10 +111,11 @@ func (r *Runner) Start(ctx context.Context) error {
 
 var errEmptyList = errors.New("no items available to enqueue")
 
-// enqueueScheduledBackups enqueues the [github.com/K0rdent/kcm/api/v1alpha1.ManagementBackup] objects which are properly annotated.
-func (r *Runner) enqueueScheduledBackups(ctx context.Context) error {
+// enqueueSchedulesOrIncompleteBackups enqueues the [github.com/K0rdent/kcm/api/v1alpha1.ManagementBackup] objects which
+// either are schedules or are not yet completed.
+func (r *Runner) enqueueSchedulesOrIncompleteBackups(ctx context.Context) error {
 	schedules := new(kcmv1alpha1.ManagementBackupList)
-	if err := r.cl.List(ctx, schedules, client.MatchingFields{kcmv1alpha1.ManagementBackupScheduledIndexKey: "true"}); err != nil {
+	if err := r.cl.List(ctx, schedules, client.MatchingFields{kcmv1alpha1.ManagementBackupIndexKey: "true"}); err != nil {
 		return fmt.Errorf("failed to list ManagementBackups in periodic runner: %w", err)
 	}
 
@@ -123,9 +124,6 @@ func (r *Runner) enqueueScheduledBackups(ctx context.Context) error {
 	}
 
 	for _, item := range schedules.Items {
-		if item.Status.Paused { // no sense to enqueue paused schedules
-			continue
-		}
 		r.eventC <- event.GenericEvent{
 			Object: &item,
 		}