From 71bf0a724ea39241cbde48bd6c9cf71385b5bb17 Mon Sep 17 00:00:00 2001 From: salmon111 Date: Thu, 9 Jan 2025 14:51:17 +0900 Subject: [PATCH 1/3] Add OwnerReferences to VolumeSnapshots Signed-off-by: salmon111 --- cmd/main.go | 7 ++-- helm/snapscheduler/templates/deployment.yaml | 3 ++ helm/snapscheduler/values.yaml | 2 ++ .../controller/snapshotschedule_controller.go | 34 +++++++++++++------ internal/controller/snapshotschedule_test.go | 24 +++++++++++-- 5 files changed, 55 insertions(+), 15 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index dbafbe7f..3a42b2b2 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -66,6 +66,7 @@ func main() { var probeAddr string var secureMetrics bool var enableHTTP2 bool + var enableOwnerReferences bool flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, @@ -75,6 +76,7 @@ func main() { "If set the metrics endpoint is served securely") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") + flag.BoolVar(&enableOwnerReferences, "enable-owner-references", false, "Enable owner references for VolumeSnapshots.") opts := zap.Options{ Development: true, TimeEncoder: zapcore.RFC3339NanoTimeEncoder, @@ -127,8 +129,9 @@ func main() { } if err = (&controller.SnapshotScheduleReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + EnableOwnerReferences: enableOwnerReferences, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "SnapshotSchedule") os.Exit(1) diff --git a/helm/snapscheduler/templates/deployment.yaml b/helm/snapscheduler/templates/deployment.yaml index d61d936d..f3b276a6 100644 --- a/helm/snapscheduler/templates/deployment.yaml +++ b/helm/snapscheduler/templates/deployment.yaml @@ -53,6 +53,9 @@ spec: - --health-probe-bind-address=:8081 - --metrics-bind-address=127.0.0.1:8080 - --leader-elect + {{- if .Values.enableOwnerReferences }} + - --enable-owner-references + {{- end }} command: - /manager image: {{ include "snapscheduler.image" . }} diff --git a/helm/snapscheduler/values.yaml b/helm/snapscheduler/values.yaml index 491b9efb..ef93f9b2 100644 --- a/helm/snapscheduler/values.yaml +++ b/helm/snapscheduler/values.yaml @@ -12,6 +12,8 @@ imagePullSecrets: [] nameOverride: "" fullnameOverride: "" +enableOwnerReferences: false + rbacProxy: image: repository: quay.io/brancz/kube-rbac-proxy diff --git a/internal/controller/snapshotschedule_controller.go b/internal/controller/snapshotschedule_controller.go index 8a00dd3b..8dbf33c9 100644 --- a/internal/controller/snapshotschedule_controller.go +++ b/internal/controller/snapshotschedule_controller.go @@ -55,7 +55,8 @@ const ( // SnapshotScheduleReconciler reconciles a SnapshotSchedule object type SnapshotScheduleReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + EnableOwnerReferences bool } //nolint:lll @@ -82,7 +83,7 @@ func (r *SnapshotScheduleReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, err } - result, err := doReconcile(ctx, instance, reqLogger, r.Client) + result, err := doReconcile(ctx, instance, reqLogger, r.Client, r.EnableOwnerReferences) // Update result in CR if err != nil { @@ -117,7 +118,7 @@ func (r *SnapshotScheduleReconciler) SetupWithManager(mgr ctrl.Manager) error { } func doReconcile(ctx context.Context, schedule *snapschedulerv1.SnapshotSchedule, - logger logr.Logger, c client.Client) (ctrl.Result, error) { + logger logr.Logger, c client.Client, enableOwnerReferences bool) (ctrl.Result, error) { // If necessary, initialize time of next snap based on schedule if schedule.Status.NextSnapshotTime.IsZero() { // Update nextSnapshot time based on current time and cronspec @@ -135,7 +136,7 @@ func doReconcile(ctx context.Context, schedule *snapschedulerv1.SnapshotSchedule // modifying .status will immediately cause an addl reconcile pass // (which will cover the rest of this reconcile function). We also don't // want to update nextSnapshot until this round is done. - return handleSnapshotting(ctx, schedule, logger, c) + return handleSnapshotting(ctx, schedule, logger, c, enableOwnerReferences) } // We always update nextSnapshot in case the schedule changed @@ -164,7 +165,7 @@ func doReconcile(ctx context.Context, schedule *snapschedulerv1.SnapshotSchedule } func handleSnapshotting(ctx context.Context, schedule *snapschedulerv1.SnapshotSchedule, - logger logr.Logger, c client.Client) (ctrl.Result, error) { + logger logr.Logger, c client.Client, enableOwnerReferences bool) (ctrl.Result, error) { pvcList, err := listPVCsMatchingSelector(ctx, logger, c, schedule.Namespace, &schedule.Spec.ClaimSelector) if err != nil { logger.Error(err, "unable to get matching PVCs") @@ -187,7 +188,7 @@ func handleSnapshotting(ctx context.Context, schedule *snapschedulerv1.SnapshotS labels = schedule.Spec.SnapshotTemplate.Labels snapshotClassName = schedule.Spec.SnapshotTemplate.SnapshotClassName } - snap := newSnapForClaim(snapName, pvc, schedule.Name, snapTime, labels, snapshotClassName) + snap := newSnapForClaim(snapName, pvc, schedule, snapTime, labels, snapshotClassName, enableOwnerReferences) if snap != nil { logger.Info("creating a snapshot", "PVC", pvc.Name, "Snapshot", snapName) if err = c.Create(ctx, snap); err != nil { @@ -287,8 +288,8 @@ func getNextSnapTime(cronspec string, when time.Time) (time.Time, error) { } func newSnapForClaim(snapName string, pvc corev1.PersistentVolumeClaim, - scheduleName string, scheduleTime time.Time, - labels map[string]string, snapClass *string) *snapv1.VolumeSnapshot { + schedule *snapschedulerv1.SnapshotSchedule, scheduleTime time.Time, + labels map[string]string, snapClass *string, enableOwnerReferences bool) *snapv1.VolumeSnapshot { numLabels := 2 if labels != nil { numLabels += len(labels) @@ -297,9 +298,9 @@ func newSnapForClaim(snapName string, pvc corev1.PersistentVolumeClaim, for k, v := range labels { snapLabels[k] = v } - snapLabels[ScheduleKey] = scheduleName + snapLabels[ScheduleKey] = schedule.Name snapLabels[WhenKey] = scheduleTime.Format(timeYYYYMMDDHHMMSS) - return &snapv1.VolumeSnapshot{ + snapshot := &snapv1.VolumeSnapshot{ ObjectMeta: metav1.ObjectMeta{ Name: snapName, Namespace: pvc.Namespace, @@ -312,4 +313,17 @@ func newSnapForClaim(snapName string, pvc corev1.PersistentVolumeClaim, VolumeSnapshotClassName: snapClass, }, } + + if enableOwnerReferences { + snapshot.OwnerReferences = []metav1.OwnerReference{ + { + APIVersion: schedule.APIVersion, + Kind: schedule.Kind, + Name: schedule.Name, + UID: schedule.UID, + }, + } + } + + return snapshot } diff --git a/internal/controller/snapshotschedule_test.go b/internal/controller/snapshotschedule_test.go index 02daab10..cc0ba2d7 100644 --- a/internal/controller/snapshotschedule_test.go +++ b/internal/controller/snapshotschedule_test.go @@ -63,39 +63,57 @@ var _ = Describe("newSnapForClaim", func() { Namespace: "mynamespace", }, } + schedule := snapschedulerv1.SnapshotSchedule{} + schedule.Name = "mysched" + schedule.UID = "bb327b3e-2e87-4e0a-9b42-12d61c08bf97" + schedule.Kind = "SnapshotSchedule" + schedule.APIVersion = "snapscheduler.backube.io/v1" snapname := "mysnap" snapClass := "snapclass" scheduleName := "mysched" schedTime, _ := time.Parse(timeFormat, "2010-07-23T01:02:00Z") - snap := newSnapForClaim(snapname, pvc, scheduleName, schedTime, nil, &snapClass) + snap := newSnapForClaim(snapname, pvc, &schedule, schedTime, nil, &snapClass, false) Expect(snap.Name).To(Equal(snapname)) Expect(snap.Namespace).To(Equal(pvc.Namespace)) Expect(*snap.Spec.Source.PersistentVolumeClaimName).To(Equal(pvc.Name)) Expect(snap.Spec.VolumeSnapshotClassName).NotTo(BeNil()) Expect(*snap.Spec.VolumeSnapshotClassName).To(Equal(snapClass)) + Expect(snap.ObjectMeta.OwnerReferences).To(BeEmpty()) Expect(snap.Labels).To(HaveKeyWithValue(ScheduleKey, scheduleName)) }) - It("allows providing addl labels", func() { + It("allows providing addl labels and checks enableOwnerReferences", func() { pvc := corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: "mypvc", Namespace: "mynamespace", }, } + schedule := snapschedulerv1.SnapshotSchedule{} + schedule.Name = "mysched" + schedule.UID = "bb327b3e-2e87-4e0a-9b42-12d61c08bf97" + schedule.Kind = "SnapshotSchedule" + schedule.APIVersion = "snapscheduler.backube.io/v1" snapname := "mysnap" scheduleName := "mysched" schedTime, _ := time.Parse(timeFormat, "2010-07-23T01:02:00Z") labels := make(map[string]string, 2) labels["one"] = "two" labels["three"] = "four" - snap := newSnapForClaim(snapname, pvc, scheduleName, schedTime, labels, nil) + snap := newSnapForClaim(snapname, pvc, &schedule, schedTime, labels, nil, true) // Some tests depend on knowing the internals :( Expect(snap.Spec.VolumeSnapshotClassName).To(BeNil()) Expect(snap.Labels).NotTo(BeNil()) Expect(snap.Labels).To(HaveKeyWithValue(ScheduleKey, scheduleName)) Expect(snap.Labels).To(HaveKeyWithValue(WhenKey, schedTime.Format(timeYYYYMMDDHHMMSS))) Expect(snap.Labels).To(HaveKeyWithValue("three", "four")) + // Test for ownerReferences + Expect(snap.OwnerReferences).To(HaveLen(1)) + ownerRef := snap.OwnerReferences[0] + Expect(ownerRef.APIVersion).To(Equal(schedule.APIVersion)) + Expect(ownerRef.Kind).To(Equal(schedule.Kind)) + Expect(ownerRef.Name).To(Equal(schedule.Name)) + Expect(ownerRef.UID).To(Equal(schedule.UID)) Expect(len(snap.Labels)).To(Equal(4)) }) }) From 3513f2e685a466d12e042056a963ef0e1f7c1a15 Mon Sep 17 00:00:00 2001 From: salmon111 Date: Thu, 9 Jan 2025 15:10:16 +0900 Subject: [PATCH 2/3] Add enableOwnerReferences flag Signed-off-by: salmon111 --- helm/snapscheduler/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/helm/snapscheduler/README.md b/helm/snapscheduler/README.md index e98439b3..fea5ff42 100644 --- a/helm/snapscheduler/README.md +++ b/helm/snapscheduler/README.md @@ -156,6 +156,8 @@ case, the defaults, shown below, should be sufficient. - `manageCRDs`: `true` - Whether the chart should automatically install, upgrade, or remove the SnapshotSchedule CRD +- `enableOwnerReferences`: `false` + - If set to `true`, owner references will be added to the VolumeSnapshot objects created by the operator. - `rbacProxy.image.repository`: `quay.io/brancz/kube-rbac-proxy` - Specifies the container image used for the RBAC proxy - `rbacProxy.image.tag`: (see values file for default tag) From 4dab3c3d9bfb915bdceb48dcd59d878843d5f019 Mon Sep 17 00:00:00 2001 From: salmon111 Date: Tue, 28 Jan 2025 10:57:51 +0900 Subject: [PATCH 3/3] docs: Fix line breaks in README Signed-off-by: salmon111 --- helm/snapscheduler/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helm/snapscheduler/README.md b/helm/snapscheduler/README.md index fea5ff42..2f498df5 100644 --- a/helm/snapscheduler/README.md +++ b/helm/snapscheduler/README.md @@ -157,7 +157,8 @@ case, the defaults, shown below, should be sufficient. - Whether the chart should automatically install, upgrade, or remove the SnapshotSchedule CRD - `enableOwnerReferences`: `false` - - If set to `true`, owner references will be added to the VolumeSnapshot objects created by the operator. + - If set to `true`, owner references will be added to the VolumeSnapshot + objects created by the operator. - `rbacProxy.image.repository`: `quay.io/brancz/kube-rbac-proxy` - Specifies the container image used for the RBAC proxy - `rbacProxy.image.tag`: (see values file for default tag)