Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions docs/how-to/deploy-aggregated-apiserver.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,25 @@ kubectl get codertemplates.aggregation.coder.com -A
kubectl logs -n coder-system deploy/coder-k8s
```

## Server-Side Apply (SSA) behavior

`coder-k8s` now includes a compatibility fallback for SSA create-on-update requests
(for example, `kubectl apply --server-side` when the target resource does not exist yet).

- For missing `coderworkspaces` / `codertemplates`, the aggregated API server's `Update`
path can delegate to `Create` when `forceAllowCreate=true`.
- This is intentionally **best-effort**: Coder resources do not currently provide a
first-class metadata store for Kubernetes `metadata.managedFields`, so SSA field-owner
conflict semantics are not durable.

Planned follow-up options (in order of preference):

1. **Preferred:** add first-class metadata support on template/workspace resources in
Coder + `codersdk`, and round-trip Kubernetes-managed metadata there.
2. Persist Kubernetes-only metadata in a shadow Kubernetes resource (ConfigMap/CRD)
managed by the aggregated API server.
3. Keep the compatibility fallback and continue documenting the limitations.

## Template build wait tuning

When updating `CoderTemplate.spec.files`, the aggregated API server now waits for
Expand Down
100 changes: 100 additions & 0 deletions internal/aggregated/storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1477,6 +1477,58 @@ func TestTemplateStorageUpdateAllowsMetadataSpecChanges(t *testing.T) {
}
}

func TestTemplateStorageUpdateForceAllowCreateCreatesWhenMissing(t *testing.T) {
t.Parallel()

server, state := newMockCoderServer(t)
defer server.Close()

templateStorage := NewTemplateStorage(newTestClientProvider(t, server.URL))
ctx := namespacedContext("control-plane")

desiredTemplate := &aggregationv1alpha1.CoderTemplate{
Spec: aggregationv1alpha1.CoderTemplateSpec{
Organization: "acme",
DisplayName: "SSA-created template",
Description: "Created via forceAllowCreate update hack",
Icon: "/icons/ssa-template.png",
Files: map[string]string{
"main.tf": "resource \"null_resource\" \"ssa_create\" {}",
},
},
}

updatedObj, created, err := templateStorage.Update(
ctx,
"acme.ssa-template",
testUpdatedObjectInfo{obj: desiredTemplate},
rest.ValidateAllObjectFunc,
rest.ValidateAllObjectUpdateFunc,
true,
nil,
)
if err != nil {
t.Fatalf("expected create-on-update to succeed for missing template: %v", err)
}
if !created {
t.Fatal("expected update created=true when forceAllowCreate creates a template")
}

createdTemplate, ok := updatedObj.(*aggregationv1alpha1.CoderTemplate)
if !ok {
t.Fatalf("expected *CoderTemplate from update, got %T", updatedObj)
}
if createdTemplate.Name != "acme.ssa-template" {
t.Fatalf("expected created template name acme.ssa-template, got %q", createdTemplate.Name)
}
if createdTemplate.Spec.Organization != "acme" {
t.Fatalf("expected created template organization acme, got %q", createdTemplate.Spec.Organization)
}
if !state.hasTemplate("acme", "ssa-template") {
t.Fatal("expected create-on-update template to be persisted in mock server state")
}
}

func TestTemplateStorageUpdateRejectsMissingResourceVersion(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1846,6 +1898,54 @@ func TestWorkspaceStorageCreateAllowsMatchingTemplateVersionID(t *testing.T) {
}
}

func TestWorkspaceStorageUpdateForceAllowCreateCreatesWhenMissing(t *testing.T) {
t.Parallel()

server, state := newMockCoderServer(t)
defer server.Close()

workspaceStorage := NewWorkspaceStorage(newTestClientProvider(t, server.URL))
ctx := namespacedContext("control-plane")

desiredWorkspace := &aggregationv1alpha1.CoderWorkspace{
Spec: aggregationv1alpha1.CoderWorkspaceSpec{
Organization: "acme",
TemplateName: "starter-template",
Running: false,
},
}

updatedObj, created, err := workspaceStorage.Update(
ctx,
"acme.alice.ssa-workspace",
testUpdatedObjectInfo{obj: desiredWorkspace},
rest.ValidateAllObjectFunc,
rest.ValidateAllObjectUpdateFunc,
true,
nil,
)
if err != nil {
t.Fatalf("expected create-on-update to succeed for missing workspace: %v", err)
}
if !created {
t.Fatal("expected update created=true when forceAllowCreate creates a workspace")
}

createdWorkspace, ok := updatedObj.(*aggregationv1alpha1.CoderWorkspace)
if !ok {
t.Fatalf("expected *CoderWorkspace from update, got %T", updatedObj)
}
if createdWorkspace.Name != "acme.alice.ssa-workspace" {
t.Fatalf("expected created workspace name acme.alice.ssa-workspace, got %q", createdWorkspace.Name)
}
if !state.hasWorkspace("alice", "ssa-workspace") {
t.Fatal("expected create-on-update workspace to be persisted in mock server state")
}
if !containsTransition(state.buildTransitionsSnapshot(), codersdk.WorkspaceTransitionStop) {
t.Fatal("expected create-on-update workspace with spec.running=false to queue stop transition")
}
}

func TestWorkspaceStorageUpdateRejectsNonRunningSpecChanges(t *testing.T) {
t.Parallel()

Expand Down
57 changes: 49 additions & 8 deletions internal/aggregated/storage/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ func (s *TemplateStorage) Update(
ctx context.Context,
name string,
objInfo rest.UpdatedObjectInfo,
_ rest.ValidateObjectFunc,
createValidation rest.ValidateObjectFunc,
updateValidation rest.ValidateObjectUpdateFunc,
forceAllowCreate bool,
_ *metav1.UpdateOptions,
Expand All @@ -435,16 +435,57 @@ func (s *TemplateStorage) Update(
if s.broadcaster == nil {
return nil, false, fmt.Errorf("assertion failed: template broadcaster must not be nil")
}
if forceAllowCreate {
return nil, false, apierrors.NewMethodNotSupported(
aggregationv1alpha1.Resource("codertemplates"),
"create on update",
)
}

currentObj, err := s.Get(ctx, name, nil)
if err != nil {
return nil, false, err
if !forceAllowCreate || !apierrors.IsNotFound(err) {
return nil, false, err
}

// FIXME: This branch intentionally enables best-effort SSA create-on-update
// semantics for codertemplates.
//
// The aggregated API is a stateless proxy to Coder and currently does not
// persist Kubernetes metadata.managedFields (or other Kubernetes-only metadata),
// so SSA field ownership/conflict semantics are not durable.
//
// Future options to remove this hack:
// 1. Preferred: add first-class metadata support on templates/workspaces in
// Coder + codersdk and round-trip Kubernetes-managed metadata there.
// 2. Persist Kubernetes-only metadata in a shadow Kubernetes resource
// (ConfigMap/CRD) managed by this aggregated API server.
// 3. Keep this fallback and continue documenting SSA limitations.
createObj, objInfoErr := objInfo.UpdatedObject(ctx, s.New())
if objInfoErr != nil {
return nil, false, objInfoErr
}
if createObj == nil {
return nil, false, fmt.Errorf("assertion failed: create-on-update template object must not be nil")
}

createTemplate, ok := createObj.(*aggregationv1alpha1.CoderTemplate)
if !ok {
return nil, false, fmt.Errorf("assertion failed: expected *CoderTemplate, got %T", createObj)
}
createTemplate = createTemplate.DeepCopy()
if createTemplate == nil {
return nil, false, fmt.Errorf("assertion failed: create-on-update template deep copy must not be nil")
}
if createTemplate.Name == "" {
createTemplate.Name = name
}
if createTemplate.Name != name {
return nil, false, apierrors.NewBadRequest(
fmt.Sprintf("updated object metadata.name %q must match request name %q", createTemplate.Name, name),
)
}

createdObj, createErr := s.Create(ctx, createTemplate, createValidation, nil)
if createErr != nil {
return nil, false, createErr
}

return createdObj, true, nil
}

currentObjForUpdate := currentObj.DeepCopyObject()
Expand Down
58 changes: 50 additions & 8 deletions internal/aggregated/storage/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ func (s *WorkspaceStorage) Update(
ctx context.Context,
name string,
objInfo rest.UpdatedObjectInfo,
_ rest.ValidateObjectFunc,
createValidation rest.ValidateObjectFunc,
updateValidation rest.ValidateObjectUpdateFunc,
forceAllowCreate bool,
_ *metav1.UpdateOptions,
Expand All @@ -417,12 +417,6 @@ func (s *WorkspaceStorage) Update(
if s.broadcaster == nil {
return nil, false, fmt.Errorf("assertion failed: workspace broadcaster must not be nil")
}
if forceAllowCreate {
return nil, false, apierrors.NewMethodNotSupported(
aggregationv1alpha1.Resource("coderworkspaces"),
"create on update",
)
}

namespace, badNamespaceErr := requiredNamespaceFromRequestContext(ctx)
if badNamespaceErr != nil {
Expand All @@ -441,7 +435,55 @@ func (s *WorkspaceStorage) Update(

currentWorkspace, err := sdk.WorkspaceByOwnerAndName(ctx, userName, workspaceName, codersdk.WorkspaceOptions{})
if err != nil {
return nil, false, coder.MapCoderError(err, aggregationv1alpha1.Resource("coderworkspaces"), name)
mappedErr := coder.MapCoderError(err, aggregationv1alpha1.Resource("coderworkspaces"), name)
if !forceAllowCreate || !apierrors.IsNotFound(mappedErr) {
return nil, false, mappedErr
}

// FIXME: This branch intentionally enables best-effort SSA create-on-update
// semantics for coderworkspaces.
//
// The aggregated API is a stateless proxy to Coder and currently does not
// persist Kubernetes metadata.managedFields (or other Kubernetes-only metadata),
// so SSA field ownership/conflict semantics are not durable.
//
// Future options to remove this hack:
// 1. Preferred: add first-class metadata support on templates/workspaces in
// Coder + codersdk and round-trip Kubernetes-managed metadata there.
// 2. Persist Kubernetes-only metadata in a shadow Kubernetes resource
// (ConfigMap/CRD) managed by this aggregated API server.
// 3. Keep this fallback and continue documenting SSA limitations.
createObj, objInfoErr := objInfo.UpdatedObject(ctx, s.New())
if objInfoErr != nil {
return nil, false, objInfoErr
}
if createObj == nil {
return nil, false, fmt.Errorf("assertion failed: create-on-update workspace object must not be nil")
}

createWorkspace, ok := createObj.(*aggregationv1alpha1.CoderWorkspace)
if !ok {
return nil, false, fmt.Errorf("assertion failed: expected *CoderWorkspace, got %T", createObj)
}
createWorkspace = createWorkspace.DeepCopy()
if createWorkspace == nil {
return nil, false, fmt.Errorf("assertion failed: create-on-update workspace deep copy must not be nil")
}
if createWorkspace.Name == "" {
createWorkspace.Name = name
}
if createWorkspace.Name != name {
return nil, false, apierrors.NewBadRequest(
fmt.Sprintf("updated object metadata.name %q must match request name %q", createWorkspace.Name, name),
)
}

createdObj, createErr := s.Create(ctx, createWorkspace, createValidation, nil)
if createErr != nil {
return nil, false, createErr
}

return createdObj, true, nil
}
if currentWorkspace.OrganizationName != orgName {
return nil, false, apierrors.NewNotFound(aggregationv1alpha1.Resource("coderworkspaces"), name)
Expand Down
Loading