From dc8f33b0a5ecdb2e2049d0ebde522261ef2dcc7c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:19:27 +0000 Subject: [PATCH 1/5] feat: add GitOps health rules for CoderAIProvider (#321) Change-Id: I453fc37a3fd4f09a28da18f4dbaf8107279e069b Signed-off-by: Thomas Kosiewski --- .../gitops/argocd-health-coderaiprovider.lua | 28 +++++++ .../flux-healthcheck-coderaiprovider.yaml | 9 +++ config/gitops/health_test.go | 81 ++++++++++++++----- docs/how-to/gitops.md | 62 +++++++++++++- 4 files changed, 160 insertions(+), 20 deletions(-) create mode 100644 config/gitops/argocd-health-coderaiprovider.lua create mode 100644 config/gitops/flux-healthcheck-coderaiprovider.yaml diff --git a/config/gitops/argocd-health-coderaiprovider.lua b/config/gitops/argocd-health-coderaiprovider.lua new file mode 100644 index 000000000..128f0c552 --- /dev/null +++ b/config/gitops/argocd-health-coderaiprovider.lua @@ -0,0 +1,28 @@ +-- Argo CD health check for CoderAIProvider (coder.com/v1alpha1). +-- Install it as the argocd-cm key +-- resource.customizations.health.coder.com_CoderAIProvider. +-- docs/how-to/gitops.md copies this file. A test keeps the copy equal. +local hs = { status = "Progressing", message = "Waiting for the controller" } +if obj.metadata.deletionTimestamp ~= nil then + -- Argo CD shows deletionMessage, not message, for a resource being deleted. + hs.deletionMessage = "Deleting the Coder AI provider" + hs.message = hs.deletionMessage + return hs +end +local st = obj.status +if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then + return hs +end +for _, c in ipairs(st.conditions or {}) do + if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then + hs.message = c.reason .. ": " .. c.message + end +end +for _, c in ipairs(st.conditions or {}) do + if c.type == "Ready" and c.status == "True" then + hs.status = "Healthy" + elseif c.type == "Stalled" and c.status == "True" then + hs.status = "Degraded" + end +end +return hs diff --git a/config/gitops/flux-healthcheck-coderaiprovider.yaml b/config/gitops/flux-healthcheck-coderaiprovider.yaml new file mode 100644 index 000000000..2c56f3b89 --- /dev/null +++ b/config/gitops/flux-healthcheck-coderaiprovider.yaml @@ -0,0 +1,9 @@ +# Flux health check expressions for CoderAIProvider (coder.com/v1alpha1). +# Add this entry to spec.healthCheckExprs of a Flux v2.5+ Kustomization +# with spec.wait: true. +# docs/how-to/gitops.md copies this file. A test keeps the copy equal. +- apiVersion: coder.com/v1alpha1 + kind: CoderAIProvider + inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" + failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" + current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" diff --git a/config/gitops/health_test.go b/config/gitops/health_test.go index ca7cc4ba9..fa96a129d 100644 --- a/config/gitops/health_test.go +++ b/config/gitops/health_test.go @@ -1,6 +1,6 @@ -// Package gitops_test checks the GitOps health rules for CoderTemplateTest -// and CoderMCPServer. The rules are data files, so this package has no -// production code. +// Package gitops_test checks the GitOps health rules for CoderTemplateTest, +// CoderMCPServer and CoderAIProvider. The rules are data files, so this +// package has no production code. package gitops_test import ( @@ -29,6 +29,7 @@ type ruleSet struct { var ruleSets = []ruleSet{ {"CoderTemplateTest", "argocd-health-codertemplatetest.lua", "flux-healthcheck-codertemplatetest.yaml", "../../docs/how-to/gitops.md", healthCases}, {"CoderMCPServer", "argocd-health-codermcpserver.lua", "flux-healthcheck-codermcpserver.yaml", "../../docs/how-to/manage-mcp-servers.md", mcpServerCases}, + {"CoderAIProvider", "argocd-health-coderaiprovider.lua", "flux-healthcheck-coderaiprovider.yaml", "../../docs/how-to/gitops.md", aiProviderCases}, } // fluxHealthCheck is one entry of a Flux Kustomization spec.healthCheckExprs. @@ -128,10 +129,10 @@ func mcpServerObject(status map[string]any) map[string]any { return obj } -// mcpServerStatus returns the three conditions that the controller always -// writes together, with one reason and message: ready, reconciling (neither -// ready nor stalled), or stalled. -func mcpServerStatus(state, reason string) map[string]any { +// readyReconcilingStalled returns the three conditions that the CoderMCPServer +// and CoderAIProvider controllers always write together, with one reason and +// message: ready, reconciling (neither ready nor stalled), or stalled. +func readyReconcilingStalled(state, reason string) map[string]any { ready, reconciling, stalled := "False", "True", "False" switch state { case "ready": @@ -140,7 +141,7 @@ func mcpServerStatus(state, reason string) map[string]any { reconciling, stalled = "False", "True" case "reconciling": default: - panic("assertion failed: unknown MCP server fixture state " + state) + panic("assertion failed: unknown condition fixture state " + state) } condition := func(conditionType, status string) any { return map[string]any{"type": conditionType, "status": status, "reason": reason, "message": "details", "observedGeneration": int64(2)} @@ -152,31 +153,73 @@ func mcpServerStatus(state, reason string) map[string]any { } func mcpServerCases() []healthCase { - stale := mcpServerStatus("ready", "Applied") + stale := readyReconcilingStalled("ready", "Applied") stale["observedGeneration"] = int64(1) noConditions := map[string]any{"observedGeneration": int64(2)} - deleting := mcpServerObject(mcpServerStatus("ready", "Applied")) + deleting := mcpServerObject(readyReconcilingStalled("ready", "Applied")) deleting["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-14T10:00:00Z" - deletingWait := mcpServerObject(mcpServerStatus("reconciling", "CoderUnavailable")) + deletingWait := mcpServerObject(readyReconcilingStalled("reconciling", "CoderUnavailable")) deletingWait["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-14T10:00:00Z" return []healthCase{ {"no status", mcpServerObject(nil), "Progressing", "Waiting for the controller", "Unknown", ""}, {"stale observedGeneration", mcpServerObject(stale), "Progressing", "Waiting for the controller", "InProgress", ""}, {"no conditions", mcpServerObject(noConditions), "Progressing", "Waiting for the controller", "InProgress", ""}, - {"applied", mcpServerObject(mcpServerStatus("ready", "Applied")), "Healthy", "Applied: details", "Current", ""}, - {"control plane not ready", mcpServerObject(mcpServerStatus("reconciling", "ControlPlaneNotReady")), "Progressing", "ControlPlaneNotReady: details", "InProgress", ""}, - {"Coder unavailable", mcpServerObject(mcpServerStatus("reconciling", "CoderUnavailable")), "Progressing", "CoderUnavailable: details", "InProgress", ""}, - {"group not found", mcpServerObject(mcpServerStatus("stalled", "GroupNotFound")), "Degraded", "GroupNotFound: details", "Failed", ""}, - {"not allowed", mcpServerObject(mcpServerStatus("stalled", "MCPServersNotAllowed")), "Degraded", "MCPServersNotAllowed: details", "Failed", ""}, - {"disabled externally", mcpServerObject(mcpServerStatus("stalled", "DisabledExternally")), "Degraded", "DisabledExternally: details", "Failed", ""}, - {"credential not opted in", mcpServerObject(mcpServerStatus("stalled", "CredentialNotOptedIn")), "Degraded", "CredentialNotOptedIn: details", "Failed", ""}, - {"slug conflict", mcpServerObject(mcpServerStatus("stalled", "SlugConflict")), "Degraded", "SlugConflict: details", "Failed", ""}, + {"applied", mcpServerObject(readyReconcilingStalled("ready", "Applied")), "Healthy", "Applied: details", "Current", ""}, + {"control plane not ready", mcpServerObject(readyReconcilingStalled("reconciling", "ControlPlaneNotReady")), "Progressing", "ControlPlaneNotReady: details", "InProgress", ""}, + {"Coder unavailable", mcpServerObject(readyReconcilingStalled("reconciling", "CoderUnavailable")), "Progressing", "CoderUnavailable: details", "InProgress", ""}, + {"group not found", mcpServerObject(readyReconcilingStalled("stalled", "GroupNotFound")), "Degraded", "GroupNotFound: details", "Failed", ""}, + {"not allowed", mcpServerObject(readyReconcilingStalled("stalled", "MCPServersNotAllowed")), "Degraded", "MCPServersNotAllowed: details", "Failed", ""}, + {"disabled externally", mcpServerObject(readyReconcilingStalled("stalled", "DisabledExternally")), "Degraded", "DisabledExternally: details", "Failed", ""}, + {"credential not opted in", mcpServerObject(readyReconcilingStalled("stalled", "CredentialNotOptedIn")), "Degraded", "CredentialNotOptedIn: details", "Failed", ""}, + {"slug conflict", mcpServerObject(readyReconcilingStalled("stalled", "SlugConflict")), "Degraded", "SlugConflict: details", "Failed", ""}, // Flux has no deletion rule: it waits for pruned objects separately. {"being deleted", deleting, "Progressing", "Deleting the Coder MCP server config", "Current", "Deleting the Coder MCP server config"}, {"being deleted while Coder is down", deletingWait, "Progressing", "Deleting the Coder MCP server config", "InProgress", "Deleting the Coder MCP server config"}, } } +// aiProviderObject returns a CoderAIProvider at generation 2 with the given +// status. +func aiProviderObject(status map[string]any) map[string]any { + obj := map[string]any{ + "apiVersion": "coder.com/v1alpha1", + "kind": "CoderAIProvider", + "metadata": map[string]any{"name": "openai", "namespace": "coder", "generation": int64(2)}, + "spec": map[string]any{"type": "openai", "baseURL": "https://api.openai.com/v1"}, + } + if status != nil { + obj["status"] = status + } + return obj +} + +func aiProviderCases() []healthCase { + stale := readyReconcilingStalled("ready", "Applied") + stale["observedGeneration"] = int64(1) + noConditions := map[string]any{"observedGeneration": int64(2)} + deleting := aiProviderObject(readyReconcilingStalled("ready", "Applied")) + deleting["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-30T10:00:00Z" + deletingWait := aiProviderObject(readyReconcilingStalled("reconciling", "CoderUnavailable")) + deletingWait["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-30T10:00:00Z" + return []healthCase{ + {"no status", aiProviderObject(nil), "Progressing", "Waiting for the controller", "Unknown", ""}, + {"stale observedGeneration", aiProviderObject(stale), "Progressing", "Waiting for the controller", "InProgress", ""}, + {"no conditions", aiProviderObject(noConditions), "Progressing", "Waiting for the controller", "InProgress", ""}, + {"applied", aiProviderObject(readyReconcilingStalled("ready", "Applied")), "Healthy", "Applied: details", "Current", ""}, + {"control plane not ready", aiProviderObject(readyReconcilingStalled("reconciling", "ControlPlaneNotReady")), "Progressing", "ControlPlaneNotReady: details", "InProgress", ""}, + {"confirming a create", aiProviderObject(readyReconcilingStalled("reconciling", "ConfirmingCreate")), "Progressing", "ConfirmingCreate: details", "InProgress", ""}, + // A disable that failed retries with the credential reason. + {"disable retrying", aiProviderObject(readyReconcilingStalled("reconciling", "CredentialNotOptedIn")), "Progressing", "CredentialNotOptedIn: details", "InProgress", ""}, + {"credential kill switch", aiProviderObject(readyReconcilingStalled("stalled", "CredentialNotOptedIn")), "Degraded", "CredentialNotOptedIn: details", "Failed", ""}, + {"name conflict", aiProviderObject(readyReconcilingStalled("stalled", "NameConflict")), "Degraded", "NameConflict: details", "Failed", ""}, + {"name reserved by env", aiProviderObject(readyReconcilingStalled("stalled", "NameReservedByEnv")), "Degraded", "NameReservedByEnv: details", "Failed", ""}, + {"disabled externally", aiProviderObject(readyReconcilingStalled("stalled", "DisabledExternally")), "Degraded", "DisabledExternally: details", "Failed", ""}, + // Flux has no deletion rule: it waits for pruned objects separately. + {"being deleted", deleting, "Progressing", "Deleting the Coder AI provider", "Current", "Deleting the Coder AI provider"}, + {"being deleted while Coder is down", deletingWait, "Progressing", "Deleting the Coder AI provider", "InProgress", "Deleting the Coder AI provider"}, + } +} + func TestArgoCDHealth(t *testing.T) { t.Parallel() for _, rules := range ruleSets { diff --git a/docs/how-to/gitops.md b/docs/how-to/gitops.md index 1f5c6409e..bef11374e 100644 --- a/docs/how-to/gitops.md +++ b/docs/how-to/gitops.md @@ -1,6 +1,6 @@ # Gate template promotion with GitOps -This guide shows how to make Argo CD or Flux wait for a [`CoderTemplateTest`](test-templates.md) result, and how to promote a template version only after its test passed. The health rules live in `config/gitops/` in this repository. This page copies them, and a unit test keeps the copies equal to the files. +This guide shows how to make Argo CD or Flux wait for a [`CoderTemplateTest`](test-templates.md) result, and how to promote a template version only after its test passed. The health rules live in `config/gitops/` in this repository. This page copies them, and a unit test keeps the copies equal to the files. The page also holds the [health rules for `CoderAIProvider`](#health-rules-for-coderaiprovider). ## Why custom health rules are needed @@ -112,6 +112,66 @@ A `CoderTemplate` update of `spec.files` activates the new version at once, befo If the import takes longer than the 34-second write budget of the aggregated API ([#117](https://github.com/coder/coder-k8s/issues/117)), the update fails and the previous version stays active, so the test resolves the previous version. `status.templateVersionName` shows which version ran. +## Health rules for CoderAIProvider + +Argo CD and Flux also need health rules to wait for a [`CoderAIProvider`](manage-ai-providers.md). Without them, Argo CD has no health check for the kind, and Flux reports a new provider as current before the controller writes its first status. The controller writes the conditions `Ready`, `Reconciling` and `Stalled` together, with one reason and message, and sets `status.observedGeneration` on every status write. Both rules wait until `status.observedGeneration` equals `metadata.generation`. Then they read the conditions: + +| Conditions | Argo CD | Flux | +| --- | --- | --- | +| no status, or a stale `observedGeneration` | `Progressing` | `InProgress` (`Unknown` before the first status) | +| `Reconciling=True` | `Progressing` | `InProgress` | +| `Ready=True` | `Healthy` | `Current` | +| `Stalled=True` | `Degraded` | `Failed` | + +A [kill switch](manage-ai-providers.md#kill-switches) stalls the provider, so it shows `Degraded` or `Failed` until the spec or the key Secret changes. The Argo CD message is `: ` of the `Ready` condition. While the object is being deleted, Argo CD shows `Deleting the Coder AI provider`. + +Add the script as the `argocd-cm` key `resource.customizations.health.coder.com_CoderAIProvider` (`config/gitops/argocd-health-coderaiprovider.lua`): + +```lua +-- Argo CD health check for CoderAIProvider (coder.com/v1alpha1). +-- Install it as the argocd-cm key +-- resource.customizations.health.coder.com_CoderAIProvider. +-- docs/how-to/gitops.md copies this file. A test keeps the copy equal. +local hs = { status = "Progressing", message = "Waiting for the controller" } +if obj.metadata.deletionTimestamp ~= nil then + -- Argo CD shows deletionMessage, not message, for a resource being deleted. + hs.deletionMessage = "Deleting the Coder AI provider" + hs.message = hs.deletionMessage + return hs +end +local st = obj.status +if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then + return hs +end +for _, c in ipairs(st.conditions or {}) do + if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then + hs.message = c.reason .. ": " .. c.message + end +end +for _, c in ipairs(st.conditions or {}) do + if c.type == "Ready" and c.status == "True" then + hs.status = "Healthy" + elseif c.type == "Stalled" and c.status == "True" then + hs.status = "Degraded" + end +end +return hs +``` + +Add this entry (`config/gitops/flux-healthcheck-coderaiprovider.yaml`) to `spec.healthCheckExprs` of a Flux v2.5+ Kustomization with `spec.wait: true`: + +```yaml +# Flux health check expressions for CoderAIProvider (coder.com/v1alpha1). +# Add this entry to spec.healthCheckExprs of a Flux v2.5+ Kustomization +# with spec.wait: true. +# docs/how-to/gitops.md copies this file. A test keeps the copy equal. +- apiVersion: coder.com/v1alpha1 + kind: CoderAIProvider + inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" + failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" + current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" +``` + ## What CI covers The unit test in `config/gitops/` runs the Lua script with gopher-lua, the Lua engine of Argo CD, and the Flux expressions with cel-go, both over fixed test objects. CI does not run Argo CD or Flux. From 3f5530489a099fc577fbca2755bdbd21595bf325 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:19:27 +0000 Subject: [PATCH 2/5] test: fail on a Coder bump until the AI provider seeding facts are re-checked (#321) Change-Id: Ie1f26ea7cabac34dd359578376bc9c5d03e85cb5 Signed-off-by: Thomas Kosiewski --- internal/controller/coderaiprovider_env.go | 2 ++ .../controller/coderaiprovider_env_test.go | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/internal/controller/coderaiprovider_env.go b/internal/controller/coderaiprovider_env.go index 24c8832b7..99a385a09 100644 --- a/internal/controller/coderaiprovider_env.go +++ b/internal/controller/coderaiprovider_env.go @@ -9,6 +9,8 @@ import ( // coderd seeds AI providers from env and flags at startup (Coder v2.37.3 // codersdk/deployment.go:1929-2021, :4440-4561 and cli/server.go:3174-3176). +// TestCoderAIProviderSeedingFactsMatchVendoredCoder fails on a Coder bump +// until these facts are checked again. // A seeded provider row with another config stops coderd from starting // (coderd/ai_providers_migrate.go:76-146). // diff --git a/internal/controller/coderaiprovider_env_test.go b/internal/controller/coderaiprovider_env_test.go index fb4542e1a..505dac1d5 100644 --- a/internal/controller/coderaiprovider_env_test.go +++ b/internal/controller/coderaiprovider_env_test.go @@ -3,6 +3,8 @@ package controller_test import ( "context" "net/http" + "os" + "strings" "testing" "time" @@ -111,3 +113,32 @@ func TestCoderAIProviderPendingCreateBeforeEnvCheck(t *testing.T) { }) } } + +// aiProviderSeedingFactsChecked lists the Coder versions whose source backs +// the seeding facts in coderaiprovider_env.go. v2.37.8 has the same +// codersdk/deployment.go, cli/server.go, codersdk/aiproviders_bedrock.go and +// coderd/ai_providers_migrate.go as v2.37.3. +var aiProviderSeedingFactsChecked = []string{"v2.37.3", "v2.37.8"} + +// #321: a Coder bump fails here until someone re-checks the seeding facts +// against the new Coder source. +func TestCoderAIProviderSeedingFactsMatchVendoredCoder(t *testing.T) { + t.Parallel() + data, err := os.ReadFile("../../vendor/modules.txt") + require.NoError(t, err) + var version string + for line := range strings.Lines(string(data)) { + fields := strings.Fields(line) + if len(fields) >= 3 && fields[0] == "#" && fields[1] == "github.com/coder/coder/v2" { + require.Len(t, fields, 3, "assertion failed: a replaced Coder module needs a manual check of the seeding facts: %s", line) + version = fields[2] + } + } + require.NotEmpty(t, version, "assertion failed: vendor/modules.txt does not list github.com/coder/coder/v2") + require.Contains(t, aiProviderSeedingFactsChecked, version, + "The vendored Coder is %s, but the AI provider seeding facts in coderaiprovider_env.go were checked only at %v. "+ + "Re-check in the Coder %s source which env variables, flags and config file (YAML) keys seed providers: "+ + "codersdk/deployment.go, cli/server.go, codersdk/aiproviders_bedrock.go and coderd/ai_providers_migrate.go. "+ + "Update the facts, their tests and docs/how-to/manage-ai-providers.md#seeding-limits, then add %s to aiProviderSeedingFactsChecked.", + version, aiProviderSeedingFactsChecked, version, version) +} From ff28bd827d1175447667d23818cfae5350c9267f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:19:28 +0000 Subject: [PATCH 3/5] fix: show Coder's enabled flag in the CoderAIProvider ENABLED column and name the kill switch in DisabledExternally (#321) Change-Id: I89cd4e9232b2137ed31723bf53eb5617d99963ad Signed-off-by: Thomas Kosiewski --- api/v1alpha1/coderaiprovider_types.go | 7 +- api/v1alpha1/zz_generated.deepcopy.go | 5 ++ .../crd/bases/coder.com_coderaiproviders.yaml | 8 +- dist/install.yaml | 8 +- docs/how-to/manage-ai-providers.md | 8 +- docs/reference/api/coderaiprovider.md | 1 + .../controller/coderaiprovider_column_test.go | 77 +++++++++++++++++++ .../controller/coderaiprovider_disable.go | 3 + .../coderaiprovider_disable_test.go | 4 +- internal/controller/coderaiprovider_sync.go | 7 +- 10 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 internal/controller/coderaiprovider_column_test.go diff --git a/api/v1alpha1/coderaiprovider_types.go b/api/v1alpha1/coderaiprovider_types.go index 453299b58..a717749e5 100644 --- a/api/v1alpha1/coderaiprovider_types.go +++ b/api/v1alpha1/coderaiprovider_types.go @@ -95,6 +95,11 @@ type CoderAIProviderStatus struct { // ProviderID is the Coder provider ID. // +optional ProviderID string `json:"providerID,omitempty"` + // Enabled is the enabled flag of the Coder provider at the controller's + // last read or update of it. It is empty while Coder holds no provider of + // this object, or before the first read. + // +optional + Enabled *bool `json:"enabled,omitempty"` // CreateAttemptTime is set before every provider create request. While it // is set and ProviderID is empty, a create request can be in flight or // already committed in Coder. @@ -115,7 +120,7 @@ type CoderAIProviderStatus struct { // +kubebuilder:resource:path=coderaiproviders,scope=Namespaced // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` -// +kubebuilder:printcolumn:name="Enabled",type=boolean,JSONPath=`.spec.enabled` +// +kubebuilder:printcolumn:name="Enabled",type=boolean,JSONPath=`.status.enabled` // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" // +kubebuilder:validation:XValidation:rule="size(self.metadata.name) <= 63 && self.metadata.name.matches('^[a-z0-9]+(-[a-z0-9]+)*$')",message="metadata.name is the Coder provider name: at most 63 lowercase letters, digits, and single hyphens between them" diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 4c37b03bc..e70d03b19 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -385,6 +385,11 @@ func (in *CoderAIProviderSpec) DeepCopy() *CoderAIProviderSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CoderAIProviderStatus) DeepCopyInto(out *CoderAIProviderStatus) { *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } if in.CreateAttemptTime != nil { in, out := &in.CreateAttemptTime, &out.CreateAttemptTime *out = (*in).DeepCopy() diff --git a/config/crd/bases/coder.com_coderaiproviders.yaml b/config/crd/bases/coder.com_coderaiproviders.yaml index 6b8a6187e..291b3c7b3 100644 --- a/config/crd/bases/coder.com_coderaiproviders.yaml +++ b/config/crd/bases/coder.com_coderaiproviders.yaml @@ -18,7 +18,7 @@ spec: - jsonPath: .spec.type name: Type type: string - - jsonPath: .spec.enabled + - jsonPath: .status.enabled name: Enabled type: boolean - jsonPath: .status.conditions[?(@.type=="Ready")].status @@ -245,6 +245,12 @@ spec: description: SecretUID is the UID of the Secret that was read. type: string type: object + enabled: + description: |- + Enabled is the enabled flag of the Coder provider at the controller's + last read or update of it. It is empty while Coder holds no provider of + this object, or before the first read. + type: boolean observedGeneration: description: ObservedGeneration is the generation the controller last processed. diff --git a/dist/install.yaml b/dist/install.yaml index 7172ee67e..7348b38a3 100644 --- a/dist/install.yaml +++ b/dist/install.yaml @@ -409,7 +409,7 @@ spec: - jsonPath: .spec.type name: Type type: string - - jsonPath: .spec.enabled + - jsonPath: .status.enabled name: Enabled type: boolean - jsonPath: .status.conditions[?(@.type=="Ready")].status @@ -636,6 +636,12 @@ spec: description: SecretUID is the UID of the Secret that was read. type: string type: object + enabled: + description: |- + Enabled is the enabled flag of the Coder provider at the controller's + last read or update of it. It is empty while Coder holds no provider of + this object, or before the first read. + type: boolean observedGeneration: description: ObservedGeneration is the generation the controller last processed. diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md index 9271550eb..662a08673 100644 --- a/docs/how-to/manage-ai-providers.md +++ b/docs/how-to/manage-ai-providers.md @@ -66,7 +66,9 @@ kubectl -n coder get coderaiproviders kubectl -n coder get coderaiprovider openai -o jsonpath='{range .status.conditions[*]}{.type}={.status} {.reason}: {.message}{"\n"}{end}' ``` -The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled object waits for a change of its spec or of an input. The most common reasons: +The `ENABLED` column shows `status.enabled`: the enabled flag of the provider in Coder at the operator's last read or update. It is empty while Coder holds no provider of this object, for example when the key Secret is refused before the create. + +The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled object waits for a change of its spec or of an input. For Argo CD and Flux health rules, see [Health rules for CoderAIProvider](gitops.md#health-rules-for-coderaiprovider). The most common reasons: | Reason | Meaning | | --- | --- | @@ -77,7 +79,7 @@ The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled ob | `NameConflict` | Coder has a provider with this name that the operator did not create. | | `NameReservedByEnv` | The control plane can seed a provider with this name. See [Seeding limits](#seeding-limits). | | `CoderRejected` | Coder rejected the create or update request. Fix the spec. | -| `DisabledExternally`, `KeyChangedExternally`, `DeletedExternally` | Someone disabled the provider, changed its key or deleted it in Coder. See [Kill switches](#kill-switches). | +| `DisabledExternally`, `KeyChangedExternally`, `DeletedExternally` | Someone disabled the provider, changed its key or deleted it in Coder. `DisabledExternally` also follows the credential kill switch, after the Secret was repaired. See [Kill switches](#kill-switches). | ## Kill switches @@ -85,7 +87,7 @@ The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled ob When the key Secret fails the credential contract for a provider that the operator created, the operator disables the provider in Coder at once. This covers every credential error: the Secret is deleted, loses its label or the destinations annotation, no longer lists the origin that Coder uses, is refused, has no such key, or holds a malformed value. The update holds only `enabled: false`. Coder keeps the old key, but no longer uses it. When a Coder call for the disable fails, including a 404 that Coder did not send (for example from a proxy), the operator retries every 30 seconds, and the condition message starts with `Coder can still send the key`. -The object stalls with the credential reason. Repairing the Secret does not enable the provider again. To enable it, fix the Secret, then edit the spec (any change of `spec`, for example `displayName`). A short read error of the Secret, such as an API server timeout, is not a credential error: the operator retries and changes nothing. +The object stalls with the credential reason. Repairing the Secret does not enable the provider again: the object then reports `DisabledExternally`, and the message names both causes, a Coder admin or the credential kill switch. To enable it, fix the Secret, then edit the spec (any change of `spec`, for example `displayName`). A short read error of the Secret, such as an API server timeout, is not a credential error: the operator retries and changes nothing. ### Dashboard changes diff --git a/docs/reference/api/coderaiprovider.md b/docs/reference/api/coderaiprovider.md index 19cbf542d..acd8ce4f4 100644 --- a/docs/reference/api/coderaiprovider.md +++ b/docs/reference/api/coderaiprovider.md @@ -28,6 +28,7 @@ | `observedGeneration` | integer | ObservedGeneration is the generation the controller last processed. | | `appliedGeneration` | integer | AppliedGeneration is the last generation whose spec Coder fully holds. | | `providerID` | string | ProviderID is the Coder provider ID. | +| `enabled` | boolean | Enabled is the enabled flag of the Coder provider at the controller's last read or update of it. It is empty while Coder holds no provider of this object, or before the first read. | | `createAttemptTime` | [Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#time-v1-meta) | CreateAttemptTime is set before every provider create request. While it is set and ProviderID is empty, a create request can be in flight or already committed in Coder. | | `credential` | [CoderAIProviderCredentialStatus](#coderaiprovidercredentialstatus) | Credential records the Secret value last sent to Coder. | | `conditions` | [Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#condition-v1-meta) array | Conditions are Ready, Reconciling, and Stalled. | diff --git a/internal/controller/coderaiprovider_column_test.go b/internal/controller/coderaiprovider_column_test.go new file mode 100644 index 000000000..2ecf9e09a --- /dev/null +++ b/internal/controller/coderaiprovider_column_test.go @@ -0,0 +1,77 @@ +package controller_test + +import ( + "context" + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" + "github.com/coder/coder-k8s/internal/credentialsecret" +) + +// enabledColumn returns the Enabled cell that kubectl get prints for +// provider, read as a server-side table from the API server. +func enabledColumn(ctx context.Context, t *testing.T, provider *coderv1alpha1.CoderAIProvider) any { + t.Helper() + server, _, err := rest.DefaultServerUrlFor(cfg) + require.NoError(t, err) + httpClient, err := rest.HTTPClientFor(cfg) + require.NoError(t, err) + server.Path = "/apis/coder.com/v1alpha1/namespaces/" + provider.Namespace + "/coderaiproviders/" + provider.Name + req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.String(), nil) + require.NoError(t, err) + req.Header.Set("Accept", "application/json;as=Table;v=v1;g=meta.k8s.io") + resp, err := httpClient.Do(req) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + require.Equal(t, http.StatusOK, resp.StatusCode) + var table metav1.Table + require.NoError(t, json.NewDecoder(resp.Body).Decode(&table)) + require.Len(t, table.Rows, 1) + for i, column := range table.ColumnDefinitions { + if column.Name == "Enabled" { + return table.Rows[0].Cells[i] + } + } + t.Fatal("assertion failed: the table has no Enabled column") + return nil +} + +// #321: the Enabled column shows what Coder holds, not spec.enabled. A +// refused object that Coder does not have shows no value. +func TestCoderAIProviderEnabledColumn(t *testing.T) { + t.Parallel() + ctx := context.Background() + t.Run("refused before a create", func(t *testing.T) { + t.Parallel() + env := newAIProviderEnv(ctx, t, true) + got, _ := env.reconcile(ctx, t, env.createProvider(ctx, t, nil)) + requireAIProviderStep(t, got, true, credentialsecret.ReasonSecretNotFound) + require.True(t, *got.Spec.Enabled) + require.Nil(t, enabledColumn(ctx, t, got)) + }) + t.Run("kill switch, then deleted in Coder", func(t *testing.T) { + t.Parallel() + env, secret, provider := newReadyAIProvider(ctx, t) + require.Equal(t, true, enabledColumn(ctx, t, provider)) + + consented := secret.DeepCopy() + editSecret(ctx, t, secret, func(s *corev1.Secret) { delete(s.Labels, credentialsecret.LabelKey) }) + got, _ := env.reconcile(ctx, t, provider) + requireAIProviderStep(t, got, true, credentialsecret.ReasonNotOptedIn) + require.Equal(t, false, enabledColumn(ctx, t, got)) + + editSecret(ctx, t, secret, func(s *corev1.Secret) { s.Labels = consented.Labels }) + require.NoError(t, env.fake.client(t, 2*time.Second).DeleteAIProvider(ctx, got.Status.ProviderID)) + got, _ = env.reconcile(ctx, t, got) + requireAIProviderStep(t, got, true, "DeletedExternally") + require.Nil(t, enabledColumn(ctx, t, got)) + }) +} diff --git a/internal/controller/coderaiprovider_disable.go b/internal/controller/coderaiprovider_disable.go index acc08229d..cf969d328 100644 --- a/internal/controller/coderaiprovider_disable.go +++ b/internal/controller/coderaiprovider_disable.go @@ -55,6 +55,7 @@ func (r *CoderAIProviderReconciler) disableWithoutConsent( switch { case isCoderResourceNotFound(err): // Only Coder's own 404 proves that nothing can use the key (#327). + provider.Status.Enabled = nil return stalled("Provider %s does not exist in Coder.", provider.Name), nil case err != nil: return unchecked("get provider: %s", coderErrorSummary(err)), nil @@ -62,6 +63,7 @@ func (r *CoderAIProviderReconciler) disableWithoutConsent( if step := checkAIProviderAnswer(ctx, provider, found); step != nil { return step, nil } + provider.Status.Enabled = &found.Enabled if !found.Enabled { return stalled("Provider %s is disabled in Coder.", provider.Name), nil } @@ -89,6 +91,7 @@ func (r *CoderAIProviderReconciler) disableWithoutConsent( if step := checkAIProviderAnswer(ctx, provider, updated); step != nil { return step, nil } + provider.Status.Enabled = &updated.Enabled if updated.Enabled { return aiProviderWait(aiProviderRetry, "CoderAnswerMismatch", "Coder answered the disable of provider %s with an enabled provider. The controller disables it again.", provider.Name), nil diff --git a/internal/controller/coderaiprovider_disable_test.go b/internal/controller/coderaiprovider_disable_test.go index 12ba95172..c1f5dd0b6 100644 --- a/internal/controller/coderaiprovider_disable_test.go +++ b/internal/controller/coderaiprovider_disable_test.go @@ -99,7 +99,9 @@ func TestCoderAIProviderDisablesWithoutConsent(t *testing.T) { editSecret(ctx, t, secret, func(s *corev1.Secret) { s.Labels, s.Annotations = consented.Labels, consented.Annotations }) writes = env.writes() got, _ = env.reconcile(ctx, t, got) - requireAIProviderStep(t, got, true, "DisabledExternally") + // #321: the controller disabled the provider, so the message + // names the kill switch, not only a Coder admin. + require.Contains(t, requireAIProviderStep(t, got, true, "DisabledExternally"), "by a Coder admin or by the controller's credential kill switch") require.Equal(t, writes, env.writes()) stored, _ = env.fake.aiProvider("openai") require.Equal(t, "sk-dashboard-0123456789abcdef", stored.keys[0].plaintext) diff --git a/internal/controller/coderaiprovider_sync.go b/internal/controller/coderaiprovider_sync.go index 019ab9018..07d0c797d 100644 --- a/internal/controller/coderaiprovider_sync.go +++ b/internal/controller/coderaiprovider_sync.go @@ -50,6 +50,7 @@ func (r *CoderAIProviderReconciler) syncAIProvider( if step := checkAIProviderAnswer(ctx, provider, found); step != nil { return step, false, nil } + provider.Status.Enabled = &found.Enabled enabled := provider.Spec.Enabled == nil || *provider.Spec.Enabled keys := found.APIKeys @@ -61,8 +62,10 @@ func (r *CoderAIProviderReconciler) syncAIProvider( // resend that committed without an answer. Such a key stalls too. switch { case applied && enabled && !found.Enabled: + // The credential kill switch keeps appliedGeneration too, so the + // controller itself can have disabled the provider. return aiProviderStalled(aiProviderResync, "DisabledExternally", - "Provider %s was disabled in Coder. The controller does not enable it again until the spec changes.", provider.Name), false, nil + "Provider %s is disabled in Coder, by a Coder admin or by the controller's credential kill switch. The controller does not enable it again until the spec changes.", provider.Name), false, nil case applied && !ownKey && resending: return aiProviderStalled(aiProviderResync, "KeyChangedExternally", "The key of provider %s in Coder is not the recorded key: it was changed in Coder, or a key resend committed without an answer. The controller does not send its key again until the spec changes.", provider.Name), false, nil @@ -141,6 +144,7 @@ func (r *CoderAIProviderReconciler) syncAIProvider( if step := checkAIProviderAnswer(ctx, provider, updated); step != nil { return step, false, nil } + provider.Status.Enabled = &updated.Enabled if updated.DisplayName != cmp.Or(provider.Spec.DisplayName, provider.Name) || updated.Icon != provider.Spec.Icon || updated.Enabled != enabled || updated.BaseURL != provider.Spec.BaseURL { return aiProviderStalled(aiProviderResync, "CoderAnswerMismatch", @@ -168,6 +172,7 @@ func (r *CoderAIProviderReconciler) syncAIProvider( // providerGone handles a provider that is missing in Coder. After a full // apply it is a kill switch. Before one, the object creates it again. func (r *CoderAIProviderReconciler) providerGone(provider *coderv1alpha1.CoderAIProvider, applied bool) (*aiProviderStep, bool, error) { + provider.Status.Enabled = nil if applied { return aiProviderStalled(aiProviderResync, "DeletedExternally", "Provider %s (ID %s) was deleted in Coder. The controller does not create it again until the spec changes.", provider.Name, provider.Status.ProviderID), false, nil From 3191e80d85e15c2dab188f034758d12f41ef55f6 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 15:28:23 +0000 Subject: [PATCH 4/5] test: cover a CoderAIProvider update interrupted before Coder commits (#321) Change-Id: I62d50f4f4f6ae0a361573150674430adb1b0c03c Signed-off-by: Thomas Kosiewski --- config/gitops/health_test.go | 8 +++++ .../controller/coderaiprovider_sync_test.go | 32 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/config/gitops/health_test.go b/config/gitops/health_test.go index fa96a129d..b91405d41 100644 --- a/config/gitops/health_test.go +++ b/config/gitops/health_test.go @@ -197,6 +197,13 @@ func aiProviderCases() []healthCase { stale := readyReconcilingStalled("ready", "Applied") stale["observedGeneration"] = int64(1) noConditions := map[string]any{"observedGeneration": int64(2)} + // An update interrupted before Coder committed it, as stored by + // controllers before the #321 fix: only the top-level generation moved. + interrupted := readyReconcilingStalled("ready", "Applied") + interrupted["appliedGeneration"] = int64(1) + for _, c := range interrupted["conditions"].([]any) { + c.(map[string]any)["observedGeneration"] = int64(1) + } deleting := aiProviderObject(readyReconcilingStalled("ready", "Applied")) deleting["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-30T10:00:00Z" deletingWait := aiProviderObject(readyReconcilingStalled("reconciling", "CoderUnavailable")) @@ -206,6 +213,7 @@ func aiProviderCases() []healthCase { {"stale observedGeneration", aiProviderObject(stale), "Progressing", "Waiting for the controller", "InProgress", ""}, {"no conditions", aiProviderObject(noConditions), "Progressing", "Waiting for the controller", "InProgress", ""}, {"applied", aiProviderObject(readyReconcilingStalled("ready", "Applied")), "Healthy", "Applied: details", "Current", ""}, + {"stale conditions", aiProviderObject(interrupted), "Progressing", "Waiting for the controller", "InProgress", ""}, {"control plane not ready", aiProviderObject(readyReconcilingStalled("reconciling", "ControlPlaneNotReady")), "Progressing", "ControlPlaneNotReady: details", "InProgress", ""}, {"confirming a create", aiProviderObject(readyReconcilingStalled("reconciling", "ConfirmingCreate")), "Progressing", "ConfirmingCreate: details", "InProgress", ""}, // A disable that failed retries with the credential reason. diff --git a/internal/controller/coderaiprovider_sync_test.go b/internal/controller/coderaiprovider_sync_test.go index eb385959c..e02203dc9 100644 --- a/internal/controller/coderaiprovider_sync_test.go +++ b/internal/controller/coderaiprovider_sync_test.go @@ -11,6 +11,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -657,3 +658,34 @@ func TestCoderAIProviderSyncUpdate404NotCoderRead(t *testing.T) { }) } } + +// #321: a key update interrupted before Coder commits leaves the resend +// marker, but status.observedGeneration must not claim the edit before the +// conditions describe it, or GitOps health checks report the old Ready. +func TestCoderAIProviderInterruptedUpdateKeepsObservedGeneration(t *testing.T) { + t.Parallel() + ctx := context.Background() + env, secret, provider := newReadyAIProvider(ctx, t) + readyGeneration := provider.Generation + provider = editSpec(ctx, t, provider, func(s *coderv1alpha1.CoderAIProviderSpec) { s.DisplayName = "OpenAI v2" }) + setSecretValue(ctx, t, secret, testAIProviderKeyRotated) + env.fake.failNext(routeUpdateAIProvider, fakeFault{Hang: true}) + short, cancel := context.WithTimeout(ctx, hangTimeout) + defer cancel() + _, err := env.r.Reconcile(short, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(provider)}) + require.Error(t, err) + + stored := &coderv1alpha1.CoderAIProvider{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(provider), stored)) + require.NotNil(t, stored.Status.Credential.ResendAttemptTime, "the marker was written before the PATCH") + require.Equal(t, readyGeneration, stored.Status.ObservedGeneration, "the marker write does not observe the edit") + require.Equal(t, readyGeneration, meta.FindStatusCondition(stored.Status.Conditions, "Ready").ObservedGeneration) + coder, _ := env.fake.aiProvider("openai") + require.NotEqual(t, "OpenAI v2", coder.displayName) + + got, _ := env.reconcile(ctx, t, stored) + requireAIProviderReady(t, got) + coder, _ = env.fake.aiProvider("openai") + require.Equal(t, "OpenAI v2", coder.displayName) + require.Equal(t, testAIProviderKeyRotated, coder.keys[0].plaintext) +} From 836072265d5101d1d4b115b7cb4c2f84425202aa Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 15:30:45 +0000 Subject: [PATCH 5/5] fix: keep CoderAIProvider observedGeneration with its conditions (#321) A key update writes the resend marker before the PATCH. That write advanced status.observedGeneration while the conditions still described the previous generation, so an interrupted update left a status that the GitOps health rules read as Ready for the unapplied spec. Only applyAIProviderStep moves status.observedGeneration now. The Argo CD and Flux rules also ignore conditions from an older generation. Change-Id: Id66dfc77bf93844320499ec2b37836d3ec3bf266 Signed-off-by: Thomas Kosiewski --- config/gitops/argocd-health-coderaiprovider.lua | 9 ++++++++- .../gitops/flux-healthcheck-coderaiprovider.yaml | 2 +- docs/how-to/gitops.md | 15 +++++++++++---- internal/controller/coderaiprovider_controller.go | 6 ++++-- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/config/gitops/argocd-health-coderaiprovider.lua b/config/gitops/argocd-health-coderaiprovider.lua index 128f0c552..fe43f2836 100644 --- a/config/gitops/argocd-health-coderaiprovider.lua +++ b/config/gitops/argocd-health-coderaiprovider.lua @@ -13,12 +13,19 @@ local st = obj.status if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then return hs end +-- A condition from an older generation does not describe the current spec. +local conditions = {} for _, c in ipairs(st.conditions or {}) do + if c.observedGeneration == obj.metadata.generation then + table.insert(conditions, c) + end +end +for _, c in ipairs(conditions) do if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then hs.message = c.reason .. ": " .. c.message end end -for _, c in ipairs(st.conditions or {}) do +for _, c in ipairs(conditions) do if c.type == "Ready" and c.status == "True" then hs.status = "Healthy" elseif c.type == "Stalled" and c.status == "True" then diff --git a/config/gitops/flux-healthcheck-coderaiprovider.yaml b/config/gitops/flux-healthcheck-coderaiprovider.yaml index 2c56f3b89..baf2aee11 100644 --- a/config/gitops/flux-healthcheck-coderaiprovider.yaml +++ b/config/gitops/flux-healthcheck-coderaiprovider.yaml @@ -4,6 +4,6 @@ # docs/how-to/gitops.md copies this file. A test keeps the copy equal. - apiVersion: coder.com/v1alpha1 kind: CoderAIProvider - inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" + inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || !status.conditions.exists(c, c.type == 'Ready' && has(c.observedGeneration) && c.observedGeneration == metadata.generation) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" diff --git a/docs/how-to/gitops.md b/docs/how-to/gitops.md index 2b4c946d0..5ec375a57 100644 --- a/docs/how-to/gitops.md +++ b/docs/how-to/gitops.md @@ -164,11 +164,11 @@ If the import takes longer than the 34-second write budget of the aggregated API ## Health rules for CoderAIProvider -Argo CD and Flux also need health rules to wait for a [`CoderAIProvider`](manage-ai-providers.md). Without them, Argo CD has no health check for the kind, and Flux reports a new provider as current before the controller writes its first status. The controller writes the conditions `Ready`, `Reconciling` and `Stalled` together, with one reason and message, and sets `status.observedGeneration` on every status write. Both rules wait until `status.observedGeneration` equals `metadata.generation`. Then they read the conditions: +Argo CD and Flux also need health rules to wait for a [`CoderAIProvider`](manage-ai-providers.md). Without them, Argo CD has no health check for the kind, and Flux reports a new provider as current before the controller writes its first status. The controller writes the conditions `Ready`, `Reconciling` and `Stalled` together, with one reason and message. It sets `status.observedGeneration` to `metadata.generation` only when it writes the conditions for that generation. Both rules wait until `status.observedGeneration` equals `metadata.generation`. They also ignore a condition whose own `observedGeneration` is older, so a status that an older controller wrote in the middle of an update does not count as healthy. Then they read the conditions: | Conditions | Argo CD | Flux | | --- | --- | --- | -| no status, or a stale `observedGeneration` | `Progressing` | `InProgress` (`Unknown` before the first status) | +| no status, or a stale `observedGeneration` on the status or the `Ready` condition | `Progressing` | `InProgress` (`Unknown` before the first status) | | `Reconciling=True` | `Progressing` | `InProgress` | | `Ready=True` | `Healthy` | `Current` | | `Stalled=True` | `Degraded` | `Failed` | @@ -193,12 +193,19 @@ local st = obj.status if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then return hs end +-- A condition from an older generation does not describe the current spec. +local conditions = {} for _, c in ipairs(st.conditions or {}) do + if c.observedGeneration == obj.metadata.generation then + table.insert(conditions, c) + end +end +for _, c in ipairs(conditions) do if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then hs.message = c.reason .. ": " .. c.message end end -for _, c in ipairs(st.conditions or {}) do +for _, c in ipairs(conditions) do if c.type == "Ready" and c.status == "True" then hs.status = "Healthy" elseif c.type == "Stalled" and c.status == "True" then @@ -217,7 +224,7 @@ Add this entry (`config/gitops/flux-healthcheck-coderaiprovider.yaml`) to `spec. # docs/how-to/gitops.md copies this file. A test keeps the copy equal. - apiVersion: coder.com/v1alpha1 kind: CoderAIProvider - inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" + inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || !status.conditions.exists(c, c.type == 'Ready' && has(c.observedGeneration) && c.observedGeneration == metadata.generation) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" ``` diff --git a/internal/controller/coderaiprovider_controller.go b/internal/controller/coderaiprovider_controller.go index c8f96fa1c..2fede36bb 100644 --- a/internal/controller/coderaiprovider_controller.go +++ b/internal/controller/coderaiprovider_controller.go @@ -347,8 +347,11 @@ func (r *CoderAIProviderReconciler) patchFinalizer(ctx context.Context, provider } // applyAIProviderStep sets the kstatus conditions for step. Messages are cut -// to the CoderTemplateTest limit. +// to the CoderTemplateTest limit. status.observedGeneration moves only here, +// so a status write between two steps never claims a generation that the +// conditions do not describe yet. func applyAIProviderStep(provider *coderv1alpha1.CoderAIProvider, step *aiProviderStep) { + provider.Status.ObservedGeneration = provider.Generation message := truncateTemplateTestMessage(step.message) ready, reconciling, stalled := metav1.ConditionFalse, metav1.ConditionTrue, metav1.ConditionFalse switch { @@ -373,7 +376,6 @@ func applyAIProviderStep(provider *coderv1alpha1.CoderAIProvider, step *aiProvid // writeStatus writes the status only when it changed. func (r *CoderAIProviderReconciler) writeStatus(ctx context.Context, provider *coderv1alpha1.CoderAIProvider, before *coderv1alpha1.CoderAIProviderStatus) error { - provider.Status.ObservedGeneration = provider.Generation if equality.Semantic.DeepEqual(before, &provider.Status) { return nil }