diff --git a/internal/orchestrator/agent_image_test.go b/internal/orchestrator/agent_image_test.go new file mode 100644 index 0000000..ef3cba1 --- /dev/null +++ b/internal/orchestrator/agent_image_test.go @@ -0,0 +1,40 @@ +package orchestrator + +import "testing" + +// Agent-mode cases were silently running :latest because --version reached the +// subject but not the agent. The bug was invisible: the case passed, and read +// as agent coverage while exercising whatever agent was published last. +func TestApplyVersionIfUntagged(t *testing.T) { + tests := []struct { + name string + image string + version string + want string + }{ + {"untagged gets the run version", "vmetric/agent", "2.1.0", "vmetric/agent:2.1.0"}, + {"bare name", "agent", "2.1.0", "agent:2.1.0"}, + + // A pinned tag is a deliberate choice — upgrade-compat cases hold the + // agent OLDER than the subject on purpose. + {"explicit tag is kept", "vmetric/agent:oneknob", "2.1.0", "vmetric/agent:oneknob"}, + {"explicit latest is kept", "vmetric/agent:latest", "2.1.0", "vmetric/agent:latest"}, + {"digest pin is kept", "vmetric/agent@sha256:abc", "2.1.0", "vmetric/agent@sha256:abc"}, + + // A registry port is a colon that is NOT a tag. Treating it as one + // would leave the image untagged and silently reintroduce the bug. + {"registry port, untagged", "registry:5000/vmetric/agent", "2.1.0", "registry:5000/vmetric/agent:2.1.0"}, + {"registry port, tagged", "registry:5000/vmetric/agent:oneknob", "2.1.0", "registry:5000/vmetric/agent:oneknob"}, + + {"empty image", "", "2.1.0", ""}, + {"empty version", "vmetric/agent", "", "vmetric/agent"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := applyVersionIfUntagged(test.image, test.version); got != test.want { + t.Errorf("applyVersionIfUntagged(%q, %q) = %q, want %q", test.image, test.version, got, test.want) + } + }) + } +} diff --git a/internal/orchestrator/docker.go b/internal/orchestrator/docker.go index e3fa046..fdd3d31 100644 --- a/internal/orchestrator/docker.go +++ b/internal/orchestrator/docker.go @@ -2606,7 +2606,20 @@ func writeCompose(path string, cfg RunConfig) error { agentEnv[k] = strings.ReplaceAll(v, "$", "$$") } vars.AgentEnabled = true - vars.AgentImage = a.Image + + // Apply the run's --version to an UNTAGGED agent image, the same way + // the subject gets one. + // + // Without this an agent case written as `image: "vmetric/agent"` runs + // :latest while the subject runs the build under test, so the case + // silently exercises whatever agent was published last — usually an + // OLD one. It still passes, and reads as agent-mode coverage while + // proving nothing about the agent being tested. + // + // A case that pins its own tag is left alone: that is how + // director_old_agent_compat_deploy holds an agent DELIBERATELY older + // than the subject, which is the point of an upgrade-compatibility case. + vars.AgentImage = applyVersionIfUntagged(a.Image, s.Version) vars.AgentCommand = formatYAMLList(agentCmd) vars.AgentEnv = agentEnv vars.AgentMountsSharedData = a.MountsSharedData @@ -2658,3 +2671,27 @@ func boolStr(b bool) string { } return "false" } + +// applyVersionIfUntagged appends ":" to an image reference that has no +// tag, leaving an explicitly tagged reference untouched. +// +// Tag detection has to cope with a registry host carrying a port +// ("registry:5000/vmetric/agent"), where the only colon is NOT a tag separator. +// A tag can only appear after the last "/", so that is where to look. +func applyVersionIfUntagged(image, version string) string { + if image == "" || version == "" { + return image + } + + name := image + if slash := strings.LastIndex(image, "/"); slash >= 0 { + name = image[slash+1:] + } + + // Already tagged (or digest-pinned) — the case chose it on purpose. + if strings.Contains(name, ":") || strings.Contains(name, "@") { + return image + } + + return image + ":" + version +}