diff --git a/.changelog/19334.txt b/.changelog/19334.txt new file mode 100644 index 000000000..cdf7d066e --- /dev/null +++ b/.changelog/19334.txt @@ -0,0 +1,3 @@ +```release-note:bug +connect: Fixed a bug where deployments would not wait for Connect sidecar task health checks to pass +``` diff --git a/client/allochealth/tracker.go b/client/allochealth/tracker.go index 4058fe289..2887ad82c 100644 --- a/client/allochealth/tracker.go +++ b/client/allochealth/tracker.go @@ -575,6 +575,8 @@ func evaluateConsulChecks(services []*structs.Service, registrations *servicereg } } + // The sidecar service and checks are created when the service is + // registered, not on job registration, so they won't appear in the jobspec. if !maps.Equal(expChecks, regChecks) { return false } @@ -595,6 +597,21 @@ func evaluateConsulChecks(services []*structs.Service, registrations *servicereg } } } + + // Check the health of Connect sidecars, so we don't accidentally + // mark an allocation healthy before min_healthy_time. These don't + // currently support on_update. + if service.SidecarService != nil { + for _, check := range service.SidecarChecks { + switch check.Status { + case api.HealthWarning: + return false + case api.HealthCritical: + return false + } + } + } + } } diff --git a/client/allochealth/tracker_test.go b/client/allochealth/tracker_test.go index 88e3e0d97..0eb51fc52 100644 --- a/client/allochealth/tracker_test.go +++ b/client/allochealth/tracker_test.go @@ -1466,6 +1466,42 @@ func TestTracker_evaluateConsulChecks(t *testing.T) { }, }, }, + { + name: "failing sidecar checks only", + exp: false, + tg: &structs.TaskGroup{ + Services: []*structs.Service{{ + Name: "group-s1", + Checks: []*structs.ServiceCheck{ + {Name: "c1"}, + }, + }}, + }, + registrations: &serviceregistration.AllocRegistration{ + Tasks: map[string]*serviceregistration.ServiceRegistrations{ + "group": { + Services: map[string]*serviceregistration.ServiceRegistration{ + "abc123": { + ServiceID: "abc123", + Checks: []*consulapi.AgentCheck{ + { + Name: "c1", + Status: consulapi.HealthPassing, + }, + }, + SidecarService: &consulapi.AgentService{}, + SidecarChecks: []*consulapi.AgentCheck{ + { + Name: "sidecar-check", + Status: consulapi.HealthCritical, + }, + }, + }, + }, + }, + }, + }, + }, } for _, tc := range cases { diff --git a/client/serviceregistration/service_registration.go b/client/serviceregistration/service_registration.go index ec34bd979..2fbaf4de1 100644 --- a/client/serviceregistration/service_registration.go +++ b/client/serviceregistration/service_registration.go @@ -144,6 +144,12 @@ type ServiceRegistration struct { // Checks is the status of the registered checks. Checks []*api.AgentCheck + + // SidecarService is the AgentService registered in Consul for any Connect sidecar + SidecarService *api.AgentService + + // SidecarChecks is the status of the registered checks for any Connect sidecar + SidecarChecks []*api.AgentCheck } func (s *ServiceRegistration) copy() *ServiceRegistration { diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go index 8c3b61039..1b5c7680a 100644 --- a/command/agent/consul/service_client.go +++ b/command/agent/consul/service_client.go @@ -1489,6 +1489,15 @@ func (c *ServiceClient) AllocRegistrations(allocID string) (*serviceregistration sreg.Checks = append(sreg.Checks, check) } } + + if sidecarService := getNomadSidecar(serviceID, services); sidecarService != nil { + sreg.SidecarService = sidecarService + for _, check := range checks { + if check.ServiceID == sidecarService.ID { + sreg.SidecarChecks = append(sreg.SidecarChecks, check) + } + } + } } }