Fix leader-election RBAC, Go version drift, PVC churn, cron names, apply order
Addresses PR review + acceptance findings: - Add leader-election RBAC: namespaced Role (coordination leases + events) and RoleBinding to the operator ServiceAccount, so controllers actually start under --leader-elect instead of looping on "leases forbidden". Verified in a kind cluster: lease acquired, both controllers start workers. - Align Go versions: bump Dockerfile.operator to golang:1.26-alpine and CI images to golang:1.26 to match go.mod (go 1.26.5); set GOTOOLCHAIN=local so the image build stays hermetic (no toolchain download). - PVC controller: add an annotation predicate so only PVCs carrying (or transitioning off of) backups.unkin.net/schedule enqueue, eliminating reconcile churn from unannotated PVCs while keeping the teardown path. - Cron validator: accept alphabetic month/day-of-week names (MON, JAN, MON-FRI) that k8up and CNPG's robfig/cron accept, still rejecting unknown names; add unit tests for both mapping paths. - Deploy ordering: extract the Namespace into its own manifest and add a config/kustomization.yaml so `kubectl apply -k config` creates the namespace first; document it in the README. - Note the credential-less push precedent (jellyfin-ha) in docker.yaml.
This commit is contained in:
@@ -3,6 +3,8 @@ when:
|
|||||||
ref: refs/tags/v*
|
ref: refs/tags/v*
|
||||||
|
|
||||||
steps:
|
steps:
|
||||||
|
# No push credentials: the in-cluster runner has push access to the artifactapi
|
||||||
|
# docker-internal registry (same credential-less pattern as jellyfin-ha).
|
||||||
- name: docker
|
- name: docker
|
||||||
image: woodpeckerci/plugin-docker-buildx
|
image: woodpeckerci/plugin-docker-buildx
|
||||||
settings:
|
settings:
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ when:
|
|||||||
|
|
||||||
steps:
|
steps:
|
||||||
- name: pre-commit
|
- name: pre-commit
|
||||||
image: golang:1.25
|
image: golang:1.26
|
||||||
commands:
|
commands:
|
||||||
- test -z "$(gofmt -l cmd/ internal/)"
|
- test -z "$(gofmt -l cmd/ internal/)"
|
||||||
- go vet -mod=vendor ./...
|
- go vet -mod=vendor ./...
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ when:
|
|||||||
|
|
||||||
steps:
|
steps:
|
||||||
- name: test
|
- name: test
|
||||||
image: golang:1.25
|
image: golang:1.26
|
||||||
commands:
|
commands:
|
||||||
# Fetch the envtest control-plane binaries; the controller tests skip
|
# Fetch the envtest control-plane binaries; the controller tests skip
|
||||||
# gracefully if this cannot be downloaded.
|
# gracefully if this cannot be downloaded.
|
||||||
|
|||||||
+5
-1
@@ -1,4 +1,8 @@
|
|||||||
FROM golang:1.25-alpine AS builder
|
FROM golang:1.26-alpine AS builder
|
||||||
|
|
||||||
|
# Fail rather than download a toolchain: the base image must already satisfy the
|
||||||
|
# go directive in go.mod so the build stays hermetic (no network at build time).
|
||||||
|
ENV GOTOOLCHAIN=local
|
||||||
|
|
||||||
WORKDIR /build
|
WORKDIR /build
|
||||||
|
|
||||||
|
|||||||
@@ -38,6 +38,19 @@ The operator resolves `destination` names against a ConfigMap in its own
|
|||||||
namespace (`autobackup-operator/autobackup-destinations` by default). See
|
namespace (`autobackup-operator/autobackup-destinations` by default). See
|
||||||
`config/samples/destinations.yaml`.
|
`config/samples/destinations.yaml`.
|
||||||
|
|
||||||
|
## Deploy
|
||||||
|
|
||||||
|
```sh
|
||||||
|
kubectl apply -k config
|
||||||
|
```
|
||||||
|
|
||||||
|
The `config/kustomization.yaml` orders the `Namespace` first so the
|
||||||
|
ServiceAccount, RBAC and Deployment land in an existing namespace. RBAC covers
|
||||||
|
both the controllers' resources (`config/rbac/role.yaml`) and the leader-election
|
||||||
|
lease/events the manager needs when run with `--leader-elect`
|
||||||
|
(`config/rbac/leader_election_role.yaml`). Then create the destinations
|
||||||
|
ConfigMap (see `config/samples/destinations.yaml`).
|
||||||
|
|
||||||
## Schedule mapping
|
## Schedule mapping
|
||||||
|
|
||||||
- **k8up** accepts cron nicknames natively and 5-field cron expressions; the
|
- **k8up** accepts cron nicknames natively and 5-field cron expressions; the
|
||||||
|
|||||||
@@ -0,0 +1,13 @@
|
|||||||
|
apiVersion: kustomize.config.k8s.io/v1beta1
|
||||||
|
kind: Kustomization
|
||||||
|
|
||||||
|
# Namespace is listed first and kustomize's kind ordering applies it ahead of the
|
||||||
|
# ServiceAccount/RoleBinding/Deployment, so `kubectl apply -k config` succeeds on
|
||||||
|
# a clean cluster without a separate namespace-creation step.
|
||||||
|
resources:
|
||||||
|
- namespace.yaml
|
||||||
|
- rbac/service_account.yaml
|
||||||
|
- rbac/role.yaml
|
||||||
|
- rbac/leader_election_role.yaml
|
||||||
|
- rbac/leader_election_role_binding.yaml
|
||||||
|
- manager/manager.yaml
|
||||||
@@ -1,8 +1,3 @@
|
|||||||
apiVersion: v1
|
|
||||||
kind: Namespace
|
|
||||||
metadata:
|
|
||||||
name: autobackup-operator
|
|
||||||
---
|
|
||||||
apiVersion: apps/v1
|
apiVersion: apps/v1
|
||||||
kind: Deployment
|
kind: Deployment
|
||||||
metadata:
|
metadata:
|
||||||
|
|||||||
@@ -0,0 +1,4 @@
|
|||||||
|
apiVersion: v1
|
||||||
|
kind: Namespace
|
||||||
|
metadata:
|
||||||
|
name: autobackup-operator
|
||||||
@@ -0,0 +1,40 @@
|
|||||||
|
---
|
||||||
|
# Namespaced permissions the controller-manager needs for leader election
|
||||||
|
# (--leader-elect). The lease lock is created in the operator's own namespace.
|
||||||
|
apiVersion: rbac.authorization.k8s.io/v1
|
||||||
|
kind: Role
|
||||||
|
metadata:
|
||||||
|
name: autobackup-operator-leader-election
|
||||||
|
namespace: autobackup-operator
|
||||||
|
rules:
|
||||||
|
- apiGroups:
|
||||||
|
- ""
|
||||||
|
resources:
|
||||||
|
- configmaps
|
||||||
|
verbs:
|
||||||
|
- get
|
||||||
|
- list
|
||||||
|
- watch
|
||||||
|
- create
|
||||||
|
- update
|
||||||
|
- patch
|
||||||
|
- delete
|
||||||
|
- apiGroups:
|
||||||
|
- coordination.k8s.io
|
||||||
|
resources:
|
||||||
|
- leases
|
||||||
|
verbs:
|
||||||
|
- get
|
||||||
|
- list
|
||||||
|
- watch
|
||||||
|
- create
|
||||||
|
- update
|
||||||
|
- patch
|
||||||
|
- delete
|
||||||
|
- apiGroups:
|
||||||
|
- ""
|
||||||
|
resources:
|
||||||
|
- events
|
||||||
|
verbs:
|
||||||
|
- create
|
||||||
|
- patch
|
||||||
@@ -0,0 +1,14 @@
|
|||||||
|
---
|
||||||
|
apiVersion: rbac.authorization.k8s.io/v1
|
||||||
|
kind: RoleBinding
|
||||||
|
metadata:
|
||||||
|
name: autobackup-operator-leader-election
|
||||||
|
namespace: autobackup-operator
|
||||||
|
roleRef:
|
||||||
|
apiGroup: rbac.authorization.k8s.io
|
||||||
|
kind: Role
|
||||||
|
name: autobackup-operator-leader-election
|
||||||
|
subjects:
|
||||||
|
- kind: ServiceAccount
|
||||||
|
name: autobackup-operator
|
||||||
|
namespace: autobackup-operator
|
||||||
@@ -9,9 +9,12 @@ import (
|
|||||||
corev1 "k8s.io/api/core/v1"
|
corev1 "k8s.io/api/core/v1"
|
||||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||||
ctrl "sigs.k8s.io/controller-runtime"
|
ctrl "sigs.k8s.io/controller-runtime"
|
||||||
|
"sigs.k8s.io/controller-runtime/pkg/builder"
|
||||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||||
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
|
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
|
||||||
|
"sigs.k8s.io/controller-runtime/pkg/event"
|
||||||
"sigs.k8s.io/controller-runtime/pkg/log"
|
"sigs.k8s.io/controller-runtime/pkg/log"
|
||||||
|
"sigs.k8s.io/controller-runtime/pkg/predicate"
|
||||||
)
|
)
|
||||||
|
|
||||||
// PVCReconciler backs up annotated PersistentVolumeClaims by provisioning an S3
|
// PVCReconciler backs up annotated PersistentVolumeClaims by provisioning an S3
|
||||||
@@ -140,9 +143,28 @@ func secretKeyRef(name, key string) *corev1.SecretKeySelector {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// hasScheduleAnnotation reports whether an object carries the schedule
|
||||||
|
// annotation that marks it for management.
|
||||||
|
func hasScheduleAnnotation(o client.Object) bool {
|
||||||
|
_, ok := o.GetAnnotations()[annSchedule]
|
||||||
|
return ok
|
||||||
|
}
|
||||||
|
|
||||||
|
// schedulePredicate limits reconciles to PVCs that carry (or, on update, used to
|
||||||
|
// carry) the schedule annotation, so unannotated PVCs no longer churn the queue
|
||||||
|
// on every resync while the annotation-removed teardown path still fires.
|
||||||
|
var schedulePredicate = predicate.Funcs{
|
||||||
|
CreateFunc: func(e event.CreateEvent) bool { return hasScheduleAnnotation(e.Object) },
|
||||||
|
DeleteFunc: func(e event.DeleteEvent) bool { return hasScheduleAnnotation(e.Object) },
|
||||||
|
GenericFunc: func(e event.GenericEvent) bool { return hasScheduleAnnotation(e.Object) },
|
||||||
|
UpdateFunc: func(e event.UpdateEvent) bool {
|
||||||
|
return hasScheduleAnnotation(e.ObjectOld) || hasScheduleAnnotation(e.ObjectNew)
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
func (r *PVCReconciler) SetupWithManager(mgr ctrl.Manager) error {
|
func (r *PVCReconciler) SetupWithManager(mgr ctrl.Manager) error {
|
||||||
return ctrl.NewControllerManagedBy(mgr).
|
return ctrl.NewControllerManagedBy(mgr).
|
||||||
For(&corev1.PersistentVolumeClaim{}).
|
For(&corev1.PersistentVolumeClaim{}, builder.WithPredicates(schedulePredicate)).
|
||||||
Owns(&k8upv1.Schedule{}).
|
Owns(&k8upv1.Schedule{}).
|
||||||
Named("pvc-autobackup").
|
Named("pvc-autobackup").
|
||||||
Complete(r)
|
Complete(r)
|
||||||
|
|||||||
@@ -13,9 +13,42 @@ import (
|
|||||||
"k8s.io/apimachinery/pkg/types"
|
"k8s.io/apimachinery/pkg/types"
|
||||||
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
|
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
|
||||||
"k8s.io/client-go/tools/record"
|
"k8s.io/client-go/tools/record"
|
||||||
|
"sigs.k8s.io/controller-runtime/pkg/event"
|
||||||
"sigs.k8s.io/controller-runtime/pkg/reconcile"
|
"sigs.k8s.io/controller-runtime/pkg/reconcile"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
func pvcWith(ann map[string]string) *corev1.PersistentVolumeClaim {
|
||||||
|
return &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "data", Namespace: "ns", Annotations: ann}}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSchedulePredicate(t *testing.T) {
|
||||||
|
annotated := pvcWith(map[string]string{annSchedule: "@daily"})
|
||||||
|
plain := pvcWith(nil)
|
||||||
|
|
||||||
|
if schedulePredicate.Create(event.CreateEvent{Object: plain}) {
|
||||||
|
t.Error("create of unannotated PVC should be filtered out")
|
||||||
|
}
|
||||||
|
if !schedulePredicate.Create(event.CreateEvent{Object: annotated}) {
|
||||||
|
t.Error("create of annotated PVC should enqueue")
|
||||||
|
}
|
||||||
|
if schedulePredicate.Generic(event.GenericEvent{Object: plain}) {
|
||||||
|
t.Error("generic event for unannotated PVC should be filtered out")
|
||||||
|
}
|
||||||
|
// Annotation removed: old had it, new does not -> must still enqueue so
|
||||||
|
// teardown runs.
|
||||||
|
if !schedulePredicate.Update(event.UpdateEvent{ObjectOld: annotated, ObjectNew: plain}) {
|
||||||
|
t.Error("annotation removal should enqueue for teardown")
|
||||||
|
}
|
||||||
|
// Annotation added.
|
||||||
|
if !schedulePredicate.Update(event.UpdateEvent{ObjectOld: plain, ObjectNew: annotated}) {
|
||||||
|
t.Error("annotation addition should enqueue")
|
||||||
|
}
|
||||||
|
// Neither old nor new annotated -> no enqueue.
|
||||||
|
if schedulePredicate.Update(event.UpdateEvent{ObjectOld: plain, ObjectNew: plain}) {
|
||||||
|
t.Error("update of unannotated PVC should be filtered out")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestPVCReconcile(t *testing.T) {
|
func TestPVCReconcile(t *testing.T) {
|
||||||
requireEnvtest(t)
|
requireEnvtest(t)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|||||||
@@ -76,6 +76,14 @@ func ScheduleForCNPG(in string) (string, error) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// cronNames are the alphabetic month and day-of-week names robfig/cron (used by
|
||||||
|
// k8up and CloudNativePG) accepts, case-insensitively, e.g. "MON" or "JAN".
|
||||||
|
var cronNames = map[string]bool{
|
||||||
|
"JAN": true, "FEB": true, "MAR": true, "APR": true, "MAY": true, "JUN": true,
|
||||||
|
"JUL": true, "AUG": true, "SEP": true, "OCT": true, "NOV": true, "DEC": true,
|
||||||
|
"SUN": true, "MON": true, "TUE": true, "WED": true, "THU": true, "FRI": true, "SAT": true,
|
||||||
|
}
|
||||||
|
|
||||||
func validateCronFields(s string, want int) error {
|
func validateCronFields(s string, want int) error {
|
||||||
fields := strings.Fields(s)
|
fields := strings.Fields(s)
|
||||||
if len(fields) != want {
|
if len(fields) != want {
|
||||||
@@ -85,11 +93,34 @@ func validateCronFields(s string, want int) error {
|
|||||||
if f == "" {
|
if f == "" {
|
||||||
return fmt.Errorf("cron %q has an empty field", s)
|
return fmt.Errorf("cron %q has an empty field", s)
|
||||||
}
|
}
|
||||||
for _, r := range f {
|
if err := validateCronField(f); err != nil {
|
||||||
if !isCronRune(r) {
|
return err
|
||||||
return fmt.Errorf("cron field %q contains invalid character %q", f, string(r))
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// validateCronField accepts cron symbols plus known alphabetic month/day names
|
||||||
|
// (e.g. MON-FRI, JAN); an unknown alphabetic run like "foo" is rejected.
|
||||||
|
func validateCronField(f string) error {
|
||||||
|
runes := []rune(f)
|
||||||
|
for i := 0; i < len(runes); {
|
||||||
|
if isLetter(runes[i]) {
|
||||||
|
j := i
|
||||||
|
for j < len(runes) && isLetter(runes[j]) {
|
||||||
|
j++
|
||||||
|
}
|
||||||
|
name := strings.ToUpper(string(runes[i:j]))
|
||||||
|
if !cronNames[name] {
|
||||||
|
return fmt.Errorf("cron field %q contains unknown name %q", f, string(runes[i:j]))
|
||||||
|
}
|
||||||
|
i = j
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if !isCronRune(runes[i]) {
|
||||||
|
return fmt.Errorf("cron field %q contains invalid character %q", f, string(runes[i]))
|
||||||
|
}
|
||||||
|
i++
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -97,3 +128,7 @@ func validateCronFields(s string, want int) error {
|
|||||||
func isCronRune(r rune) bool {
|
func isCronRune(r rune) bool {
|
||||||
return (r >= '0' && r <= '9') || r == '*' || r == '/' || r == ',' || r == '-' || r == '?'
|
return (r >= '0' && r <= '9') || r == '*' || r == '/' || r == ',' || r == '-' || r == '?'
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func isLetter(r rune) bool {
|
||||||
|
return (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z')
|
||||||
|
}
|
||||||
|
|||||||
@@ -16,10 +16,13 @@ func TestScheduleForK8up(t *testing.T) {
|
|||||||
{"0 2 * * *", "0 2 * * *", false},
|
{"0 2 * * *", "0 2 * * *", false},
|
||||||
{"*/15 * * * *", "*/15 * * * *", false},
|
{"*/15 * * * *", "*/15 * * * *", false},
|
||||||
{"0 0 * * 1-5", "0 0 * * 1-5", false},
|
{"0 0 * * 1-5", "0 0 * * 1-5", false},
|
||||||
|
{"0 0 1 * MON", "0 0 1 * MON", false}, // alphabetic day-of-week
|
||||||
|
{"0 0 1 JAN *", "0 0 1 JAN *", false}, // alphabetic month
|
||||||
|
{"0 0 * * mon-fri", "0 0 * * mon-fri", false}, // lowercase range of names
|
||||||
{"@bogus", "", true},
|
{"@bogus", "", true},
|
||||||
{"0 2 * *", "", true}, // 4 fields
|
{"0 2 * *", "", true}, // 4 fields
|
||||||
{"0 0 0 * * *", "", true}, // 6 fields not valid for k8up
|
{"0 0 0 * * *", "", true}, // 6 fields not valid for k8up
|
||||||
{"0 2 * * foo", "", true}, // invalid chars
|
{"0 2 * * foo", "", true}, // unknown alphabetic name
|
||||||
{"", "", true},
|
{"", "", true},
|
||||||
}
|
}
|
||||||
for _, c := range cases {
|
for _, c := range cases {
|
||||||
@@ -57,10 +60,12 @@ func TestScheduleForCNPG(t *testing.T) {
|
|||||||
{"*/15 * * * *", "0 */15 * * * *", false},
|
{"*/15 * * * *", "0 */15 * * * *", false},
|
||||||
{"30 0 3 * * *", "30 0 3 * * *", false}, // already 6-field
|
{"30 0 3 * * *", "30 0 3 * * *", false}, // already 6-field
|
||||||
{" @daily ", "0 0 0 * * *", false},
|
{" @daily ", "0 0 0 * * *", false},
|
||||||
|
{"0 0 1 * MON", "0 0 0 1 * MON", false}, // 5-field w/ alphabetic dow gains seconds
|
||||||
|
{"0 0 0 1 JAN *", "0 0 0 1 JAN *", false}, // 6-field w/ alphabetic month
|
||||||
{"@bogus", "", true},
|
{"@bogus", "", true},
|
||||||
{"0 2 * *", "", true}, // 4 fields
|
{"0 2 * *", "", true}, // 4 fields
|
||||||
{"1 2 3 4 5 6 7", "", true}, // 7 fields
|
{"1 2 3 4 5 6 7", "", true}, // 7 fields
|
||||||
{"0 2 * * bad", "", true}, // invalid chars
|
{"0 2 * * bad", "", true}, // unknown alphabetic name
|
||||||
{"", "", true},
|
{"", "", true},
|
||||||
}
|
}
|
||||||
for _, c := range cases {
|
for _, c := range cases {
|
||||||
|
|||||||
Reference in New Issue
Block a user