Skip to content

Commit d63b3db

Browse files
authored
🤖 feat: unify controller, aggregated API server, and MCP into --app=all default mode (#53)
## Summary Adds a new `--app=all` mode (now the **default**) that runs the controller, aggregated API server, and MCP HTTP server in a single process with a shared controller-runtime cache/client. This eliminates the need for three separate Deployments and enables single-pod deployment. ## Background Previously, `coder-k8s` required deploying three separate pods: - `--app=controller` — the operator managing `CoderControlPlane`, `WorkspaceProxy`, and `CoderProvisioner` CRDs - `--app=aggregated-apiserver` — the aggregated API server for `CoderWorkspace`/`CoderTemplate` (required manual `--coder-url`, `--coder-session-token`, `--coder-namespace` flags) - `--app=mcp-http` — the MCP server for AI tool integration This created operational overhead and prevented the MCP server from sharing the controller's cache-backed informers, leading to redundant API reads. ## Implementation ### Architecture: Shared cache composition root Created `internal/app/allapp/allapp.go` as a **single composition root** that: 1. Builds **one** controller-runtime manager with a shared scheme (core + `coder.com/v1alpha1` + `aggregation.coder.com/v1alpha1`) 2. Wires all three components through the same manager: - **Controller reconcilers** — registered via `controllerapp.SetupControllers(mgr)`, leader-election gated - **Aggregated API server** — runs as a non-leader runnable (all replicas), uses dynamic control-plane discovery - **MCP HTTP server** — runs as a non-leader runnable, shares the manager's cache-backed `client.Client` ### Dynamic control-plane discovery The aggregated API server no longer requires manual `--coder-url`/`--coder-session-token` flags in `all` mode. Instead, a new `ControlPlaneClientProvider` (`internal/aggregated/coder/controlplane_provider.go`) dynamically discovers Coder instances: 1. Lists `CoderControlPlane` objects in the request namespace 2. Filters eligible instances (operator access enabled + ready, secret ref present, URL present, name doesn't contain `.`) 3. Fetches the operator token from the referenced Secret (using the manager's **uncached** API reader to avoid caching secrets in informers) 4. Builds a codersdk client for API calls ### Refactored components for reuse - **`controllerapp`**: Extracted `NewManager()`, `SetupControllers()`, `SetupProbes()` for reuse by `allapp` - **`mcpapp`**: Added `RunHTTPWithClients()` to accept injected clients instead of constructing its own - **`sharedscheme`**: New package providing a unified scheme used by all components - **`apiserverapp`**: Added `Options.ClientProvider` override so `allapp` can inject the dynamic provider ### Unified deployment manifests Replaced three separate Deployments with a single `deploy/deployment.yaml`: - One Deployment, one ServiceAccount, one ClusterRole (union of all permissions) - No `--app` arg needed (defaults to `all`) - Exposes all three ports: `8081` (health), `6443` (aggregated API), `8090` (MCP) - Services updated to target the unified pod selector ### Backward compatibility Existing explicit `--app=controller`, `--app=aggregated-apiserver`, and `--app=mcp-http` modes remain fully functional for users who prefer split deployments. ## Validation - `make verify-vendor` ✅ - `make build` ✅ - `make test` ✅ (all existing + new tests) - `make lint` ✅ ## Risks - **Memory**: Running all three components in one process increases per-pod memory. The shared cache helps (avoids duplicate informers between controller and MCP), but the aggregated API server's codersdk calls are still per-request. - **Blast radius**: A crash in any component takes down all three. Existing split deployment mode remains available as a mitigation path. - **Multi-instance**: The dynamic provider currently returns `BadRequest` when multiple eligible `CoderControlPlane` instances exist in the same namespace. Multi-instance support (with control-plane-prefixed naming) is planned as a follow-up. --- <details> <summary>📋 Implementation Plan</summary> # Plan: unify controller + aggregated API server + MCP into one `--app=all` (shared cache) ## Context / Why Today `coder-k8s` runs as **one of** three separate app modes selected by `--app`: - `controller` → controller-runtime operator for `coder.com/v1alpha1` (`CoderControlPlane`, etc.) - `aggregated-apiserver` → aggregated API server for `aggregation.coder.com/v1alpha1` (`CoderWorkspace`, `CoderTemplate`) - `mcp-http` → MCP server (HTTP transport) that interacts with both sets of APIs Goal: 1. Add a new app mode **`--app=all`** and make it the **default**. 2. In `all` mode, run all three components **in a single process** and support deploying them **as a single pod**. 3. Ensure the controller and MCP server **share a single controller-runtime cache/client**, so we don’t run separate informer stacks or issue redundant API reads. Non-goals (for this change): - Re-architect the codersdk-backed aggregated API storage beyond adding dynamic control-plane routing. - Redesign authn/z of the aggregated API server (it currently uses anonymous + always-allow). --- ## Evidence (what we verified) From repo inspection: - `app_dispatch.go` currently requires `--app` and supports: `controller`, `aggregated-apiserver`, `mcp-http`. - `internal/app/controllerapp/controllerapp.go` creates a controller-runtime `Manager` and uses the manager’s cache-backed client. - `api/v1alpha1/codercontrolplane_types.go` + `internal/controller/codercontrolplane_controller.go` implement operator access: - `spec.operatorAccess.disabled` gates whether the operator/admin token is managed - `status.operatorTokenSecretRef` points at the Secret/key containing the admin token - `status.operatorAccessReady` indicates token bootstrap success - `status.url` is the in-cluster coderd base URL used for service discovery - `internal/app/apiserverapp/apiserverapp.go` serves `coderworkspaces`/`codertemplates` via **codersdk-backed** storage (`internal/aggregated/storage/*`). - Storage resolves a codersdk client through `internal/aggregated/coder.ClientProvider`. - Current deployment mode uses `coder.NewStaticClientProvider` configured via `--coder-url`, `--coder-session-token`, and `--coder-namespace`. - `internal/app/mcpapp/server.go` builds its own **non-cached** controller-runtime client (`client.New`) + client-go clientset; `mcpapp.NewServer` already supports injecting a `client.Client` + `clientset`. - `internal/app/mcpapp/tools.go` uses: - controller-runtime client for CRUD/listing of `CoderControlPlane`, `CoderWorkspace`, `CoderTemplate`, Pods, Services, Deployments - client-go clientset for Events and Pod logs - `controller-runtime` cache supports starting new informers on-demand by default (`ReaderFailOnMissingInformer` defaults to `false`). - `controller-runtime` `Manager.Add(...)` behavior: any runnable that doesn’t implement `LeaderElectionRunnable` is treated as **leader-election gated**. To run MCP/APIServer on all replicas, they must implement `NeedLeaderElection() bool { return false }`. - `deploy/` currently has **three Deployments** and separate ServiceAccounts/RBAC; a single pod requires **one ServiceAccount** with the union of permissions. Key files consulted: - `app_dispatch.go`, `main_test.go` - `internal/app/controllerapp/controllerapp.go` - `internal/app/apiserverapp/apiserverapp.go` - `internal/app/mcpapp/http.go`, `internal/app/mcpapp/server.go`, `internal/app/mcpapp/tools.go` - `deploy/*.yaml` (controller/apiserver/mcp) - `vendor/sigs.k8s.io/controller-runtime/pkg/cache/*`, `vendor/.../pkg/manager/*` --- ## Implementation details ### 1) Introduce an `all`-mode “composition root” package Create a new package: - `internal/app/allapp/allapp.go` It will be the **single place** where we wire together: - controller-runtime manager (shared cache) - aggregated API server runnable - MCP HTTP server runnable High-level shape: ```go package allapp func Run(ctx context.Context) error { if ctx == nil { return fmt.Errorf("assertion failed: context must not be nil") } // 1) Build shared scheme (core + coder + aggregation) // 2) Build one rest.Config // 3) Build one controller-runtime manager // 4) Setup reconcilers on the manager // 5) Add MCP + aggregated apiserver as non-leader runnables // 6) Start manager (single blocking call) } ``` Why a dedicated package? - Keeps `main`/dispatch simple. - Guarantees `all` mode really shares a single cache and config. - Makes it easier to test cache sharing via dependency injection. <details> <summary>Why not just run the existing 3 Run() functions concurrently?</summary> Calling `controllerapp.Run(ctx)` + `mcpapp.RunHTTP(ctx)` concurrently would *not* share controller-runtime cache/client instances, because `controllerapp.Run` constructs a `Manager` internally and `mcpapp.RunHTTP` constructs its own client/clientset. To share cache, we need a composition root that constructs the manager once and passes its cache-backed client into MCP. </details> --- ### 2) Create a shared scheme builder used by controller + MCP We need the manager’s scheme to include: - `clientgoscheme.AddToScheme` - `coderv1alpha1.AddToScheme` - `aggregationv1alpha1.AddToScheme` (MCP reads/patches these types) Options (pick one; recommended is **A**): A) Create `internal/app/sharedscheme/sharedscheme.go`: ```go package sharedscheme func New() *runtime.Scheme { scheme := runtime.NewScheme() utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(coderv1alpha1.AddToScheme(scheme)) utilruntime.Must(aggregationv1alpha1.AddToScheme(scheme)) return scheme } ``` Then: - `controllerapp.NewScheme()` becomes a thin wrapper calling `sharedscheme.New()` (or remove/replace). - `mcpapp.newScheme()` becomes a thin wrapper calling `sharedscheme.New()`. - `allapp` uses `sharedscheme.New()`. B) Expand `controllerapp.NewScheme()` to also register `aggregationv1alpha1` and delete `mcpapp.newScheme()`. (A avoids circular dependencies and keeps the scheme definition single-sourced.) --- ### 3) Refactor controller mode to allow reuse of the manager (minimal surgical extraction) Currently `controllerapp.Run` both **builds** the manager and **starts** it. We want `allapp` to build the manager once and start it once, but reuse controller setup. Edits in `internal/app/controllerapp/controllerapp.go`: 1) Extract manager construction: ```go func NewManager(cfg *rest.Config, scheme *runtime.Scheme) (manager.Manager, error) { // asserts // ctrl.NewManager(cfg, ctrl.Options{ ... }) } ``` 2) Extract controller wiring: ```go func SetupControllers(mgr manager.Manager) error { // create reconcilers and call SetupWithManager } ``` 3) Extract health checks: ```go func SetupProbes(mgr manager.Manager) error { // AddHealthzCheck + AddReadyzCheck } ``` 4) Keep existing `Run(ctx)` behavior by composing the above: ```go func Run(ctx context.Context) error { cfg := ctrl.GetConfigOrDie() scheme := sharedscheme.New() mgr, err := NewManager(cfg, scheme) ... if err := SetupControllers(mgr); err != nil { ... } if err := SetupProbes(mgr); err != nil { ... } return mgr.Start(ctx) } ``` Also: `detectLeaderElectionNamespace()` is currently unexported; keep it in controllerapp and use it from `NewManager`. --- ### 4) Make MCP HTTP server runnable accept injected (shared) clients We want MCP to use the manager’s **cache-backed** client: - `k8sClient := mgr.GetClient()` and a clientset built from the same config: - `clientset := kubernetes.NewForConfig(mgr.GetConfig())` Edits in `internal/app/mcpapp/http.go`: - Add a new function (preferred): ```go func RunHTTPWithClients(ctx context.Context, k8sClient client.Client, clientset kubernetes.Interface) error { // assert ctx/k8sClient/clientset server := NewServer(k8sClient, clientset) // start HTTP server (existing logic) } ``` - Keep `RunHTTP(ctx)` for standalone mode by delegating: ```go func RunHTTP(ctx context.Context) error { k8sClient, clientset, err := newClients() ... return RunHTTPWithClients(ctx, k8sClient, clientset) } ``` This allows `allapp` to reuse the same cache-backed client **without** duplicating MCP’s HTTP lifecycle code. --- ### 5) Aggregated API server in `all` mode: dynamic discovery + operator-token auth Current state (post-rebase): - Aggregated API storage is already **codersdk-backed** (`internal/aggregated/storage/*`). - Storage resolves a Coder API client through `internal/aggregated/coder.ClientProvider` (today: a namespace-pinned static provider built from `--coder-url`, `--coder-session-token`, `--coder-namespace` in `apiserverapp.buildClientProvider`). Goal for `all` mode: - No manual `--coder-*` wiring. - For each request, discover the relevant `CoderControlPlane`(s), fetch the operator admin token from the Secret created by the controller, and use it for codersdk calls. - If `spec.operatorAccess.disabled: true`, that control plane must be treated as **invisible**: do not read its Secret, do not call its coderd, and do not return its resources. #### 5.1) Implement a control-plane-backed ClientProvider Add `internal/aggregated/coder/controlplane_provider.go` (name flexible) that uses Kubernetes to build codersdk clients on demand. Recommended interface evolution (minimal but supports LIST across instances): ```go type ClientProvider interface { // Existing behavior: used by GET/CREATE/UPDATE/DELETE once the control plane is known. ClientForNamespace(ctx context.Context, namespace string) (*codersdk.Client, error) // New: used by LIST to support multiple control planes per namespace and cluster-wide listing. ClientsForNamespace(ctx context.Context, namespace string) (map[types.NamespacedName]*codersdk.Client, error) } ``` Provider construction inputs (from `allapp`): - `client.Reader` for `CoderControlPlane` discovery (cached `mgr.GetClient()` is fine) - `client.Reader` for Secret reads (**prefer `mgr.GetAPIReader()` to avoid caching secret tokens in informers**) - request timeout (reuse `--coder-request-timeout`, default 30s) Eligibility filter (MUST run before any Secret read or coderd call): - `cp.Spec.OperatorAccess.Disabled == true` ⇒ skip - `cp.Status.OperatorAccessReady != true` ⇒ skip - `cp.Status.OperatorTokenSecretRef == nil` ⇒ skip - `strings.TrimSpace(cp.Status.URL) == ""` ⇒ skip Token fetch: - Secret namespace: `cp.Namespace` - Secret name: `cp.Status.OperatorTokenSecretRef.Name` - Secret key: `cp.Status.OperatorTokenSecretRef.Key` (default to `coderv1alpha1.DefaultTokenSecretKey` when empty) SDK construction: - Parse `cp.Status.URL` as a `url.URL` - `coder.NewSDKClient(coder.Config{CoderURL: parsed, SessionToken: token, RequestTimeout: timeout})` Error handling rules: - Never log/return token contents. - If a namespace has **zero eligible** control planes, LIST returns an empty list; GET returns NotFound. - If a namespace has **multiple eligible** control planes and the request doesn’t identify one, return BadRequest with guidance (see §5.2). #### 5.2) Disambiguate multiple control planes per namespace (instance selection) To support “multiple deployments in the same namespace” without collisions, encode the `CoderControlPlane` name into aggregated object names. Recommended naming scheme: - Workspace: `<control-plane>.<org>.<user>.<workspace>` - Template: `<control-plane>.<org>.<template>` Implementation: - Update `internal/aggregated/coder/names.go`: - Add Parse/Build helpers that include a control-plane segment. - Keep backward compatibility: accept old names (no control-plane segment) only when exactly one eligible control plane exists in that namespace; otherwise return BadRequest asking for the prefixed form. - Update `api/aggregation/v1alpha1/types.go` comments to match the new naming scheme. - Add labels for filtering/debugging: - `aggregation.coder.com/control-plane-name=<cp.Name>` - `aggregation.coder.com/control-plane-namespace=<cp.Namespace>` <details> <summary>Note: control plane names containing '.'</summary> The aggregated naming scheme uses '.' as a separator. Kubernetes object names may contain '.', so for the initial implementation we will **treat any `CoderControlPlane` whose name contains '.' as ineligible for aggregation** (skip it) and surface a clear warning/error explaining why it’s being skipped. </details> #### 5.3) Update converters + storage to use the dynamic provider Update converters: - `internal/aggregated/convert/workspace.go` and `template.go`: - accept `controlPlaneName string` - build names with the new prefix - set the control-plane labels Update storages: - `internal/aggregated/storage/workspace.go` and `template.go`: - LIST: - if request namespace is set: call `provider.ClientsForNamespace(ctx, namespace)` and merge results across all eligible control planes - if request namespace is empty (kubectl -A): call `provider.ClientsForNamespace(ctx, "")` and merge across all namespaces - GET/CREATE/UPDATE/DELETE: - parse the control-plane segment from `name` - resolve the appropriate codersdk client (either via a dedicated `ClientForControlPlane` helper, or by storing the chosen control-plane in `ctx` before calling `ClientForNamespace`) Critically: when operator access is disabled, objects from that control plane must not appear in LIST results and must behave like NotFound/BadRequest for other verbs. #### 5.4) Wire dynamic provider only in `all` mode Update `internal/app/apiserverapp/apiserverapp.go`: - Extend `Options` with a provider override, e.g. `ClientProvider coder.ClientProvider`. - In `RunWithOptions`, if override is non-nil, use it; otherwise keep the existing static-provider behavior for `--app=aggregated-apiserver`. In `internal/app/allapp/allapp.go`: - Construct the control-plane-backed provider using `mgr.GetClient()` + `mgr.GetAPIReader()`. - Pass it to `apiserverapp.RunWithOptions`. --- ### 6) Run aggregated apiserver + MCP as non-leader manager runnables In `internal/app/allapp/allapp.go`: 1) Build one manager: - `scheme := sharedscheme.New()` - `cfg := ctrl.GetConfigOrDie()` - `mgr, err := controllerapp.NewManager(cfg, scheme)` - `controllerapp.SetupControllers(mgr)` - `controllerapp.SetupProbes(mgr)` 2) Add the aggregated API server as a runnable. Because we want it to run on **every pod replica** (not gated by leader election), wrap it in a type implementing `LeaderElectionRunnable`: ```go type nonLeaderRunnable struct { run func(context.Context) error } func (r nonLeaderRunnable) Start(ctx context.Context) error { return r.run(ctx) } func (r nonLeaderRunnable) NeedLeaderElection() bool { return false } ``` Then (wait for cache sync and run with the dynamic control-plane provider): ```go _ = mgr.Add(nonLeaderRunnable{run: func(ctx context.Context) error { syncCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() if ok := mgr.GetCache().WaitForCacheSync(syncCtx); !ok { return fmt.Errorf("cache did not sync before timeout") } provider, err := coder.NewControlPlaneClientProvider( mgr.GetClient(), // cached discovery for CoderControlPlane mgr.GetAPIReader(), // uncached Secret reads (avoid caching tokens) 30*time.Second, ) if err != nil { return err } return apiserverapp.RunWithOptions(ctx, apiserverapp.Options{ ClientProvider: provider, CoderRequestTimeout: 30 * time.Second, }) }}) ``` 3) Add MCP runnable that waits for cache sync before serving: ```go _ = mgr.Add(nonLeaderRunnable{run: func(ctx context.Context) error { // Defensive: don’t serve MCP until cache is ready. syncCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() if ok := mgr.GetCache().WaitForCacheSync(syncCtx); !ok { return fmt.Errorf("cache did not sync before timeout") } clientset, err := kubernetes.NewForConfig(mgr.GetConfig()) ... return mcpapp.RunHTTPWithClients(ctx, mgr.GetClient(), clientset) }}) ``` 4) Finally start the manager: ```go return mgr.Start(ctx) ``` This yields a **single top-level blocking call** and ensures the MCP server uses the **exact same cache and config** as the operator. <details> <summary>Note on cache size / informers</summary> Using `mgr.GetClient()` in MCP means reads/lists will go through the shared cache and may start informers for object types MCP touches (Pods, Services, Deployments, aggregated API types, etc.). This is desirable for “share caches” but may increase memory usage vs direct REST reads. If this becomes an issue, a follow-up optimization is: - set `Cache.ReaderFailOnMissingInformer = true` - use `mgr.GetAPIReader()` for types we explicitly do **not** want to cache (e.g., Pods) That refinement can be postponed until profiling shows it’s needed. </details> --- ### 7) Update app dispatch: add `--app=all` and make it default Edits in `app_dispatch.go`: - Add `internal/app/allapp` import. - Update `supportedAppModes` and help text to include: `all, controller, aggregated-apiserver, mcp-http`. - Make `--app` default `"all"`. - Add `case "all": return allapp.Run(setupSignalHandler())`. - Remove the `case "": ... required` branch (`--app` is no longer required). - Keep the existing aggregated-apiserver flags (`--coder-url`, `--coder-session-token`, `--coder-namespace`, `--coder-request-timeout`) and their validation logic for `--app=aggregated-apiserver`. - In `all` mode these should not be required; they can be ignored (or later repurposed as global codersdk tuning knobs). --- ### 8) Deploy manifests: single Deployment/Pod, preserve Services Add a new unified Deployment manifest and migrate existing Services/selectors. 1) Replace the three example Deployments with a single unified Deployment manifest. Recommended approach: - Create `deploy/deployment.yaml` (or rename `deploy/controller-deployment.yaml` → `deploy/deployment.yaml`). - Remove `deploy/apiserver-deployment.yaml` and `deploy/mcp-deployment.yaml` from the “happy path” examples (either delete them or move them under `deploy/legacy/`), to avoid users accidentally deploying 3 pods that each run *all* components. Unified Deployment details: - single Deployment name (e.g., `coder-k8s`) - one container `ghcr.io/coder/coder-k8s:latest` - **drop the container `args:` entirely** (no `--app`), relying on the new default `--app=all` - expose ports: `8081` (controller probes), `6443` (aggregated apiserver), `8090` (MCP) 2) Update `deploy/apiserver-service.yaml` selector to match the combined deployment label. 3) Update `deploy/mcp-service.yaml` selector to match the combined deployment label. 4) Update `deploy/rbac.yaml`: - Replace 3 ServiceAccounts with 1 (e.g., `coder-k8s`). - Bind: - controller ClusterRole rules - MCP ClusterRole rules - apiserver auth delegation bindings (`system:auth-delegator` + `extension-apiserver-authentication-reader`) This yields a single pod with the union of permissions. 5) Probes: - Initially, point readiness/liveness at the controller probe port `:8081`. - Extend `controllerapp.SetupProbes` (in all mode only, or generally) with additional readiness checks that confirm: - aggregated apiserver is serving (TCP dial `localhost:6443` or HTTP GET `https://localhost:6443/readyz` if available) - MCP server is serving (TCP dial `localhost:8090` or expose an atomic ready flag) (Implementation choice depends on whether we want to introduce a new composite endpoint vs extending the controller’s existing `/readyz`.) 6) Update any other “example deployment” YAMLs that run `coder-k8s` to rely on the default `all` mode. - `config/e2e/deployment.yaml`: remove `args: ["--app=controller"]` so the container starts in default `all` mode. - (Optional) Add container ports `6443` and `8090` so the manifest matches the unified runtime. --- ### 9) Tests Update and add tests to prevent regressions. 1) `main_test.go` - Replace `TestRunRejectsEmptyMode` with `TestRunDefaultsToAllMode`. - Add `TestRunDispatchesAllMode`. - Keep `TestRunRejectsUnknownMode`. 2) New tests for cache sharing (recommended) Create `internal/app/allapp/allapp_test.go` with dependency injection to avoid requiring a real Kubernetes cluster: - In `allapp`, define package-level vars for constructors used inside `Run`, e.g.: - `newManager` (wraps `controllerapp.NewManager`) - `newClientset` (wraps `kubernetes.NewForConfig`) - `runAggregatedAPIServer` (wraps `apiserverapp.RunWithOptions`) - `runMCPHTTPWithClients` (wraps `mcpapp.RunHTTPWithClients`) In tests, stub these to: - return a fake manager with: - deterministic `GetClient()` pointer - fake `GetCache().WaitForCacheSync` returning true - capture the `k8sClient` passed into `runMCPHTTPWithClients` - assert pointer equality with `mgr.GetClient()` This directly enforces the “shared cache client” requirement. 3) New tests for dynamic discovery + operator-access skipping (recommended) - `internal/aggregated/coder/*_test.go`: - provider skips `spec.operatorAccess.disabled=true` **without** reading Secrets or calling coderd - provider skips `status.operatorAccessReady=false` - provider errors clearly when multiple eligible control planes exist but the request omits the control-plane name segment - `internal/aggregated/storage/*_test.go`: - LIST merges results across multiple eligible control planes in the same namespace and prefixes names correctly - LIST returns empty when all control planes are disabled/not-ready --- ### 10) Validation (when implementing) Run: - `make test` - `make build` - `make lint` - `make verify-vendor` If deploy manifests are considered part of release artifacts, also validate: - `go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.10` (only if workflow edits happen) --- ## Rollout / migration notes - Existing deployments that explicitly set `--app=controller` / `--app=aggregated-apiserver` / `--app=mcp-http` remain valid. - The new unified deployment can omit `--app` and will run all components. - Because a single pod can only have one ServiceAccount, unified deployment requires new combined RBAC. --- ## Suggested execution strategy (single agent vs. multiple agents) **Recommendation: implement with a single agent/engineer** if possible. Rationale: - The change is cross-cutting (dispatch + controller refactor + MCP injection + new `allapp` wiring + RBAC/manifests) and benefits from tight iteration with end-to-end validation (`make test/build/lint`). - Multiple agents working in parallel will likely collide on the same core files (e.g. `controllerapp/controllerapp.go`, `app_dispatch.go`, deployment YAMLs), creating merge churn. If you *do* want to parallelize, the safest split is along file ownership boundaries, with one “integration agent” responsible for final wiring + validation: 1. **Agent A (runtime wiring / shared cache)** - `internal/app/allapp/*` - `internal/app/sharedscheme/*` - `internal/app/controllerapp/*` refactor (`NewManager`, `SetupControllers`, `SetupProbes`) 2. **Agent B (MCP refactor)** - `internal/app/mcpapp/http.go` (`RunHTTPWithClients`), keep standalone mode working 3. **Agent C (aggregated API server dynamic provider + multi-instance support)** - `internal/app/apiserverapp/*` (add `Options.ClientProvider` override; keep static `--coder-*` mode working) - `internal/aggregated/coder/*` (control-plane-backed provider, naming helpers) - `internal/aggregated/storage/*` + `internal/aggregated/convert/*` (control-plane-prefixed names; LIST merge across instances; enforce disabled skip) - `api/aggregation/v1alpha1/types.go` + generated docs if naming comments change 4. **Agent D (dispatch + unit tests + docs)** - `app_dispatch.go`, `main_test.go` - Update docs/README snippets that mention `--app=...` (if desired in this change) 5. **Agent E (manifests/RBAC)** - `deploy/*.yaml`, `config/e2e/deployment.yaml` - Create unified Deployment manifest + update Services + unify ServiceAccount/RBAC 6. **Integration agent (final pass)** - Rebases/merges work from A–E - Ensures the unified deployment examples are coherent and don’t accidentally create 3 pods each running `all` - Runs final validation and fixes any test/lint fallout </details> --- _Generated with `mux` • Model: `anthropic:claude-opus-4-6` • Thinking: `xhigh` • Cost: `$1.51`_ <!-- mux-attribution: model=anthropic:claude-opus-4-6 thinking=xhigh costs=1.51 -->
1 parent e142bce commit d63b3db

36 files changed

Lines changed: 3866 additions & 171 deletions

app_dispatch.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,18 @@ import (
1010

1111
ctrl "sigs.k8s.io/controller-runtime"
1212

13+
"github.com/coder/coder-k8s/internal/app/allapp"
1314
"github.com/coder/coder-k8s/internal/app/apiserverapp"
1415
"github.com/coder/coder-k8s/internal/app/controllerapp"
1516
"github.com/coder/coder-k8s/internal/app/mcpapp"
1617
)
1718

18-
const supportedAppModes = "controller, aggregated-apiserver, mcp-http"
19+
const supportedAppModes = "all, controller, aggregated-apiserver, mcp-http"
1920

2021
var (
21-
runControllerApp = controllerapp.Run
22-
runAggregatedAPIServerApp = func(ctx context.Context, opts apiserverapp.Options) error {
22+
runAllApp func(context.Context, time.Duration) error = allapp.Run
23+
runControllerApp = controllerapp.Run
24+
runAggregatedAPIServerApp = func(ctx context.Context, opts apiserverapp.Options) error {
2325
return apiserverapp.RunWithOptions(ctx, opts)
2426
}
2527
runMCPHTTPApp = mcpapp.RunHTTP
@@ -35,7 +37,7 @@ func run(args []string) error {
3537
coderNamespace string
3638
coderRequestTimeout time.Duration
3739
)
38-
fs.StringVar(&appMode, "app", "", "Application mode (controller, aggregated-apiserver, mcp-http)")
40+
fs.StringVar(&appMode, "app", "all", "Application mode (all, controller, aggregated-apiserver, mcp-http)")
3941
fs.StringVar(
4042
&coderSessionToken,
4143
"coder-session-token",
@@ -82,6 +84,8 @@ func run(args []string) error {
8284
}
8385

8486
switch appMode {
87+
case "all":
88+
return runAllApp(setupSignalHandler(), coderRequestTimeout)
8589
case "controller":
8690
return runControllerApp(setupSignalHandler())
8791
case "aggregated-apiserver":
@@ -94,8 +98,6 @@ func run(args []string) error {
9498
return runAggregatedAPIServerApp(setupSignalHandler(), opts)
9599
case "mcp-http":
96100
return runMCPHTTPApp(setupSignalHandler())
97-
case "":
98-
return fmt.Errorf("assertion failed: --app flag is required; must be one of: %s", supportedAppModes)
99101
default:
100102
return fmt.Errorf("assertion failed: unsupported --app value %q; must be one of: %s", appMode, supportedAppModes)
101103
}

config/e2e/deployment.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ spec:
1919
containers:
2020
- name: manager
2121
image: ghcr.io/coder/coder-k8s:e2e
22+
# E2E tests validate controller mode only; all mode requires
23+
# additional infrastructure (APIService, TLS) not provisioned here.
2224
args: ["--app=controller"]
2325
imagePullPolicy: Never
2426
ports:

deploy/apiserver-deployment.yaml

Lines changed: 0 additions & 23 deletions
This file was deleted.

deploy/apiserver-service.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ metadata:
55
namespace: coder-system
66
spec:
77
selector:
8-
app: coder-k8s-apiserver
8+
app: coder-k8s
99
ports:
1010
- name: https
1111
port: 443
Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,29 @@
11
apiVersion: apps/v1
22
kind: Deployment
33
metadata:
4-
name: coder-k8s-controller
4+
name: coder-k8s
55
namespace: coder-system
66
spec:
77
replicas: 1
88
selector:
99
matchLabels:
10-
app: coder-k8s-controller
10+
app: coder-k8s
1111
template:
1212
metadata:
1313
labels:
14-
app: coder-k8s-controller
14+
app: coder-k8s
1515
spec:
16-
serviceAccountName: coder-k8s-controller
16+
serviceAccountName: coder-k8s
1717
containers:
18-
- name: controller
18+
- name: coder-k8s
1919
image: ghcr.io/coder/coder-k8s:latest
20-
args: ["--app=controller"]
2120
ports:
2221
- containerPort: 8081
2322
name: health
23+
- containerPort: 6443
24+
name: https
25+
- containerPort: 8090
26+
name: mcp
2427
livenessProbe:
2528
httpGet:
2629
path: /healthz

deploy/mcp-deployment.yaml

Lines changed: 0 additions & 31 deletions
This file was deleted.

deploy/mcp-service.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ metadata:
55
namespace: coder-system
66
spec:
77
selector:
8-
app: coder-k8s-mcp
8+
app: coder-k8s
99
ports:
1010
- name: mcp
1111
port: 8090

deploy/rbac.yaml

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,15 @@
11
apiVersion: v1
22
kind: ServiceAccount
33
metadata:
4-
name: coder-k8s-controller
4+
name: coder-k8s
55
namespace: coder-system
66
---
7-
apiVersion: v1
8-
kind: ServiceAccount
9-
metadata:
10-
name: coder-k8s-apiserver
11-
namespace: coder-system
12-
---
13-
# ClusterRole for the controller to manage CoderControlPlane resources.
14-
# Permissions match the kubebuilder RBAC markers on CoderControlPlaneReconciler.
157
apiVersion: rbac.authorization.k8s.io/v1
168
kind: ClusterRole
179
metadata:
18-
name: coder-k8s-controller
10+
name: coder-k8s
1911
rules:
12+
# Controller: manage CoderControlPlane resources
2013
- apiGroups: ["coder.com"]
2114
resources: ["codercontrolplanes"]
2215
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
@@ -26,81 +19,90 @@ rules:
2619
- apiGroups: ["coder.com"]
2720
resources: ["codercontrolplanes/finalizers"]
2821
verbs: ["update"]
22+
# Controller: manage WorkspaceProxy resources
23+
- apiGroups: ["coder.com"]
24+
resources: ["workspaceproxies"]
25+
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
26+
- apiGroups: ["coder.com"]
27+
resources: ["workspaceproxies/status"]
28+
verbs: ["get", "update", "patch"]
29+
- apiGroups: ["coder.com"]
30+
resources: ["workspaceproxies/finalizers"]
31+
verbs: ["update"]
32+
# Controller: manage CoderProvisioner resources
33+
- apiGroups: ["coder.com"]
34+
resources: ["coderprovisioners"]
35+
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
36+
- apiGroups: ["coder.com"]
37+
resources: ["coderprovisioners/status"]
38+
verbs: ["get", "update", "patch"]
39+
- apiGroups: ["coder.com"]
40+
resources: ["coderprovisioners/finalizers"]
41+
verbs: ["update"]
42+
# Controller: leader election
43+
- apiGroups: ["coordination.k8s.io"]
44+
resources: ["leases"]
45+
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
46+
# Controller + MCP: events
47+
- apiGroups: [""]
48+
resources: ["events"]
49+
verbs: ["get", "list", "watch", "create", "patch"]
50+
# Controller: manage deployments and services for control planes
51+
- apiGroups: ["apps"]
52+
resources: ["deployments"]
53+
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
54+
- apiGroups: [""]
55+
resources: ["services"]
56+
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
57+
# Dynamic provider: read operator token secrets
58+
- apiGroups: [""]
59+
resources: ["secrets"]
60+
verbs: ["get"]
61+
# MCP: read aggregated API resources
62+
- apiGroups: ["aggregation.coder.com"]
63+
resources: ["coderworkspaces", "codertemplates"]
64+
verbs: ["get", "list", "watch", "update", "patch"]
65+
# MCP: read pods, logs, namespaces
66+
- apiGroups: [""]
67+
resources: ["pods", "pods/log", "namespaces"]
68+
verbs: ["get", "list", "watch"]
2969
---
3070
apiVersion: rbac.authorization.k8s.io/v1
3171
kind: ClusterRoleBinding
3272
metadata:
33-
name: coder-k8s-controller
73+
name: coder-k8s
3474
roleRef:
3575
apiGroup: rbac.authorization.k8s.io
3676
kind: ClusterRole
37-
name: coder-k8s-controller
77+
name: coder-k8s
3878
subjects:
3979
- kind: ServiceAccount
40-
name: coder-k8s-controller
80+
name: coder-k8s
4181
namespace: coder-system
4282
---
4383
apiVersion: rbac.authorization.k8s.io/v1
4484
kind: ClusterRoleBinding
4585
metadata:
46-
name: coder-k8s-apiserver-auth-delegator
86+
name: coder-k8s-auth-delegator
4787
roleRef:
4888
apiGroup: rbac.authorization.k8s.io
4989
kind: ClusterRole
5090
name: system:auth-delegator
5191
subjects:
5292
- kind: ServiceAccount
53-
name: coder-k8s-apiserver
93+
name: coder-k8s
5494
namespace: coder-system
5595
---
5696
apiVersion: rbac.authorization.k8s.io/v1
5797
kind: RoleBinding
5898
metadata:
59-
name: coder-k8s-apiserver-authentication-reader
99+
name: coder-k8s-authentication-reader
60100
namespace: kube-system
61101
roleRef:
62102
apiGroup: rbac.authorization.k8s.io
63103
kind: Role
64104
name: extension-apiserver-authentication-reader
65105
subjects:
66106
- kind: ServiceAccount
67-
name: coder-k8s-apiserver
68-
namespace: coder-system
69-
---
70-
apiVersion: v1
71-
kind: ServiceAccount
72-
metadata:
73-
name: coder-k8s-mcp
74-
namespace: coder-system
75-
---
76-
# ClusterRole for the MCP server to read operator resources.
77-
apiVersion: rbac.authorization.k8s.io/v1
78-
kind: ClusterRole
79-
metadata:
80-
name: coder-k8s-mcp
81-
rules:
82-
- apiGroups: ["coder.com"]
83-
resources: ["codercontrolplanes", "codercontrolplanes/status"]
84-
verbs: ["get", "list", "watch"]
85-
- apiGroups: ["aggregation.coder.com"]
86-
resources: ["coderworkspaces", "codertemplates"]
87-
verbs: ["get", "list", "watch", "update", "patch"]
88-
- apiGroups: ["apps"]
89-
resources: ["deployments"]
90-
verbs: ["get", "list", "watch"]
91-
- apiGroups: [""]
92-
resources: ["services", "pods", "pods/log", "events", "namespaces"]
93-
verbs: ["get", "list", "watch"]
94-
---
95-
apiVersion: rbac.authorization.k8s.io/v1
96-
kind: ClusterRoleBinding
97-
metadata:
98-
name: coder-k8s-mcp
99-
roleRef:
100-
apiGroup: rbac.authorization.k8s.io
101-
kind: ClusterRole
102-
name: coder-k8s-mcp
103-
subjects:
104-
- kind: ServiceAccount
105-
name: coder-k8s-mcp
107+
name: coder-k8s
106108
namespace: coder-system

docs/how-to/deploy-aggregated-apiserver.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ kubectl create namespace coder-system
1616

1717
## 2. Apply RBAC
1818

19-
The RBAC manifest includes service accounts for both the controller and the aggregated API server.
19+
The RBAC manifest creates the unified `coder-k8s` ServiceAccount used by all app modes.
2020

2121
```bash
2222
kubectl apply -f deploy/rbac.yaml
@@ -26,9 +26,13 @@ kubectl apply -f deploy/rbac.yaml
2626

2727
```bash
2828
kubectl apply -f deploy/apiserver-service.yaml
29-
kubectl apply -f deploy/apiserver-deployment.yaml
29+
kubectl apply -f deploy/deployment.yaml
3030
```
3131

32+
`deploy/deployment.yaml` defaults to `--app=all`, which runs the controller, aggregated API server, and MCP server in a single pod.
33+
34+
For split deployments, you can still run individual components by setting `--app=controller`, `--app=aggregated-apiserver`, or `--app=mcp-http` in the Deployment args.
35+
3236
## 4. Register the APIService
3337

3438
```bash
@@ -40,7 +44,7 @@ kubectl apply -f deploy/apiserver-apiservice.yaml
4044
Wait for the deployment:
4145

4246
```bash
43-
kubectl rollout status deployment/coder-k8s-apiserver -n coder-system
47+
kubectl rollout status deployment/coder-k8s -n coder-system
4448
```
4549

4650
Check the APIService:

docs/how-to/deploy-controller.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,23 @@ kubectl apply -f config/crd/bases/
2424
kubectl apply -f deploy/rbac.yaml
2525
```
2626

27-
## 4. Deploy the controller
27+
## 4. Deploy `coder-k8s`
2828

2929
```bash
30-
kubectl apply -f deploy/controller-deployment.yaml
30+
kubectl apply -f deploy/deployment.yaml
3131
```
3232

33+
`deploy/deployment.yaml` defaults to `--app=all`, which runs the controller, aggregated API server, and MCP server in a single pod.
34+
35+
For split deployments, you can still run individual components by setting `--app=controller`, `--app=aggregated-apiserver`, or `--app=mcp-http` in the Deployment args.
36+
3337
## 5. Verify
3438

3539
```bash
36-
kubectl rollout status deployment/coder-k8s-controller -n coder-system
40+
kubectl rollout status deployment/coder-k8s -n coder-system
3741
kubectl get pods -n coder-system
3842
```
3943

4044
## Customizing the image
4145

42-
By default, `deploy/controller-deployment.yaml` uses `ghcr.io/coder/coder-k8s:latest`. For a different image tag, edit the deployment manifest before applying it.
46+
By default, `deploy/deployment.yaml` uses `ghcr.io/coder/coder-k8s:latest`. For a different image tag, edit the deployment manifest before applying it.

0 commit comments

Comments
 (0)