Skip to content

Commit 26f9704

Browse files
authored
🤖 fix: stabilize CoderProvisioner reconcile and startup (#73)
## Summary This PR fixes CoderProvisioner bring-up and reconcile stability issues discovered in smoke testing. ## Background During an end-to-end smoke test, `CoderProvisioner` failed to reach Ready due to RBAC escalation issues and provisioner pod startup failures. Logs also showed repeated metadata backfill/rotation activity that could repeatedly hit coderd and trigger rate limiting when reconcile retries occurred. ## Implementation - Added manager RBAC markers for delegated provisioner role verbs on: - `pods` (`get/list/watch/create/update/patch/delete`) - `persistentvolumeclaims` (`get/list/watch/create/update/patch/delete`) - Regenerated RBAC output so `config/rbac/role.yaml` includes those verbs. - Stopped coderd metadata-backfill churn when a usable key secret already exists by stamping status baseline fields locally instead of re-calling/rotating keys in that retry path. - Updated provisioner pod launch wiring: - explicit `command: ["coder"]` - args use `coder provisioner start` - removed `CODER_ORGANIZATION` injection for key-auth daemon startup (avoids CLI rejection when `--key` is used). - Updated controller tests accordingly. - Regenerated CRD/docs for the updated API comment text. ## Validation - `make verify-vendor` - `make test` - `make build` - `make lint` - `make test-integration` - `make manifests` - `make docs-reference` - Runtime smoke verification in kind: - `CoderProvisioner` reaches `phase=Ready`, `readyReplicas=1` - provisioner pod logs include `successfully connected to coderd` ## Risks - Low to moderate: changes are isolated to `CoderProvisioner` reconcile/deployment composition and generated RBAC/docs. - Main behavior change is intentionally avoiding metadata-backfill coderd calls when a usable key secret already exists; this reduces churn/rate-limit risk during retry loops. --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.00`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=0.00 -->
1 parent 21eb14d commit 26f9704

6 files changed

Lines changed: 44 additions & 19 deletions

File tree

‎api/v1alpha1/coderprovisioner_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ type CoderProvisionerSpec struct {
6969
Tags map[string]string `json:"tags,omitempty"`
7070
// Image is the container image. Defaults to the control plane image.
7171
Image string `json:"image,omitempty"`
72-
// ExtraArgs are appended after "provisionerd start".
72+
// ExtraArgs are appended after "provisioner start".
7373
ExtraArgs []string `json:"extraArgs,omitempty"`
7474
// ExtraEnv are injected into the provisioner container.
7575
ExtraEnv []corev1.EnvVar `json:"extraEnv,omitempty"`

‎config/crd/bases/coder.com_coderprovisioners.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ spec:
8787
type: object
8888
x-kubernetes-map-type: atomic
8989
extraArgs:
90-
description: ExtraArgs are appended after "provisionerd start".
90+
description: ExtraArgs are appended after "provisioner start".
9191
items:
9292
type: string
9393
type: array

‎config/rbac/role.yaml‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,15 @@ rules:
1818
- ""
1919
resources:
2020
- namespaces
21-
- pods
2221
verbs:
2322
- get
2423
- list
2524
- watch
2625
- apiGroups:
2726
- ""
2827
resources:
29-
- pods/log
30-
verbs:
31-
- get
32-
- apiGroups:
33-
- ""
34-
resources:
28+
- persistentvolumeclaims
29+
- pods
3530
- secrets
3631
- serviceaccounts
3732
- services
@@ -43,6 +38,12 @@ rules:
4338
- patch
4439
- update
4540
- watch
41+
- apiGroups:
42+
- ""
43+
resources:
44+
- pods/log
45+
verbs:
46+
- get
4647
- apiGroups:
4748
- aggregation.coder.com
4849
resources:

‎docs/reference/api/coderprovisioner.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
| `replicas` | integer | Replicas is the desired number of provisioner pods. |
2121
| `tags` | object (keys:string, values:string) | Tags are attached to the provisioner key for job routing. |
2222
| `image` | string | Image is the container image. Defaults to the control plane image. |
23-
| `extraArgs` | string array | ExtraArgs are appended after "provisionerd start". |
23+
| `extraArgs` | string array | ExtraArgs are appended after "provisioner start". |
2424
| `extraEnv` | [EnvVar](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#envvar-v1-core) array | ExtraEnv are injected into the provisioner container. |
2525
| `resources` | [ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#resourcerequirements-v1-core) | Resources for the provisioner container. |
2626
| `imagePullSecrets` | [LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#localobjectreference-v1-core) array | ImagePullSecrets are used by the pod to pull private images. |

‎internal/controller/coderprovisioner_controller.go‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ type CoderProvisionerReconciler struct {
6868
// +kubebuilder:rbac:groups=coder.com,resources=coderprovisioners/status,verbs=get;update;patch
6969
// +kubebuilder:rbac:groups=coder.com,resources=coderprovisioners/finalizers,verbs=update
7070
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
71+
// +kubebuilder:rbac:groups="",resources=pods;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete
7172
// +kubebuilder:rbac:groups="",resources=secrets;serviceaccounts,verbs=get;list;watch;create;update;patch;delete
7273
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete
7374

@@ -441,7 +442,7 @@ func (r *CoderProvisionerReconciler) Reconcile(ctx context.Context, req ctrl.Req
441442
"ProvisionerKeyReady",
442443
"Provisioner key is available in coderd",
443444
)
444-
} else if status.OrganizationName == "" || status.TagsHash == "" || status.ControlPlaneRefName == "" {
445+
} else if status.OrganizationName == "" || status.TagsHash == "" || status.ControlPlaneRefName == "" || status.ControlPlaneURL == "" {
445446
// Secret is usable and no drift detected, but status metadata is empty
446447
// (e.g. upgrade from older version). Call EnsureProvisionerKey to populate
447448
// IDs and key name. If coderd reports an existing key (no plaintext key
@@ -550,6 +551,30 @@ func (r *CoderProvisionerReconciler) Reconcile(ctx context.Context, req ctrl.Req
550551
"Provisioner key secret is available",
551552
)
552553

554+
// Persist key-related status metadata immediately so retries after later
555+
// reconciliation failures (for example RBAC/deployment errors) do not
556+
// repeatedly re-run metadata backfill/rotation against coderd.
557+
keyStatusSnapshot := provisioner.Status.DeepCopy()
558+
if keyStatusSnapshot == nil {
559+
return ctrl.Result{}, fmt.Errorf("assertion failed: key status snapshot must not be nil")
560+
}
561+
provisioner.Status.OrganizationID = organizationID
562+
provisioner.Status.OrganizationName = appliedOrgName
563+
provisioner.Status.ProvisionerKeyID = provisionerKeyID
564+
provisioner.Status.ProvisionerKeyName = provisionerKeyName
565+
provisioner.Status.TagsHash = appliedTagsHash
566+
provisioner.Status.ControlPlaneRefName = appliedControlPlaneRefName
567+
provisioner.Status.ControlPlaneURL = appliedControlPlaneURL
568+
provisioner.Status.SecretRef = &coderv1alpha1.SecretKeySelector{Name: keySecretName, Key: keySecretKey}
569+
if !equality.Semantic.DeepEqual(*keyStatusSnapshot, provisioner.Status) {
570+
if err := r.Status().Update(ctx, provisioner); err != nil {
571+
return ctrl.Result{}, fmt.Errorf("update coderprovisioner key status: %w", err)
572+
}
573+
statusSnapshot = provisioner.Status.DeepCopy()
574+
if statusSnapshot == nil {
575+
return ctrl.Result{}, fmt.Errorf("assertion failed: status snapshot must not be nil after key status update")
576+
}
577+
}
553578
serviceAccountName := provisionerServiceAccountName(provisioner.Name)
554579
if _, err := r.reconcileServiceAccount(ctx, provisioner, serviceAccountName); err != nil {
555580
return ctrl.Result{}, err
@@ -573,7 +598,7 @@ func (r *CoderProvisionerReconciler) Reconcile(ctx context.Context, req ctrl.Req
573598
}
574599

575600
secretRef := &coderv1alpha1.SecretKeySelector{Name: keySecretName, Key: keySecretKey}
576-
deployment, err := r.reconcileDeployment(ctx, provisioner, image, controlPlane.Status.URL, organizationName, secretRef, serviceAccountName, secretChecksum)
601+
deployment, err := r.reconcileDeployment(ctx, provisioner, image, controlPlane.Status.URL, secretRef, serviceAccountName, secretChecksum)
577602
if err != nil {
578603
return ctrl.Result{}, err
579604
}
@@ -946,7 +971,6 @@ func (r *CoderProvisionerReconciler) reconcileDeployment(
946971
provisioner *coderv1alpha1.CoderProvisioner,
947972
image string,
948973
coderURL string,
949-
organizationName string,
950974
secretRef *coderv1alpha1.SecretKeySelector,
951975
serviceAccountName string,
952976
secretChecksum string,
@@ -971,7 +995,7 @@ func (r *CoderProvisionerReconciler) reconcileDeployment(
971995
terminationGracePeriodSeconds = *provisioner.Spec.TerminationGracePeriodSeconds
972996
}
973997

974-
args := []string{"provisionerd", "start"}
998+
args := []string{"provisioner", "start"}
975999
args = append(args, provisioner.Spec.ExtraArgs...)
9761000

9771001
env := []corev1.EnvVar{
@@ -984,9 +1008,6 @@ func (r *CoderProvisionerReconciler) reconcileDeployment(
9841008
}},
9851009
},
9861010
}
987-
if organizationName != "" && organizationName != defaultProvisionerOrganizationName {
988-
env = append(env, corev1.EnvVar{Name: "CODER_ORGANIZATION", Value: organizationName})
989-
}
9901011
env = append(env, provisioner.Spec.ExtraEnv...)
9911012

9921013
deployment.Spec.Replicas = &replicas
@@ -1005,6 +1026,7 @@ func (r *CoderProvisionerReconciler) reconcileDeployment(
10051026
Containers: []corev1.Container{{
10061027
Name: "provisioner",
10071028
Image: image,
1029+
Command: []string{"coder"},
10081030
Args: args,
10091031
Env: env,
10101032
Resources: provisioner.Spec.Resources,

‎internal/controller/coderprovisioner_controller_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -518,14 +518,16 @@ func TestCoderProvisionerReconciler_BasicCreate(t *testing.T) {
518518
container := deployment.Spec.Template.Spec.Containers[0]
519519
require.Equal(t, "provisioner", container.Name)
520520
require.Equal(t, "provisioner-image:test", container.Image)
521-
require.Equal(t, []string{"provisionerd", "start", "--test-mode=true"}, container.Args)
521+
require.Equal(t, []string{"coder"}, container.Command)
522+
require.Equal(t, []string{"provisioner", "start", "--test-mode=true"}, container.Args)
522523

523524
envByName := make(map[string]corev1.EnvVar, len(container.Env))
524525
for _, envVar := range container.Env {
525526
envByName[envVar.Name] = envVar
526527
}
527528
require.Equal(t, "https://coder.example.com", envByName["CODER_URL"].Value)
528-
require.Equal(t, "acme", envByName["CODER_ORGANIZATION"].Value)
529+
_, hasOrganizationEnv := envByName["CODER_ORGANIZATION"]
530+
require.False(t, hasOrganizationEnv)
529531
require.Equal(t, "extra-value", envByName["EXTRA_ENV"].Value)
530532
keyEnv, ok := envByName["CODER_PROVISIONER_DAEMON_KEY"]
531533
require.True(t, ok)

0 commit comments

Comments
 (0)