ImageCheckPullPolicy gets overriden by ImagePullPolicy

[dziemba]

The recent changes of image pull policies in #459 created an issue where the CheckPullPolicy isn't correctly computed.

Looking at this code:

agent-stack-k8s/internal/controller/scheduler/scheduler.go

Lines 708 to 721 in 107b1cf

	policy := pnr.policy
	if policy == "" {
	policy = w.cfg.DefaultImageCheckPullPolicy
	}
	if policy == "" {
	// If pull policy is unspecified by either the podSpec{,Patch} or
	// controller configuration, use Always, unless there is a
	// digest that has pinned the ref, in which case use IfNotPresent.
	// (No need to pull a pinned image ref if it's present.)
	policy = corev1.PullAlways
	if _, hasDigest := pnr.ref.(reference.Digested); hasDigest {
	policy = corev1.PullIfNotPresent
	}
	}

When no DefaultImageCheckPullPolicy is set, correct defaults are applied based on the image ref (Always if it doesn't have a digest, otherwise IfNotPresent). It seems however that these defaults never get applied when the policy of the container is already set to something, here:

agent-stack-k8s/internal/controller/scheduler/scheduler.go

Lines 410 to 412 in 107b1cf

	if c.ImagePullPolicy == "" {
	c.ImagePullPolicy = w.cfg.DefaultImagePullPolicy
	}

So if there is a DefaultImagePullPolicy set or the container spec has an explicit policy, that will always take precedence over the CheckPolicy, either set by DefaultImageCheckPullPolicy or calculated by the logic mentioned above.

When now using the default config

agent-stack-k8s/cmd/controller/controller.go

Lines 118 to 127 in 107b1cf

	cmd.Flags().String(
	"default-image-pull-policy",
	"IfNotPresent",
	"Configures a default image pull policy for containers that do not specify a pull policy and non-init containers created by the stack itself",
	)
	cmd.Flags().String(
	"default-image-check-pull-policy",
	"",
	"Sets a default PullPolicy for image-check init containers, used if an image pull policy is not set for the corresponding container in a podSpec or podSpecPatch",
	)

that sets default-image-pull-policy: "IfNotPresent" and "default-image-check-pull-policy": ""
we never get the desired behavior of always pulling images with no digest.

It seems like the only way to get the pull logic based on the digest being present is to unset the defaultImagePullPolicy. I assume then we'd fall back to the defaults the K8s sets: https://github.com/kubernetes/api/blob/b849e76aab64dcc5d2d4947cccccebdf0d8ec3c0/core/v1/types.go#L2912

This would be reasonable in a standalone scenario, but would pull twice again with your check logic.

It seems like there's no combination of settings that would achieve correctness and pull deduplication as originally indented by #459