diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md index d79020d2..0bc9c157 100644 --- a/docs/how-to/manage-ai-providers.md +++ b/docs/how-to/manage-ai-providers.md @@ -10,7 +10,7 @@ A `CoderAIProvider` manages one AI provider in Coder (Coder's `/api/v2/ai/provid - A `CoderControlPlane` in the same namespace with `status.operatorAccessReady: true`. The operator user must be a site owner. Coder answers `403` to anyone else on the provider routes. - An API key for the provider, for example an OpenAI key. - Coder v2.37.2 or later. The Kind end-to-end test runs Coder v2.37.2, and the operator is built against the v2.37.3 SDK. Coder has the provider API since v2.34.0, but versions before v2.37.2 are not tested. -- An operator release after v0.1.3. The `dist/install.yaml` on `main` installs the `CoderAIProvider` CRD, but its `ghcr.io/coder/coder-k8s:latest` image is v0.1.3 until the next release, and v0.1.3 has no `CoderAIProvider` controller. Install from a release tag that includes `CoderAIProvider`, and pin the image to the same version (see [Deploy a Coder Control Plane](../tutorials/getting-started.md#1-install-the-operator)). +- coder-k8s v0.1.4 or later. Install from the release tag, and pin the image to the same version (see [Deploy a Coder Control Plane](../tutorials/getting-started.md#1-install-the-operator)). ## 1. Prepare the key Secret @@ -76,7 +76,7 @@ The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled ob | `CredentialSecretNotFound`, `CredentialNotOptedIn`, `CredentialDestinationNotAllowed`, `CredentialRefused`, `CredentialKeyMissing`, `CredentialMalformed` | The key Secret fails the credential contract. See [the credential kill switch](#the-credential-kill-switch). | | `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 request. Fix the spec. | +| `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). | ## Kill switches diff --git a/internal/controller/codertemplatetest_confirm.go b/internal/controller/codertemplatetest_confirm.go index 6da72739..6f3240e9 100644 --- a/internal/controller/codertemplatetest_confirm.go +++ b/internal/controller/codertemplatetest_confirm.go @@ -61,7 +61,8 @@ func (r *CoderTemplateTestReconciler) confirmCreate( // a deleted one only without a live one (workspaceByOwnerAndName). workspace, err := sdk.WorkspaceByOwnerAndName(ctx, tt.Status.OwnerID, tt.Status.WorkspaceName, codersdk.WorkspaceOptions{IncludeDeleted: true}) found := err == nil - if !found && !isCoderNotFound(err) { + // Only Coder's own 404 proves that no workspace exists (#351). + if !found && !isCoderResourceNotFound(err) { return coderUnavailable("get workspace by name", err), nil } live := false diff --git a/internal/controller/codertemplatetest_confirm_test.go b/internal/controller/codertemplatetest_confirm_test.go index c667bb75..9b171a5e 100644 --- a/internal/controller/codertemplatetest_confirm_test.go +++ b/internal/controller/codertemplatetest_confirm_test.go @@ -156,6 +156,26 @@ func TestTemplateTestConfirmSettleWindow(t *testing.T) { require.Equal(t, 1, e.fake.requestCount(routeCreateWorkspace), "never a second create request") } +// Only Coder's own 404 proves that no workspace exists. A proxy's 404 page, +// or "Route not found." for a wrong base URL, keeps the marker and waits, +// also after the settle window (#351). +func TestTemplateTestConfirmTrustsOnlyCodersNotFound(t *testing.T) { + t.Parallel() + e := newTemplateTestEnv(t) + for _, tc := range nonCoderNotFounds { + t.Run(tc.name, func(t *testing.T) { // Not parallel: the cases share e and its clock. + creates := e.fake.requestCount(routeCreateWorkspace) + key := e.uncertainCreate(t, fakeFault{Status: 502}, 3600) + e.clock.SetTime(e.clock.Now().Add(15 * time.Minute)) + e.fake.failNext(routeWorkspaceByName, tc.fault) + requireTemplateTestRunning(t, e.reconcile(t, key, 1), "CoderUnavailable", "get workspace by name: Coder answered 404") + // Coder's own 404 after the window proves that no workspace exists. + requireTemplateTestFailed(t, e.settle(t, key), "CreateOutcomeUnknown") + require.Equal(t, creates+1, e.fake.requestCount(routeCreateWorkspace), "never a second create request") + }) + } +} + func TestTemplateTestConfirmWrongAnswers(t *testing.T) { t.Parallel() e := newTemplateTestEnv(t) diff --git a/internal/controller/codertemplatetest_controller.go b/internal/controller/codertemplatetest_controller.go index a15accc7..9356a129 100644 --- a/internal/controller/codertemplatetest_controller.go +++ b/internal/controller/codertemplatetest_controller.go @@ -281,7 +281,7 @@ func (r *CoderTemplateTestReconciler) resolveInputs( // Nothing of this test exists yet, so the workspace belongs to // someone else. The controller never touches it. return templateTestFail("WorkspaceNameConflict", "Coder user %s already has a workspace named %s.", ownerID, workspaceName), nil - case !isCoderNotFound(err): + case !isCoderResourceNotFound(err): // Only Coder's own 404 proves that the name is free (#351). return coderUnavailable("get workspace by name", err), nil } return nil, nil diff --git a/internal/controller/codertemplatetest_create_test.go b/internal/controller/codertemplatetest_create_test.go index c75f476e..0b59a1b1 100644 --- a/internal/controller/codertemplatetest_create_test.go +++ b/internal/controller/codertemplatetest_create_test.go @@ -241,6 +241,37 @@ func TestTemplateTestConfirmKeepsCreateAnswer(t *testing.T) { } } +// nonCoderNotFounds are 404 answers that Coder itself never gives on the +// workspace routes: a proxy's page, and Coder's own answer to a wrong route +// (coderd/httpapi/httpapi.go:180 in v2.37.3). +var nonCoderNotFounds = []struct { + name string + fault fakeFault +}{ + {name: "HTML 404", fault: fakeFault{Status: 404, HTML: "404 Not Found"}}, + {name: "Route not found", fault: fakeFault{Status: 404, Message: "Route not found."}}, +} + +// Before the create, only Coder's own 404 proves that the workspace name is +// free. Another 404 waits and sends no create request (#351). +func TestTemplateTestLookupTrustsOnlyCodersNotFound(t *testing.T) { + t.Parallel() + e := newTemplateTestEnv(t) + for _, tc := range nonCoderNotFounds { + t.Run(tc.name, func(t *testing.T) { // Not parallel: the cases share e. + creates := e.fake.requestCount(routeCreateWorkspace) + key := e.createTest(t, "default.docker", coderv1alpha1.CoderTemplateTestVersion{Name: "v1"}) + e.fake.failNext(routeWorkspaceByName, tc.fault) + tt := e.reconcile(t, key, 3) // Finalizer, initialization, then lookups. + requireTemplateTestWaiting(t, tt, "CoderUnavailable", "get workspace by name: Coder answered 404") + require.Nil(t, tt.Status.CreateAttemptTime, "no marker without a create request") + require.Equal(t, creates, e.fake.requestCount(routeCreateWorkspace), "no create request") + requireTemplateTestRunning(t, e.settle(t, key), "WaitingForBuild", "") + require.Equal(t, creates+1, e.fake.requestCount(routeCreateWorkspace)) + }) + } +} + func TestTemplateTestDeadlineAfterCreate(t *testing.T) { t.Parallel() e := newTemplateTestEnv(t) diff --git a/internal/controller/coderworkspacepoolmember_ready.go b/internal/controller/coderworkspacepoolmember_ready.go index aec9c13e..31b798bc 100644 --- a/internal/controller/coderworkspacepoolmember_ready.go +++ b/internal/controller/coderworkspacepoolmember_ready.go @@ -57,7 +57,8 @@ func confirmPoolMemberCreate( // With include_deleted, Coder answers the live workspace first. workspace, err := sdk.WorkspaceByOwnerAndName(ctx, spec.OwnerID, name, codersdk.WorkspaceOptions{IncludeDeleted: true}) found := err == nil - if !found && !isCoderNotFound(err) { + // Only Coder's own 404 proves that no workspace exists (#351). + if !found && !isCoderResourceNotFound(err) { return coderStepUnavailable("get workspace by name", err), nil } live := false diff --git a/internal/controller/coderworkspacepoolmember_ready_test.go b/internal/controller/coderworkspacepoolmember_ready_test.go index 11482ead..090c3bcf 100644 --- a/internal/controller/coderworkspacepoolmember_ready_test.go +++ b/internal/controller/coderworkspacepoolmember_ready_test.go @@ -138,6 +138,30 @@ func TestPoolMemberReadinessFailures(t *testing.T) { } } +// Only Coder's own 404 proves that no workspace of the member exists. A +// proxy's 404 page, or "Route not found.", keeps the marker and waits, also +// after the settle window (#351). +func TestPoolMemberConfirmTrustsOnlyCodersNotFound(t *testing.T) { + t.Parallel() + e := newPoolMemberEnv(t) + long := func(m *coderv1alpha1.CoderWorkspacePoolMember) { m.Spec.ReadyTimeoutSeconds = 7200 } + for _, tc := range nonCoderNotFounds { + t.Run(tc.name, func(t *testing.T) { // Not parallel: the cases share e and its clock. + creates := e.fake.requestCount(routeCreateWorkspace) + key := e.uncertainMember(t, fakeFault{Status: 502}, long) + marker := e.reconcileMember(t, key, 0).Status.CreateAttemptTime.Time + e.clock.SetTime(marker.Add(15 * time.Minute)) + e.fake.failNext(routeWorkspaceByName, tc.fault) + member := e.reconcileMember(t, key, 1) + requireMember(t, member, coderv1alpha1.CoderWorkspacePoolMemberPhaseCreating, "CoderUnavailable", "get workspace by name: Coder answered 404") + require.NotNil(t, member.Status.CreateAttemptTime, "the marker stays") + // Coder's own 404 after the window proves that no workspace exists. + requireMemberFailed(t, e.reconcileMember(t, key, 1), "CreateOutcomeUnknown", "appeared within 15m0s", metav1.ConditionTrue) + require.Equal(t, creates+1, e.fake.requestCount(routeCreateWorkspace), "never a second create request") + }) + } +} + func TestPoolMemberConfirmsCreate(t *testing.T) { t.Parallel() e := newPoolMemberEnv(t) diff --git a/internal/controller/fakecoder_test.go b/internal/controller/fakecoder_test.go index b416130f..91468374 100644 --- a/internal/controller/fakecoder_test.go +++ b/internal/controller/fakecoder_test.go @@ -615,11 +615,12 @@ func (f *fakeCoder) createWorkspace(r *http.Request) (int, any) { } // getWorkspaceByOwnerAndName prefers the live workspace. With -// include_deleted=true it falls back to the newest deleted one. +// include_deleted=true it falls back to the newest deleted one. Its 404 has +// Coder's own message, which the controllers check (#351). func (f *fakeCoder) getWorkspaceByOwnerAndName(r *http.Request) (int, any) { owner, ok := f.userByIdent(r.PathValue("user")) if !ok { - return fakeError(http.StatusNotFound, "Resource not found.") + return http.StatusNotFound, fakeResourceNotFound } name := r.PathValue("name") if fw := f.liveWorkspace(owner.ID, name); fw != nil { @@ -634,7 +635,7 @@ func (f *fakeCoder) getWorkspaceByOwnerAndName(r *http.Request) (int, any) { } } if newest == nil { - return fakeError(http.StatusNotFound, "Resource not found.") + return http.StatusNotFound, fakeResourceNotFound } return http.StatusOK, newest.view() }