Skip to content

Add V2 namespace and physical container resources - #223

Open
David Negstad (danegsta) wants to merge 24 commits into
mainfrom
danegsta-v2-namespace-support
Open

Add V2 namespace and physical container resources#223
David Negstad (danegsta) wants to merge 24 commits into
mainfrom
danegsta-v2-namespace-support

Conversation

@danegsta

Copy link
Copy Markdown
Member

Why

This starts the V2 resource model so new DCP resources can require namespaces, support namespace-scoped cleanup, and avoid repeating direct runtime lifecycle logic across V1 controllers.

V1 resources remain cluster-scoped and continue to work without namespaces. The new V2 model gives us a place to build cleaner physical runtime primitives first, then layer higher-level logical resources on top.

What changed

  • Adds the V2 API group with Namespace, PhysicalContainerImage, and PhysicalContainer resources.
  • Adds V2 namespace cleanup and side-effect blocking for missing, terminating, or inactive namespaces.
  • Adds physical container/image controllers with queued long-running work, in-memory progress data, Ready conditions, runtime event watches, referenced image watches, and port mapping status.
  • Moves shared container API fragments into pkg/commonapi and decouples container orchestrator packages from V1/V2 API packages.
  • Adds shared cleanup ordering metadata and updates shutdown cleanup paths for V2 resources.
  • Adds docs/v2-resource-plan.md with V2 guidelines and the follow-up roadmap.

Notes for reviewers

This is intentionally the initial V2 foundation rather than a complete V2 application model. Physical resources represent concrete runtime objects; future logical resources will own persistence/reuse policy and delegate runtime work to these physical primitives.

The V1 JSON API remains compatible. Shared fields added to common fragments are optional, and V1 resources remain cluster-scoped.

Validation

  • make generate
  • make test-prereqs
  • Focused Go tests for changed API/controller/helper packages
  • Targeted V2 integration tests
  • Manual Docker smoke test for V2 namespace cleanup

Introduce the initial V2 namespace model, physical container/image APIs and controllers, shared common API fragments, namespace cleanup support, and V2 roadmap documentation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Introduces the initial namespaced V2 resource model and physical container primitives while retaining V1 compatibility.

Changes:

  • Adds V2 namespace, image, and container APIs and controllers.
  • Moves reusable container definitions into commonapi.
  • Extends cleanup, orchestration, generation, and integration coverage.

The namespace-finalization race, V1 lifecycle-key regression, and missing reference/protocol validation require changes.

Reviewed changes

Copilot reviewed 98 out of 103 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Makefile Generates API artifacts for V1 and V2.
api/v1/common_types.go Re-exports shared API definitions.
api/v1/container_exec_types.go Uses shared environment types.
api/v1/container_network_connection_types.go Uses shared networking types.
api/v1/container_types.go Migrates container fragments to commonapi.
api/v1/container_types_test.go Updates V1 container tests.
api/v1/executable_replica_set_types.go Uses shared environment definitions.
api/v1/executable_types.go Uses shared environment definitions.
api/v1/executable_types_test.go Updates shared-type assertions.
api/v1/groupversion_info.go Shares resource-creation state.
api/v1/service_types.go Uses shared protocol definitions.
api/v1/zz_generated.deepcopy.go Regenerates V1 deep-copy code.
api/v1/zz_generated.model_name.go Regenerates V1 model names.
api/v2/common_types.go Adds V2 metadata helpers and validation.
api/v2/common_types_test.go Tests V2 metadata validation.
api/v2/doc.go Declares the V2 API package.
api/v2/groupversion_info.go Registers the V2 API group.
api/v2/namespace_types.go Defines the V2 Namespace resource.
api/v2/namespace_types_test.go Tests Namespace validation.
api/v2/physical_container_image_types.go Defines physical image APIs.
api/v2/physical_container_image_types_test.go Tests physical image validation.
api/v2/physical_container_types.go Defines physical container APIs.
api/v2/physical_container_types_test.go Tests physical container validation.
api/v2/zz_generated.deepcopy.go Adds V2 deep-copy implementations.
api/v2/zz_generated.model_name.go Adds V2 model names.
controllers/container_common.go Adapts shared container definitions.
controllers/container_controller.go Converts V1 specs to runtime options.
controllers/container_exec_controller.go Migrates shared environment types.
controllers/container_network_tunnel_proxy_controller.go Updates shared runtime types.
controllers/container_watcher.go Generalizes container event handling.
controllers/controller_common.go Adds namespace and status helpers.
controllers/executable_controller.go Migrates shared environment types.
controllers/executable_run_info.go Uses common controller setters.
controllers/executable_start_result.go Uses common controller setters.
controllers/namespace_controller.go Implements namespace lifecycle cleanup.
controllers/object_state_map.go Adds deferred operation tracking.
controllers/object_state_map_test.go Tests deferred state behavior.
controllers/physical_container_controller.go Implements physical container reconciliation.
controllers/physical_container_data.go Stores physical container progress.
controllers/physical_container_image_controller.go Implements image reconciliation.
controllers/physical_container_image_data.go Stores image operation progress.
controllers/running_container_data.go Updates shared runtime state.
controllers/service_controller_metrics.go Uses shared protocol types.
controllers/volume_controller.go Adopts common setter conventions.
docs/v2-resource-plan.md Documents the V2 architecture roadmap.
internal/apiserver/admin_http_handler_test.go Updates shutdown tests.
internal/apiserver/apiserver.go Registers V2 API resources.
internal/appmgmt/shutdown.go Includes V2 resources in shutdown.
internal/containers/container_orchestrator.go Decouples orchestrator options from APIs.
internal/containers/containers_common.go Adds API-neutral container types.
internal/containers/image_layers.go Uses shared build definitions.
internal/containers/image_layers_test.go Updates image-layer tests.
internal/containers/image_orchestrator.go Adds image operation options.
internal/containers/test_container_orchestrator_client.go Updates orchestrator test client.
internal/dcp/commands/cleanup_test.go Covers revised cleanup metadata.
internal/dcpclient/client.go Adds V2 namespace workload resolution.
internal/dcpclient/client_test.go Tests V2 workload resolution.
internal/dcpclient/scheme.go Registers the V2 scheme.
internal/dcpctrl/commands/run_controllers.go Starts the V2 controllers.
internal/dcpproc/dcpproc_test.go Updates shared-type usage.
internal/dcptun/image_builder.go Uses API-neutral image options.
internal/dcptun/tunnel_test.go Updates shared-type usage.
internal/docker/cli_orchestrator.go Implements API-neutral Docker options.
internal/docker/cli_orchestrator_test.go Tests revised Docker options.
internal/exerunners/ide_requests_responses.go Uses shared environment types.
internal/exerunners/ide_requests_responses_test.go Updates IDE protocol tests.
internal/exerunners/process_executable_runner.go Uses shared environment types.
internal/networking/networking.go Uses shared protocol definitions.
internal/networking/networking_test.go Updates networking tests.
internal/podman/cli_orchestrator.go Implements API-neutral Podman options.
internal/podman/cli_orchestrator_test.go Tests revised Podman options.
internal/proxy/netproxy.go Uses shared protocol definitions.
internal/proxy/proxy_api.go Uses shared protocol definitions.
internal/proxy/proxy_test.go Updates proxy tests.
internal/resourcecleanup/resources.go Adds ordered V2 cleanup metadata.
internal/resourcecleanup/resources_test.go Tests cleanup ordering.
internal/templating/value_templates.go Uses shared container fragments.
internal/testutil/ctrlutil/test_container_orchestrator.go Supports V2 controller testing.
internal/testutil/ctrlutil/test_proxy.go Uses shared protocols.
pkg/commonapi/annotations.go Adds common annotation definitions.
pkg/commonapi/annotations_test.go Tests common annotations.
pkg/commonapi/container_deepcopy.go Adds shared deep-copy helpers.
pkg/commonapi/container_types.go Defines shared container API fragments.
pkg/commonapi/resource_creation.go Shares shutdown creation state.
pkg/commonapi/types.go Adds common workload identifiers.
pkg/generated/openapi/zz_generated.openapi.go Regenerates V1/V2 OpenAPI schemas.
pkg/kubeconfig/kubeconfig.go Includes V2 API configuration.
test/integration/advanced_test_env.go Registers V2 test resources.
test/integration/container_controller_test.go Migrates shared container types.
test/integration/container_network_connection_test.go Migrates network types.
test/integration/container_network_tunnel_proxy_test.go Migrates tunnel test types.
test/integration/controllers_common_test.go Adds V2 controller test helpers.
test/integration/endpoints_test.go Migrates shared protocol types.
test/integration/executable_controller_test.go Migrates environment types.
test/integration/executable_replica_set_controller_test.go Migrates environment types.
test/integration/resource_harvesting_test.go Includes V2 cleanup behavior.
test/integration/service_controller_test.go Migrates shared service types.
test/integration/service_controller_test_not_darwin.go Migrates platform service tests.
test/integration/standard_test_env.go Starts V2 reconcilers in tests.
test/integration/test_env_common.go Adds V2 controller selection flags.
test/integration/v2_namespace_controller_test.go Tests namespace activation and cleanup.
test/integration/v2_physical_container_controller_test.go Tests physical container lifecycle behavior.
test/integration/v2_physical_container_image_controller_test.go Tests image pull and build behavior.
Files not reviewed (4)
  • api/v1/zz_generated.deepcopy.go: Generated file
  • api/v1/zz_generated.model_name.go: Generated file
  • api/v2/zz_generated.deepcopy.go: Generated file
  • api/v2/zz_generated.model_name.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/commonapi/container_types.go Outdated
Comment thread controllers/namespace_controller.go
Comment thread api/v2/physical_container_types.go Outdated
Comment thread pkg/commonapi/container_types.go Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Copilot Code Review

Independent review of the code before reading the PR description, then reconciled against it. This is a review without approval — 19 findings below (5 ❌ error, 9 ⚠️ warning, 5 💡 suggestion), posted as inline comments.

I deliberately skipped the 4 findings already raised by the earlier automated review (containerPortEnd lifecycle key, namespace-finalization race, imageRef name validation, port protocol validation) to avoid duplicate feedback. Note that finding on api/v1/container_types.go:499 is a second, independent cause of the lifecycle-key regression that the existing containerPortEnd fix would not address.

Holistic assessment: ⚠️ Needs Changes

Motivation is sound: a namespaced V2 model with physical primitives gives DCP one place to implement runtime lifecycle instead of duplicating orchestrator calls across V1 controllers, and docs/v2-resource-plan.md lays out a credible roadmap.

Approach bundles two largely independent changes: (1) a mechanical move of shared container API fragments into pkg/commonapi plus a rework of the orchestrator CreateContainerOptions boundary, touching ~70 existing files; and (2) the genuinely new V2 API group + three controllers. The new controllers follow existing repo patterns closely (ReconcilerBase, ObjectStateMap, resiliency.WorkQueue, ContainerWatcher), which is good.

I'd suggest splitting this PR. The api/v1 -> pkg/commonapi move is self-contained and independently reviewable/revertable; separating it would make the V1 regression risk far easier to reason about, and would let the V2 foundation be evaluated on its own.

The blocking concerns are concentrated in the V1 blast radius and the steady-state cost of the new controllers, not in the V2 design itself:

  • The commonapi move silently changes the V1 persistent-container lifecycle key (two independent causes), causing existing persistent containers to be removed and recreated on upgrade.
  • Both new controllers requeue unconditionally every ~5-7s forever, each cycle shelling out to docker inspect.
  • V2-created containers carry no creator-process labels, so the harvester can never reclaim them after a crash — and removeOnDeletion defaults to false, so they also survive a clean shutdown.

Where a human reviewer should focus / where I'm less confident:

  • The lifecycle-key finding is the one I'd most want a second opinion on. I verified empirically that gob embeds struct name + field list in the hashed byte stream, and that container_controller.go:445 removes+recreates on mismatch — but whether the resulting churn is acceptable in practice is a product call.
  • The api/v1 source-break + OpenAPI $ref rename: I don't know whether anything outside this repo consumes api/v1 as a Go package or generates clients from the DCP OpenAPI document. Someone who owns the Aspire/DCP client contract should confirm.
  • The reconcile-frequency cost depends on typical container counts; I did not measure it.

Validation performed locally on this branch

Check Result
go build ./api/... ./controllers/... ./pkg/... ./internal/resourcecleanup/...
make lint ✅ 0 issues
make generate ✅ no drift in generated files
go test ./api/... ./pkg/commonapi/... ./controllers/... ./internal/dcpclient/... ./internal/resourcecleanup/...
go test ./test/integration/... (full suite)

No go.mod changes, so no new external dependencies.

Highest-risk files for human attention

File Risk categories
api/v1/container_types.go, api/v1/common_types.go, api/v1/groupversion_info.go Public API
api/v2/*_types.go, pkg/commonapi/container_types.go Public API
pkg/generated/openapi/zz_generated.openapi.go Public API, Network Communications
controllers/physical_container{,_image}_controller.go Controller Code, Concurrency and Resiliency, Process Manipulation
controllers/namespace_controller.go, controllers/object_state_map.go Controller Code, Concurrency and Resiliency
controllers/container_controller.go, container_common.go, running_container_data.go Controller Code, Process Manipulation
internal/containers/*, internal/docker/cli_orchestrator.go, internal/podman/cli_orchestrator.go Process Manipulation, Program Invocation
internal/dcpctrl/commands/run_controllers.go, internal/appmgmt/shutdown.go, internal/resourcecleanup/resources.go Program Invocation, Controller Code
internal/apiserver/apiserver.go, internal/dcpclient/* Public API, Network Communications
pkg/kubeconfig/kubeconfig.go Security

Comment thread api/v1/container_types.go Outdated
Comment thread controllers/physical_container_controller.go Outdated
Comment thread controllers/physical_container_image_controller.go
Comment thread controllers/physical_container_controller.go Outdated
Comment thread api/v2/physical_container_types.go Outdated
Comment thread pkg/commonapi/container_types.go Outdated
Comment thread controllers/namespace_controller.go Outdated
Comment thread controllers/physical_container_controller.go Outdated
Comment thread controllers/physical_container_controller.go Outdated
Comment thread controllers/physical_container_controller.go
Comment thread controllers/controller_common.go
David Negstad (danegsta) and others added 22 commits July 29, 2026 12:05
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Invert PhysicalContainer removeOnDeletion into preserveOnDeletion so the
default behavior is to remove the runtime container when the resource is
deleted, with opt-in preservation.

Also fold in the remaining review fixes:
- Preserve legacy V1 gob encodings when computing container lifecycle keys
  so existing persistent containers are not recreated on upgrade, using the
  new port encoding only when a port range is specified.
- Reject unsupported container port protocols during validation.
- Validate PhysicalContainer imageRef as a resource name, and bring V2
  container name and label validation to parity with V1.
- Require an Active, finalized namespace before V2 child resources perform
  side effects.
- Expand V1 port ranges in the integration test helper like production does.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Each API version now owns its own container shapes instead of sharing
versioned fragments through pkg/commonapi. api/v1 is restored to gob
parity with main so existing container lifecycle keys stay valid, and
api/v2 owns its own copies including the containerPortEnd range feature.

pkg/commonapi is reduced to trivially simple cross-cutting types
(EnvVar, Label, PortProtocol), exposed in api/v1 as type aliases that
preserve gob identity. The lifecycle_hash_types.go workaround is removed
and replaced by a golden lifecycle key regression test that catches type
name, field list, and encoder registration order changes.

Orchestrator-neutral container types live in the internal/containers
orchestrator files that use them, so internal/containers still imports
neither api/v1 nor api/v2 and controllers convert at the boundary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
calculateAnnotationsSize is only used within pkg/commonapi, and
annotationsSizeInfo has no callers at all. Both were unexported in
api/v1 before the move, so promoting them to exported pkg/ API added
public surface without a consumer.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Add DCP creator and persistence labels when the V2 controller creates runtime containers so startup harvesting can remove abandoned non-preserved containers after a DCP crash. Keep adopted runtime containers unstamped and map the harvester persistence label to PreserveOnDeletion.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Add API server validation for V2 PhysicalContainer and PhysicalContainerImage creation so children cannot be admitted after their DCP namespace is missing or terminating. This keeps namespace cleanup from racing finalizer removal.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
The custom V2 resource storage registration dropped tilt's generic
subresource branch, so any future V2 type implementing
ObjectWithGenericSubResource would silently lose those endpoints.

Also make the API-server-only PhysicalContainer test apply the finalizer
and Active phase that the namespace controller normally applies, so it
passes the new namespace lifecycle admission gate.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
The API server admission gate duplicated ensureNamespace in
controllers/controller_common.go, which already rejects children whose
namespace is missing, terminating, unfinalized, or not Active, and does
so before any runtime work happens. The gate only prevented an inert API
object from being created, at the cost of forking tilt's resource
registration path.

Keep the NamespaceFinalizer constant on the API type, which is a useful
cleanup independent of the gate.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Both V2 controllers passed LongDelay to SaveChangesWithDelay, which sets
additionalReconciliationNeeded before the noChange early return, so every
object requeued every ~5-7s for the life of the process. Each cycle shelled
out to the container runtime to inspect a container or image that had not
changed.

Follow the V1 pattern instead and let steady states go quiet:

- Add a MonitoringDelay tier (30s +/- 5s) for resources in a steady state
  where reconciliation only guards against missed runtime events.
- PhysicalContainer reconciles at MonitoringDelay while Running and at
  StandardDelay otherwise. applyInspectedPhysicalContainerStatus requests
  reconciliation only for running and transitional states; exited and dead
  containers go quiet and rely on the container event watch.
- Terminal file copy and start failures no longer request reconciliation,
  which previously spun with no possible progress.
- PhysicalContainerImage reconciles at StandardDelay. Pulls now retry with
  bounded exponential backoff inside the queued operation rather than by
  re-entering the pull path on every reconcile, and a recorded pull or build
  failure is terminal. Transient reconciliation failures stay retryable so a
  namespace read error cannot strand an image.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Picks up the cel-spec repository move from google/cel-spec to cel-expr/cel-spec.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
The pull retry budget was a fixed 30 seconds, which meant a misspelled or
unreachable image took that long to report failure. Shorten the default and
let callers override or disable retries per image.

Add PhysicalContainerImageSpec.PullRetryLimit: unset uses a default of 3
retries, and zero disables retries so the pull fails on the first error.
The backoff is also tightened to 1s initial, 5s maximum interval, and a 15s
overall budget, so the default worst case is about 7 seconds instead of 30.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Do not run create validation from V2 update validation so status updates can proceed during shutdown while spec immutability checks still reject spec changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
PhysicalContainerImage had no finalizer, so deletion removed the object
immediately and the reconcile triggered by the completing pull or build
returned at the NotFound branch before the in-memory state and its queued
deferred op were discarded. Both leaked for the process lifetime, and the
DeletionTimestamp branch was unreachable because tilt-apiserver only starts
a two-phase delete when finalizers are present.

Register a finalizer so deletion is observed, cancel the in-flight pull or
build instead of letting it run to completion against a deleted resource,
and discard the state. The image itself is left in the container runtime,
since it is a shared artifact that outlives the resource describing it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Namespace deletion previously logged cleanup errors and requeued without
surfacing anything in status, so a namespace stuck in Terminating gave no
indication of what it was waiting for or why.

The cleanup handlers now return the number of remaining resources instead of
a bool, and the CleanupComplete condition reports either the failing error or
the resource kinds still pending deletion.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
PortProtocol moved to pkg/commonapi but api/v1 kept TCP/UDP aliases, leaving
two spellings for the same constant with the alias in the majority of uses.

Dropping the aliases lets internal/proxy, internal/networking, and
pkg/kubeconfig stop importing api/v1 entirely; they referenced it only to
name a port protocol. Removing the local alias also lets deepcopy-gen
recognize ContainerPort and ServiceSpec as shallow-copyable.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
The golden container spec pointed Build.Dockerfile at the POSIX-absolute
literal "/src/Dockerfile.golden". GetLifecycleKey hashes the Dockerfile
contents when the file is readable and falls back to hashing the resolved
path otherwise, and that fallback is not portable: filepath.IsAbs requires
a volume name on Windows, so Windows joined the literal against the build
context and hashed "\src\src\Dockerfile.golden" while POSIX hashed the
literal unchanged. The test therefore failed on Windows only, with the same
actual key on every run.

Point the fixture at a real file under t.TempDir() instead. A temp path is
absolute on every platform, so the IsAbs branch is skipped and no filepath
function contributes to the key, and the read succeeds so the key covers the
Dockerfile contents rather than the error fallback. The constant is
recomputed because the fixture inputs deliberately changed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
V2 has no log surface: AdditionalTypes is empty and no V2 resource exposes
a log subresource, so output from a V2 runtime container is unreachable
through the V2 API. The roadmap did not mention logs at all.

Record the gap, summarize the V1 subresource mechanism, and capture the open
question of whether log streams belong to physical resources, logical
resources, or both. Note the evidence that bears on it: the orchestrator
layer defines only stdout and stderr because that is what a runtime
produces, while the V1 API adds startup_stdout, startup_stderr, and system
as DCP-level concepts, and the image pull/build output that V1 surfaces as
startup_* belongs to PhysicalContainerImage in V2.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
The V2 types register unconditionally with the API server and carried no
stability notice, even though the roadmap plans a breaking change to
PhysicalContainer that replaces direct runtime network and volume names with
references to V2 resources. Record that V2 is unstable in the package doc so
the planned churn is not a surprise to callers.

ResolveNamespaceWorkloadID has no production caller: V2 has no state store or
lifecycle key, so nothing consumes a workload ID yet and the namespace
annotation is validated without affecting behavior. Add a TODO marking it as
groundwork for V2 persistent resources rather than leaving it to read as dead
code.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Make the golden lifecycle key test independent of the ambient environment.
GetLifecycleKey hashes os.Getenv(Source) for env type build secrets, so using
NPM_TOKEN as the fixture source made the key depend on whether that variable
happens to be set, failing the test on any machine that sets it. Point the
fixture at a name that is never set and recompute the constant. t.Setenv is not
usable here because the test is parallel.

Add Namespace.ValidateUpdate so annotation validation, including the workload ID
annotation, applies to updates and not just creates. It deliberately omits the
ResourceCreationProhibited gate, which is creation-only and would otherwise block
the namespace controller from writing cleanup status or removing its finalizer
during shutdown.

Also document three behaviors that are load bearing but not evident from the
code: why runtime side effects of namespaced V2 resources are gated in the
controllers rather than at admission, why CleanupAllResources must run before the
controller host shuts down, and that WorkQueue drops queued items once its
lifetime context is done.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Clear the golden fixture's environment-backed build secret explicitly instead
of relying on a variable name that is unlikely to be set. This requires the
test to run serially because testing.T.Setenv cannot be used by a parallel
test.

Also refine the ensureNamespace documentation to distinguish normal runtime
work from deletion cleanup and record the accepted race where an inert child
API record can outlive namespace cleanup until the in-memory server exits.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
@danegsta
David Negstad (danegsta) marked this pull request as ready for review July 31, 2026 21:12
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2bcfab76-f43d-4564-a714-303ce7779074
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants