From 92e7646221adaa56a76605bfc5033b68f93523d2 Mon Sep 17 00:00:00 2001 From: Samuel Enocsson Date: Tue, 13 Oct 2026 08:25:21 +0200 Subject: [PATCH] fix(master): say why a pinned pick fails on a protocol 2 agent (#9) --- internal/master/hub/route.go | 6 +++++- internal/master/hub/route_test.go | 4 ++-- internal/master/jobs/jobs.go | 5 +++-- internal/master/jobs/jobs_test.go | 5 ++++- internal/master/model/job.go | 12 ++++++++++++ 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/internal/master/hub/route.go b/internal/master/hub/route.go index 19fd603..46309b5 100644 --- a/internal/master/hub/route.go +++ b/internal/master/hub/route.go @@ -16,7 +16,8 @@ type candidate struct { // Route picks the connection a job for svc goes to, among agents that speak // at least protocol version minProtocol. It returns model.ErrAgentOffline or -// model.ErrAgentTooOld when there is none. +// model.ErrAgentTooOld (model.ErrAgentTooOldForPinned when minProtocol asks +// for a pinned ref) when there is none. func (h *Hub) Route(svc model.Service, minProtocol int) (model.Route, error) { h.mu.Lock() conns := make([]candidate, 0, len(h.conns)) @@ -64,6 +65,9 @@ func route(svc model.Service, conns []candidate, minProtocol int) (model.Route, } } if tooOld { + if minProtocol >= protocol.MinPinnedRefVersion { + return model.Route{}, model.ErrAgentTooOldForPinned + } return model.Route{}, model.ErrAgentTooOld } return model.Route{}, model.ErrAgentOffline diff --git a/internal/master/hub/route_test.go b/internal/master/hub/route_test.go index 9393cc8..c0c2fa3 100644 --- a/internal/master/hub/route_test.go +++ b/internal/master/hub/route_test.go @@ -30,7 +30,7 @@ func TestRoute(t *testing.T) { {"compose, own agent too old", composeSvc, []candidate{{2, 20, protocol.Hello{}}}, jobs, model.Route{}, model.ErrAgentTooOld}, {"compose, own agent missing", composeSvc, nil, jobs, model.Route{}, model.ErrAgentOffline}, {"compose, only another agent online", composeSvc, []candidate{{5, 50, v2}}, jobs, model.Route{}, model.ErrAgentOffline}, - {"compose, v2 agent and a pinned ref", composeSvc, []candidate{{2, 20, v2}}, pinned, model.Route{}, model.ErrAgentTooOld}, + {"compose, v2 agent and a pinned ref", composeSvc, []candidate{{2, 20, v2}}, pinned, model.Route{}, model.ErrAgentTooOldForPinned}, {"compose, v3 agent and a pinned ref", composeSvc, []candidate{{2, 20, v3}}, pinned, model.Route{AgentID: 2, ConnID: 20}, nil}, {"swarm on reporting agent", swarmSvc, []candidate{{1, 10, mgrV2}, {3, 30, mgrV2}}, jobs, model.Route{AgentID: 1, ConnID: 10}, nil}, {"swarm, reporting agent offline, other manager", swarmSvc, []candidate{{3, 30, mgrV2}}, jobs, model.Route{AgentID: 3, ConnID: 30}, nil}, @@ -47,7 +47,7 @@ func TestRoute(t *testing.T) { []candidate{{1, 10, protocol.Hello{ProtocolVersion: 2, SwarmManager: true}}}, jobs, model.Route{}, model.ErrAgentOffline}, {"swarm, pinned ref falls back to a v3 manager when the reporting one is v2", swarmSvc, []candidate{{1, 10, mgrV2}, {3, 30, mgrV3}}, pinned, model.Route{AgentID: 3, ConnID: 30}, nil}, - {"swarm, pinned ref and only v2 managers", swarmSvc, []candidate{{1, 10, mgrV2}}, pinned, model.Route{}, model.ErrAgentTooOld}, + {"swarm, pinned ref and only v2 managers", swarmSvc, []candidate{{1, 10, mgrV2}}, pinned, model.Route{}, model.ErrAgentTooOldForPinned}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/master/jobs/jobs.go b/internal/master/jobs/jobs.go index f479dd3..a1bb514 100644 --- a/internal/master/jobs/jobs.go +++ b/internal/master/jobs/jobs.go @@ -156,8 +156,9 @@ func (s *Service) TriggerVersion(ctx context.Context, serviceID, versionID int64 } // Start starts an update of svc. It returns the job, which may already be -// failed ("agent offline", "agent too old for updates", the latter also when -// the ref has a digest or no tag and the agent speaks protocol version 2); a +// failed ("agent offline", "agent too old for updates", or "agent too old to +// deploy a pinned version" when the ref has a digest or no tag and the agent +// speaks protocol version 2); a // *model.NotUpdatableError when the service cannot be updated (no job // created); the active job and model.ErrJobActive when one is queued or // running. diff --git a/internal/master/jobs/jobs_test.go b/internal/master/jobs/jobs_test.go index 7ad6044..ed8509b 100644 --- a/internal/master/jobs/jobs_test.go +++ b/internal/master/jobs/jobs_test.go @@ -83,6 +83,9 @@ type fakeHub struct { func (h *fakeHub) Route(_ model.Service, minProtocol int) (model.Route, error) { h.mins = append(h.mins, minProtocol) if h.agentProtocol != 0 && minProtocol > h.agentProtocol { + if minProtocol >= protocol.MinPinnedRefVersion { + return model.Route{}, model.ErrAgentTooOldForPinned + } return model.Route{}, model.ErrAgentTooOld } return h.route, h.routeErr @@ -481,7 +484,7 @@ func TestTriggerVersion_RequiresPinnedProtocol(t *testing.T) { pinned, err := s.TriggerVersion(context.Background(), 1, 10, "sub-1") require.NoError(t, err) assert.Equal(t, model.JobFailed, pinned.Status) - assert.Equal(t, "agent too old for updates", pinned.Error) + assert.Equal(t, "agent too old to deploy a pinned version (needs protocol 3)", pinned.Error) assert.Empty(t, h.starts, "nothing sent to the old agent") plain, err := s.TriggerVersion(context.Background(), 1, 11, "sub-1") diff --git a/internal/master/model/job.go b/internal/master/model/job.go index 5c1f8b9..ea0f24f 100644 --- a/internal/master/model/job.go +++ b/internal/master/model/job.go @@ -15,8 +15,20 @@ var ( // ErrAgentTooOld means the only agent that could run the job speaks a // protocol version without jobs. ErrAgentTooOld = errors.New("agent too old for updates") + // ErrAgentTooOldForPinned means the only agent that could run the job + // speaks protocol version 2, which cannot deploy a digest-pinned or + // tagless ref. errors.Is(err, ErrAgentTooOld) is true for it as well. + ErrAgentTooOldForPinned error = agentTooOldForPinned{} ) +type agentTooOldForPinned struct{} + +func (agentTooOldForPinned) Error() string { + return "agent too old to deploy a pinned version (needs protocol 3)" +} + +func (agentTooOldForPinned) Is(target error) bool { return target == ErrAgentTooOld } + // NotUpdatableError says why a service cannot be updated. Reason is shown // in the UI as is. type NotUpdatableError struct {