Make imagePullPolicy a template variable instead of hardcoding it #3

Merged

GuillaumeHemmen merged 1 commit from fix/image-pull-policy into master

2026-09-29 15:03:24 +00:00

claude-bot commented

2026-09-29 14:55:19 +00:00

Member

Problem

image_pull_policy was hardcoded to "Always", so every workspace start made kubelet re-resolve the tag against the Forgejo registry even when the full layer set was already cached on the node.

Approach

Rather than flipping the constant, this exposes it as a template variable so whoever pushes the template decides. It validates against Always / IfNotPresent / Never, plus auto.

variable "image_pull_policy" {
    type    = string
    default = "auto"
    validation {
        condition     = contains(["auto", "Always", "IfNotPresent", "Never"], var.image_pull_policy)
        error_message = "image_pull_policy must be one of: auto, Always, IfNotPresent, Never."
    }
}

Why `auto` is the default

The policy and the tag are not independent knobs. Current registry digests:

coder-xfce-vnc-latest   -> sha256:b2c68494...
coder-xfce-vnc-26.37.0  -> sha256:b2c68494...   (same digest today)
coder-latest            -> sha256:50bda30e...
coder-26.37.0           -> sha256:50bda30e...   (same digest today)

latest is a moving alias CI re-points at each release, so it needs Always or a workspace silently pins to whatever layer set its node happened to have cached. Version and build-SHA tags are published once and never reused, so IfNotPresent is always correct for them.

auto derives the policy from image_tag on exactly that rule. Since image_tag still defaults to "latest", auto preserves today's behaviour on the default path and only relaxes it where relaxing is safe. An admin who wants a fixed policy sets one explicitly.

What this actually buys

Resilience, not speed. The registry round-trip measured 75ms on the last workspace start, so this saves no meaningful wall-clock time. The real gain is decoupling: under Always, a workspace on a pinned tag fails with ImagePullBackOff whenever the registry is unreachable, even though the 2.6 GB image is already on the node. Forgejo runs in this same cluster, so a registry outage currently takes pinned workspaces down with it.

Testing

Not applied to the cluster; investigation was read-only. terraform validate was not run (terraform is not installed in the workspace) — worth a terraform validate or a dry-run template push before merging, since this adds a validation block and a locals block rather than just changing a literal. Digests above were read from the Forgejo registry API. Verification is to push the template and confirm a pinned-tag workspace starts with no Pulling event.

Follow-up to #2.

🤖 Generated with Claude Code

https://claude.ai/code/session_015QKnVbbjpAi4TaSmrkNXAE

## Problem `image_pull_policy` was hardcoded to `"Always"`, so every workspace start made kubelet re-resolve the tag against the Forgejo registry even when the full layer set was already cached on the node. ## Approach Rather than flipping the constant, this exposes it as a **template variable** so whoever pushes the template decides. It validates against `Always` / `IfNotPresent` / `Never`, plus `auto`. ```terraform variable "image_pull_policy" { type = string default = "auto" validation { condition = contains(["auto", "Always", "IfNotPresent", "Never"], var.image_pull_policy) error_message = "image_pull_policy must be one of: auto, Always, IfNotPresent, Never." } } ``` ## Why `auto` is the default The policy and the tag are not independent knobs. Current registry digests: ``` coder-xfce-vnc-latest -> sha256:b2c68494... coder-xfce-vnc-26.37.0 -> sha256:b2c68494... (same digest today) coder-latest -> sha256:50bda30e... coder-26.37.0 -> sha256:50bda30e... (same digest today) ``` `latest` is a moving alias CI re-points at each release, so it needs `Always` or a workspace silently pins to whatever layer set its node happened to have cached. Version and build-SHA tags are published once and never reused, so `IfNotPresent` is always correct for them. `auto` derives the policy from `image_tag` on exactly that rule. Since `image_tag` still defaults to `"latest"`, **`auto` preserves today's behaviour on the default path** and only relaxes it where relaxing is safe. An admin who wants a fixed policy sets one explicitly. ## What this actually buys **Resilience, not speed.** The registry round-trip measured 75ms on the last workspace start, so this saves no meaningful wall-clock time. The real gain is decoupling: under `Always`, a workspace on a pinned tag fails with `ImagePullBackOff` whenever the registry is unreachable, *even though the 2.6 GB image is already on the node*. Forgejo runs in this same cluster, so a registry outage currently takes pinned workspaces down with it. ## Testing Not applied to the cluster; investigation was read-only. `terraform validate` was **not** run (terraform is not installed in the workspace) — worth a `terraform validate` or a dry-run template push before merging, since this adds a `validation` block and a `locals` block rather than just changing a literal. Digests above were read from the Forgejo registry API. Verification is to push the template and confirm a pinned-tag workspace starts with no `Pulling` event. Follow-up to #2. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_015QKnVbbjpAi4TaSmrkNXAE

claude-bot added 1 commit

2026-09-29 14:55:19 +00:00

Only force image re-pull when the tag is a moving alias c43d7967d1

image_pull_policy was unconditionally "Always", so every workspace
start hits the Forgejo registry to re-resolve the tag even when the
layer set is already cached on the node.

For a pinned tag that check can never change the outcome. Version and
build-SHA tags are published once per release and never reused, so the
cached image is by definition the right one. "Always" only buys
something for "latest", which CI re-points at each release:

  coder-xfce-vnc-latest   -> sha256:b2c68494...
  coder-xfce-vnc-26.37.0  -> sha256:b2c68494...   (same digest today)
  coder-latest            -> sha256:50bda30e...
  coder-26.37.0           -> sha256:50bda30e...   (same digest today)

Note that var.image_tag still defaults to "latest", so the default
path keeps "Always" and keeps picking up new releases. A blanket
"IfNotPresent" would have pinned those workspaces to whatever layer
set the node happened to have cached.

The win is resilience rather than speed: the measured registry check
is only ~75ms, but under "Always" a workspace on a pinned tag fails
to start with ImagePullBackOff whenever the registry is unreachable,
even though the image is sitting on the node. Forgejo runs in this
same cluster, so that failure mode takes workspaces down with it.

Follow-up to #2.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QKnVbbjpAi4TaSmrkNXAE

claude-bot force-pushed fix/image-pull-policy from c43d7967d1 to 45eae290e7

2026-09-29 15:00:05 +00:00

Compare

claude-bot changed title from ~~Only force image re-pull when the tag is a moving alias~~ to Make imagePullPolicy a template variable instead of hardcoding it

2026-09-29 15:00:23 +00:00

claude-bot commented

2026-09-29 15:00:23 +00:00

Author

Member

Reworked per review: instead of hardcoding IfNotPresent, the policy is now a template variable (image_pull_policy) chosen at coder templates push, validated against auto / Always / IfNotPresent / Never.

image_tag is left as-is, as requested.

The default is auto, which derives the policy from image_tag — Always for the moving latest alias, IfNotPresent for pinned version/SHA tags. Since image_tag still defaults to latest, this preserves current behaviour on the default path.

Branch was force-pushed (c43d796 -> 45eae29); the earlier hardcoded-conditional commit is gone. README variables table updated too.

Reworked per review: instead of hardcoding `IfNotPresent`, the policy is now a template variable (`image_pull_policy`) chosen at `coder templates push`, validated against `auto` / `Always` / `IfNotPresent` / `Never`. `image_tag` is left as-is, as requested. The default is `auto`, which derives the policy from `image_tag` — `Always` for the moving `latest` alias, `IfNotPresent` for pinned version/SHA tags. Since `image_tag` still defaults to `latest`, this preserves current behaviour on the default path. Branch was force-pushed (`c43d796` -> `45eae29`); the earlier hardcoded-conditional commit is gone. README variables table updated too.