Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
fix(deps): update go dependencies
fix(deps): update container base images
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
k8s.io/api v0.37.0 adds new fields to the embedded corev1 types, which controller-gen renders into the Server CRD schema: - user / defaultUser on projected, secret and configMap volume items (alpha, gated behind AtomicWriteVolumeUserFields) - bindMountOptions on VolumeMount - reworded descriptions for mountPath and dataSource Regenerated with 'make manifests' (controller-gen v0.21.0, pinned via the go.mod tool directive) to fix the manifests drift check.
fix(deps): update go dependencies to v0.37.0
A bool field with omitempty serialises false as absent, so the API server re-applies the CRD default of true. Seven fields could therefore never be set to false. The most visible effect was on ServerSpec.Server: the CA-only role (ca: true, server: false) was unreachable, because the server flag always resolved to true and role never became RoleCA. Also affected: ReadOnlyRootFilesystem, Storeconfigs, AllowSubjectAltNames, AllowAuthorizationExtensions, EnableInfraCRL and AllowAutoRenewal. The CRD defaults are unchanged, so existing manifests keep working. BREAKING CHANGE: the Go types of the listed fields change from bool to *bool. Consumers of the Go API must use the BoolValue helper. Custom resources are not affected.
…hem as NotFound findCertificateAuthority and findSigningPolicies returned nil on any error, not just on NotFound. A timeout or a 5xx therefore caused the Config ConfigMap to be rendered without CA settings, and the autosign policy secret to be overwritten with an empty (deny-all) policy -- followed by Phase=Running as if everything had succeeded. Both helpers now return an error, which ends the reconcile and lets controller-runtime back off, leaving the previous valid state in place. Closes #509
Without observedGeneration it is not decidable whether a status refers to the current spec. Controllers had no way to detect that a resource had been changed since it was last reconciled. Each status type now carries observedGeneration, set only on a successful reconcile, and every condition records the generation it was derived from. Also drops the explicit LastTransitionTime: metav1.Now() from all condition literals. meta.SetStatusCondition manages that field and only updates it on an actual status transition; setting it manually produced a fresh timestamp on every reconcile.
…changes A Certificate in phase Signed went straight to the renewal check, so changes to dnsAltNames or csrExtensions only took effect at the next renewal. The controller now records a hash of the signing-relevant fields in status.signedSpecHash and re-signs when it no longer matches the spec. renewBefore is excluded from the hash: it moves the renewal point but does not change the certificate. Certificates issued before the field existed carry an empty hash and are adopted rather than re-signed, so an operator upgrade does not re-issue every certificate in the cluster. Renewal is no longer driven by status.phase either. Whether a certificate has entered its renewal window is recomputed from status.notAfter, spec.renewBefore and the cooldown annotation, so a lost or hand-edited status no longer changes behaviour. Phase remains as a display value.
Watch map functions listed every resource in the namespace to find matching references. Indexing spec.configRef, spec.certificateRef and spec.authorityRef lets them query directly and keeps the added watches cheap. The index set is defined once and registered both on the manager and on the test client, so map functions behave in tests the way they do in a cluster.
The Server controller derived image, resources, code and the config hash from Config, Certificate and CertificateAuthority without watching any of them, so changing Config.spec.image.tag produced no rollout until an unrelated event happened to trigger a reconcile. The Config controller had the same gap for CertificateAuthority, which meant a CA created after its Config never caused a re-render. Database never reconciled on certificate rotation, leaving its ssl-secret-hash stale. The secret watchers and the new map functions are covered by tests; none of them had any before. Closes #508
Deleting a CertificateAuthority garbage-collected the CA data PVC and the CA key secret through their owner references, destroying the private key while servers and agents were still using it. There was no confirmation and no warning. The controller now holds a finalizer and refuses to release it while any Certificate still references the CA, surfacing the blocking certificates in a DeletionBlocked condition and a warning event. The admission webhook rejects such a delete outright, and warns about the irreversible key loss otherwise. The reference documentation describes the intended deletion order and the manual finalizer removal for the cases where it has to be forced.
…sources Managed child resources are addressed by a name derived from their owner, so a pre-existing resource that happens to share that name was silently taken over and overwritten. assertControlledBy refuses that: the operator does not own the object, and a reconcile error makes the collision visible instead of destroying it.
…pdate The PodDisruptionBudget, HorizontalPodAutoscaler and NetworkPolicy were reconciled with a hand-rolled get-then-create-or-update sequence. That left a window between get and create in which a parallel reconcile could win the create, wrote on every reconcile even when nothing had changed, and emitted an "updated" event each time regardless. controllerutil.CreateOrUpdate handles the already-exists race, skips no-op writes, and reports the actual operation, so events now only fire on real changes. Each mutate function asserts ownership first, and the delete-when- disabled paths do the same before removing anything. The Deployment keeps its explicit conflict retry, which is correct as it is, and gains the same ownership check.
…h CreateOrUpdate Same treatment as the Server child resources: the already-exists race is gone, no-op writes no longer happen, events fire only on real changes, and every mutate function asserts ownership before touching anything. Both Services now write only the fields their owner controls. Replacing the whole spec would have dropped the Kubernetes-assigned clusterIP, and clearing an unset nodePort would hand out a new port and break every client using the old one.
…eateOrUpdate Completes the conversion. The shared createOrUpdateSecret helper, which the Certificate and CertificateAuthority controllers also use, now goes through the same path and asserts ownership. Test fixtures that stand in for previously created child resources gained a controller reference through a new ownedBy helper. Without one they modelled a foreign object, which the reconcilers now correctly refuse to touch.
The Pool controller reset Certificate.status.phase to Pending to force a re-signing after injecting a DNS alt name, racing the Certificate controller for the same subresource. With spec drift detection in place that write is no longer needed: changing the alt names changes the signed spec hash, and the Certificate controller re-signs on its own. ReportProcessor status is no longer written by the Config controller either. The ReportProcessor controller, until now a no-op that only logged, derives its own state from what it can observe -- whether the referenced Config exists and whether its endpoint made it into the rendered webhook Secret -- and watches both. Rendering failures are reported as an event on the Config, which owns that Secret.
…inistic findConfigForCA returned the first match from an unordered list, so with two Configs referencing the same CA the winner could change between reconciles -- and the Config determines the image of the CA setup job. The lookup now sorts by name, propagates list errors instead of treating them as "no Config yet", and reports the ambiguity through a warning event naming all claimants. The Config webhook rejects a second Config referencing an already-referenced CA, which is where the 1:1 relationship actually belongs; the controller-side tie-break remains for deployments that run without webhooks.
Stopping the operator for a single resource previously meant scaling the whole deployment to zero, which affects every resource in the cluster. Setting openvox.voxpupuli.org/paused=true now skips reconciliation for that one object across all seven controllers and reports it through a Paused condition. The status is only written when the condition actually changes, so a paused resource stays quiet. Deletion is handled before the pause check, so a paused resource can still be removed and its finalizers still run. An annotation was chosen over a spec field so the flag stays out of GitOps repositories, where a temporary operational pause does not belong. PoolStatus gained the conditions field it was missing, which is what made the Paused condition observable there as well.
…the default The CEL rule compared storage.size against the literal '1Gi', so a CA that set storage.size to exactly the default value alongside external passed validation, and changing the default would have broken the rule silently. Making storage a pointer gives has(self.storage) real meaning and reduces the rule to the condition it was always meant to express. The default size now resolves in code, matching how environmentPath is handled -- a nested kubebuilder default does not apply when the parent object is omitted. The envtest suite covers the case the old rule let through.
The gauges were never removed, so a client library that keeps every label combination it has seen kept reporting them for the lifetime of the operator process. An alert on openvox_server_replicas_ready == 0 kept firing for a Server that no longer existed, expiry alerts fired for certificates nobody held any more, and a cluster with churn grew its series count without bound. Server gauges are retired on the NotFound path, the only signal available without a finalizer. Certificate and CertificateAuthority retire theirs when their finalizer is released, with the NotFound path as a safety net for objects that were force-deleted. Also adds openvox_crl_last_refresh_timestamp_seconds, which the review found missing: a CRL that is no longer refreshed means revoked agents keep being accepted, and nothing else surfaced that. Documented in the monitoring guide together with an alert rule. Closes #554
The collision check returned an error, so controller-runtime retried it with exponential backoff indefinitely. Nothing about a duplicate hostname improves by waiting -- only a spec change resolves it -- so the retry loop produced only a growing backlog of identical events. The conflict is now reported as Ready=False with reason HostnameConflict, naming the Pool that holds the hostname, plus a single warning event on the transition. The reconcile returns without an error. The check was also racy: two Pools reconciled concurrently could each conclude the hostname was free and both create a TLSRoute for it. The winner is now decided by a property every observer agrees on, the oldest creationTimestamp with the name as tie-breaker, so both converge on the same Pool. A Pool that loses a hostname it previously held releases its TLSRoute, and a terminating Pool no longer keeps a hostname hostage. Since the conflict is no longer polled, Pools competing for the same hostname watch each other, so a Pool picks the hostname up once the holder gives it up or is deleted. Closes #557
reconcileJob deleted and recreated the CA setup Job on every failure with no attempt counter and no terminal state. A Job that cannot succeed -- a bad image reference, a broken PVC, an unschedulable pod -- therefore produced a delete/create cycle roughly every 15 seconds indefinitely, while the CertificateAuthority sat in Initializing with no indication of why. Consecutive failures are now counted in the openvox.voxpupuli.org/ setup-attempts annotation. After five the controller stops recreating, sets CAReady=False with reason SetupFailed carrying the Job's own failure message, moves the phase to Error and emits one warning event. The failed Job is kept, since its logs are the only record of what went wrong. The counter is cleared when the Job succeeds and when the image it runs changes, so correcting spec.image on the Config resumes the setup without a manual step. The job-succeeded-but-secret-missing path draws from the same budget, as it loops just as endlessly. Closes #553
spec.image on the Config reached no Server until #549 removed the nested default from ImageSpec: a nested default is applied whether or not the parent object was specified, so every Server came back from the API server with repository=ghcr.io/slauger/openvox-server-8 and tag=latest, and resolveImage always took the override branch. The chart's servers[] entries do not set an image and the e2e tests configure it on the Config, so neither value reached the Server pods. The suite ran against openvox-server-8:latest rather than the image built for the run, and nothing failed when it did. Assert in single-node and multi-server that the running pods carry IMAGE_REGISTRY/openvox-server-MAJOR:IMAGE_TAG, so a regression here fails the suite instead of passing silently. Closes #550
The polling loop ran thirty iterations of sleep 5 plus the kubectl exec overhead, which exceeds the step's three-minute timeout. chainsaw killed the script before it reached the diagnostic, so a failure produced twenty odd 'waiting for facts' lines and nothing else -- neither the nodes query nor any hint of what went wrong. Budget the loop by wall clock with headroom under a four-minute timeout so the diagnostic always runs, and distinguish the two failure modes: a query API that never answered says nothing about the facts, while one that answered without the node is a real routing failure. curl gets an explicit --max-time so a hung exec cannot eat the budget either.
updateStatusWithRetry re-reads the whole object on every attempt, so a Pool whose spec.route is removed while the reconcile is in flight comes back with a nil Spec.Route. Both the conflict condition and the event that follows it read the hostname from the refreshed object, which panics the controller on that interleaving. Capture the hostname when the conflict is detected and report from that. The regression test drives the interleaving through an interceptor that strips the route from every read after the first; it panics without this change.
The pool-gateway assertion pinned the dnsAltNames the chart produced before #561, which derives the Service name of every Pool a server joins into its certificate. This server has poolRefs [ca, server], so it is reachable through both Services and its certificate now carries both names. That is the point of #561: a server reached through a Pool Service whose name is missing from the certificate fails the TLS handshake, which is what broke database-cnpg with 'Server hostname did not match server certificate'. The expectation was stale, not the behaviour. The failure only surfaced now because e2e-base had been red since 6 August, so the gateway group never ran.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
docs: document the CRD update step for helm upgrade
fix(ca): bound the setup job retry loop and report the failure
test(e2e): assert the image under test, and fix the facts assertion timeout
fix(controller): make status, metrics and hostname conflicts honest
The webhooks group is the only one that installs cert-manager, which serves a Certificate kind of its own. 'kubectl delete certificate' there resolves to cert-manager's resource and silently deletes nothing, so the OpenVox Certificate survived its own cleanup. The following 'kubectl delete certificateauthority' then blocked on the CA finalizer, which correctly refuses to release while a Certificate still references the CA. kubectl delete waits without a timeout by default, and chainsaw's step timeout cannot stop it: the kubectl started through sh -c outlives the step's cancelled context and keeps the pipe open. The job ran for four hours until it was cancelled, with an orphaned kubectl still alive. Use the fully qualified resource name in the two tests that create their own Certificate, and bound every CA and Certificate delete with --timeout=120s so a blocked finalizer fails the step instead of hanging the job. cert-rotation already used the qualified name. This only surfaced now because the webhooks group had never run to completion: every earlier run failed in a group before it.
…guity test(e2e): delete the OpenVox Certificate, not cert-manager's
The Analyze changes with AI step calls the GitHub Models API and falls back to a commit list when the call yields nothing. That fallback was unreachable: the step runs under bash -e, so the failing curl aborted it before the check could run. Since 2 September the PR body has not been updated at all, and #540 still describes three dependency commits while carrying 64. The endpoint had gone away underneath it. models.inference.ai.azure.com is a CNAME to modelmesh-nexus-global-main.trafficmanager.net, which no longer resolves, so curl exits 6 - the exit code the failing job reported. Keep the curl exit code instead of letting it kill the step, and log it together with the response body, so the next outage is diagnosable rather than silent. Point at models.github.ai, which is where the API moved, and namespace the model id as the current API expects. Note that models.github.ai currently answers 410 github_models_retirement_brownout for every path, so the fallback description is what this workflow will produce until a provider decision is made. That is still the commit and file list, which beats a stale body and a red run.
…d API GitHub Models was retired on 2026-07-30, endpoint included. The step called models.inference.ai.azure.com, whose CNAME target no longer resolves, so curl exited 6. The step runs under bash -e, so that aborted it before the fallback could run, and the auto-PR workflow has been failing since. Make the description deterministic. The commits are grouped the way semantic-release categorises them, with the file list in a collapsed block, so the result stands on its own without any external service. Verified against the current develop: 51 commits in, 51 listed. The LLM call is now opt-in. With no LLM_API_KEY secret the step logs a notice and uses the generated summary; with one it posts to LLM_API_URL, any OpenAI-compatible chat/completions endpoint. On failure the exit code, curl's stderr and the start of the response are logged, since the previous warning could not distinguish a vanished endpoint from an empty answer. The description file now holds the body only, so the consumer no longer strips its first line, and the footer says a replaced description is left alone, which is what the is_auto check already did.
fix(ci): generate the PR description instead of depending on a retired API
fix(deps): update docker.io/library/golang docker tag to v1.27.1
The generated schemas are unchanged; only the version annotation each CRD carries moves from v0.21.0 to v0.22.0. Without this the check-manifests job fails, since it regenerates with the version go.mod pins and compares.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
chore(deps): update controller-tools to v0.22.0 and regenerate manifests
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
1 participant
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.
Release develop -> main. 64 commits, 125 files, +7108/-903 since the last merge.
The bulk is a hardening pass over the API and the controllers, plus the e2e suite that had been red since 6 August and is now fully green (19 tests across four groups).
Upgrade notes
Apply the CRDs before upgrading. Helm installs a chart's
crds/directory on first install and never updates it onhelm upgrade. Almost everything in this release depends on the new schema --observedGeneration, all conditions,signedSpecHash, the CEL rules, the list types. Without the CRD update the operator writes status fields the API server strips, and the symptom (a status that never fills in) points away from the cause.Existing Servers stay pinned to their old image.
ImageSpecused to defaultrepositoryandtag, and a nested default is applied even when the parent object is omitted -- so every stored Server carriesghcr.io/slauger/openvox-server-8:latestin its spec, whether or not anyone set it.resolveImageprefers a non-empty Server image, so those Servers keep the baked-in value and still ignorespec.imageon the Config. For chart users ahelm upgradeclears the field and inheritance takes over; hand-managed resources needrepositoryandtagset to""explicitly.Rolling back leaves a finalizer behind. Every CertificateAuthority gains
openvox.voxpupuli.org/ca-protection. An older operator does not know it, so CA deletion would hang indefinitely. To recover:Duplicate list entries become invalid.
dnsAltNames,poolRefs,service.externalIPs, the signing-policy allow lists and the authorization-rule methods are nowx-kubernetes-list-type: set. An existing resource holding a duplicate stays readable but is rejected on the next write. Worth a quick check before upgrading.Features
openvox.voxpupuli.org/pausedannotationcertnameis immutable and CA storage cannot be shrunkFixes
signedSpecHashopenvox_crl_last_refresh_timestamp_secondsfalseis settableStorageSpec.Sizeis typed asresource.Quantityhelm uninstall --waithangRefactors
CreateOrUpdatebehind an ownership guard, replacing hand-rolled Get/Create/UpdateChores
15 dependency updates via Renovate.
Testing
make testand golangci-lint are clean. The full e2e suite is green: 12 base tests, 2 ENC tests, the Gateway API test and the 4 webhook tests with cert-manager.Not covered by e2e: the namespace-scoped mode, and there is no upgrade scenario that installs the previous release and upgrades onto this one.