feat: rewrite operator in Go (Kubebuilder v4)#298
Merged
Merged
Conversation
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- Tests for US1.1 (v3applicationcredential), US1.2 (interface/region), US1.3 (revoked credential / catalog 404), US1.4 (custom CA) - Bug : s.userID not assigned in getToken - Remplacing custom errors_as by errors.As stdlib - Adding Session.HasEndpoint() for test introspection - 24 tests, covering cloud.go 85–100% Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- resources_test.go (new): 13 tests covering US2.1 (filtering by cluster, description, multi-FIP) and US2.2 (successful deletion, transient 400/409 errors absorbed with verification, 500 error propagated, persistent FIPs → error, empty list → no verification, missing network endpoint → CatalogError) - resources.go — DeleteFloatingIPs: moved `deleted = true` before the DELETE call so verification always triggers on transient errors; added explicit log for those errors Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- resources_test.go: 9 tests covering US3.1 (kube_service_<cluster>_ prefix filtering), US3.2 (HTTP error on list → log + return nil, no exception propagated), and US3.3 (cascade=true, successful deletion, transient 400/409 absorbed with verification, 500 propagated, persistent LBs → error, empty list → no verification, missing load-balancer endpoint → skip) - resources.go — DeleteLoadBalancers: moved deleted = true before DELETE call and added explicit warning log for transient errors (consistent with FIP cleanup pattern) Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- resources_test.go: 6 tests covering US4.1 (description prefix/suffix filtering, different cluster, wrong prefix, unrelated description), US4.2 (successful deletion, transient HTTP 409 absorbed with verification, 500 propagated, persistent SG → error, nothing to delete → no verification) - resources.go — DeleteSecurityGroups: moved deleted = true before DELETE call; added explicit warning log for transient errors (consistent with FIP/LB cleanup pattern) Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- resources_test.go: 6 tests — description filter (prefix "Security Group for" + suffix "Service LoadBalancer in cluster <name>"), transient HTTP 409 absorbed with verification triggered, HTTP 500 propagated, still-present → error, nothing-to-delete → no verification - resources.go — DeleteSecurityGroups: moved deleted=true before DELETE call; added explicit warning log for transient errors Epic 5 — Cinder Volumes (US5.1): - resources_test.go: 6 tests — metadata filter (cinder.csi.openstack.org/cluster), keep annotation (janitor.capi.azimuth-cloud.com/keep=true) skips deletion, transient HTTP 409 absorbed with verification triggered, HTTP 500 propagated, still-present → error, nothing-to-delete → no verification - resources.go — DeleteVolumes: aligned on FIP/LB/SG pattern (deleted=true before DELETE, explicit warning log for transient errors) Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- resources_test.go: 6 tests covering US6.1 — CSI metadata filter (cinder.csi.openstack.org/cluster), transient HTTP 409 absorbed with verification triggered, HTTP 500 propagated, still-present → error, nothing-to-delete → no verification - resources.go — DeleteSnapshots: aligned on FIP/LB/SG/Volume pattern (deleted=true before DELETE, explicit warning log for transient errors) Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- resources_test.go: 5 tests covering US7.1 — successful deletion (204), already-deleted (404 → nil), forbidden (403 → graceful skip), HTTP 500 propagated, no identity endpoint → CatalogError - resources.go: no changes needed (implementation already correct) Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- controller.go: add PurgeFunc/SleepFunc injectable fields; replace openstack.PurgeResources and time.Sleep calls with r.purge/r.sleep helpers (defaults preserved, no behavior change in production) - openstackcluster_controller_test.go: 8 tests covering US8.1 (finalizer add when absent, idempotent when present), US8.2 (cluster name from label vs metadata.name), US8.3 (finalizer removed after successful purge, cleanup skipped when finalizer absent), US8.4 (retry annotation set on purge error, NotFound ignored when cluster deleted during retry) Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- config_test.go: 10 tests covering US9.1 — DefaultVolumesFromEnv (default "delete", reads "keep", reads "delete"), RetryDelayFromEnv (default 60, reads 120, invalid value → fallback 60), reconciler volumes policy (global delete/keep, annotation overrides in both directions), retry delay applied to SleepFunc Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
US10.1 — Secure image - Dockerfile: full rewrite from Python/kopf to Go multi-stage (golang:1.26 builder + gcr.io/distroless/static:nonroot, UID 65532) US10.2 — Helm chart - chart/values.yaml: add retryDefaultDelay (default 60 s) - chart/templates/deployment.yaml: env CAPI_JANITOR_RETRY_DEFAULT_DELAY, livenessProbe (/healthz:8081) and readinessProbe (/readyz:8081) - chart/templates/clusterrole.yaml: add "update" verb on openstackclusters (required by r.Update for finalizer management); remove stale kopf comments - chart/tests/packaging_test.yaml: 11 helm-unittest tests covering US10.1 and US10.2 (security context, injectable env vars, probes, RBAC) US10.3 — Nix-based OCI build (no Flake) + SBOM - nix/nixpkgs.nix: pin nixos-26.05 (Go 1.26 required) - nix/default.nix: four attributes — manager (buildGoModule, CGO_ENABLED=0), amd64 image, arm64 image (cross-compiled via pkgsCross.aarch64-multiplatform), sbom (syft scan of the binary → CycloneDX JSON) - .github/workflows/build-push-artifacts.yaml: replace docker-multiarch-build-push with nix-build + skopeo push + docker manifest for multi-arch support; SBOM generation uploaded as a CI artifact Note: vendorHash = lib.fakeHash — update on first nix-build run. Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
…netes events) (TDD)
US11.1 — Prometheus metrics
- internal/controller/metrics.go: Metrics struct exposing
capi_janitor_cleanups_total{result="success|failure"} CounterVec;
NewMetrics(reg) registers it against any prometheus.Registerer
- OpenStackClusterReconciler.Metrics field (optional, injected in tests);
SetupWithManager auto-registers against ctrlmetrics.Registry when nil
- incMetric("success"|"failure") called after each purge attempt
US11.2 — Kubernetes events
- OpenStackClusterReconciler.Recorder field (record.EventRecorder);
SetupWithManager wires mgr.GetEventRecorderFor("capi-janitor") when nil
- Normal/CleanupSucceeded emitted on successful OpenStack cleanup
- Warning/CleanupFailed emitted on purge error (message carries the error)
Tests
- internal/controller/observability_test.go: 4 TDD tests using
prometheus.NewRegistry() + prometheus/testutil.ToFloat64 for metric
assertions, record.NewFakeRecorder for event assertions
go.mod: prometheus/client_golang promoted from indirect to direct dependency
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
…(TDD) US12.1 — HTTP client timeout - cloud.go: add http.Client.Timeout = 30s as a safety net for callers that do not set a context deadline; context cancellation remains the primary mechanism (all requests already use http.NewRequestWithContext) US12.2 — Cinder legacy alias - resources.go: cinderEndpoint() now tries "volumev3", "block-storage", then "volume" — adds compatibility with OpenStack installations older than Stein that advertise Cinder under the "volume" service type Tests - internal/openstack/robustness_test.go: 3 TDD tests - TestAuthenticate_ReturnsError_WhenContextAlreadyCancelled (US12.1) - TestCinderEndpoint_FallsBackToBlockStorage (regression) - TestCinderEndpoint_FallsBackToVolumeAlias (US12.2) Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- packaging_test.yaml: remove `hasDocuments: count: 4` assertion —
helm-unittest applies it per template file (not globally), so each
file fails because it only renders 1 document. The snapshot test
already validates the full set of rendered resources.
- snapshot_test.yaml.snap: regenerate to match current chart templates:
- ClusterRole (snapshot 1): add "update" verb on openstackclusters
- Deployment (snapshot 3): add CAPI_JANITOR_RETRY_DEFAULT_DELAY env var,
livenessProbe (/healthz:8081) and readinessProbe (/readyz:8081)
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Reorder infrav1 imports per gofmt/goimports convention, set the real vendorHash in nix/default.nix (was fakeHash), and update the README to document `nix-build nix -A tests` and the macOS/Linux binary linkage differences. Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Restore non-fatal handling of LB verification-list errors (Octavia is known to be slower/less reliable, per PR #261), drop the unconditional 5s delay before the first deletion check, and stop the polling loop promptly on context cancellation. Also fill in two test helpers that had lost their IsAuthenticated() assertion, and document the polling mechanism in ROADMAP.md. Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
- If a test file has been modified, it means that the behaviour is different. A integration test is then needed. - If the helm chart has been modified, it means that the installation behaviour might change. We need to try to install the product. Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
…osystems The image has been built via `nix-build nix -A image` since the CI rewrite; remove the now-vestigial Dockerfile/.dockerignore and the Makefile docker-build/docker-push/docker-buildx targets, and update every real reference (README, ROADMAP, AGENTS.md, e2e image build/load) to the Nix path. Also fix dependabot.yml, which tracked GitHub Actions and a leftover Python `pip` ecosystem but nothing for go.mod: swap `pip` for `gomod`. Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
Signed-off-by: Mathieu Grzybek <mathieu@stackhpc.com>
|
hi, @mgrzybek @Banh-Canh this direction is really good to me, could i also add some commit on this PR? |
Contributor
Author
Hi, I think that we should merge this PR as fast as possible and then write new patches. I don’t like long-living PRs. |
Banh-Canh
approved these changes
Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Complete rewrite of the operator from Python/kopf to Go using Kubebuilder v4,
developed fully in TDD (Gherkin specs → implementation).
internal/openstack/) — authentication viav3applicationcredentialwith custom CA support, catalog filtering byinterface/region, purge of floating IPs, load balancers, security groups,
Cinder volumes & snapshots, and application credentials; Cinder endpoint
with
volumev3/block-storage/volumefallback chain; HTTP clienttimeout (30 s) as safety net
internal/controller/) — finalizer lifecycle, cluster nameresolution from label or
metadata.name, configurable volumes policy(
delete/keep) via env var and per-cluster annotation, retry mechanismvia annotation on purge failure, Prometheus counter
capi_janitor_cleanups_total{result="success|failure"}, Kubernetes eventsCleanupSucceeded/CleanupFailed65532); Nix-based reproducible OCI images (amd64 + arm64 cross-compiled)
with CycloneDX SBOM via syft; CI updated to use
nix-build+skopeo+docker manifestretryDefaultDelayvalue, liveness/readiness probes,updated RBAC (
updateverb onopenstackclusters); 11 helm-unittest tests