From a8c0b2ebb537119c1ce5aa921552767ed03a4aa1 Mon Sep 17 00:00:00 2001
From: hc-github-team-nomad-core
 <82989552+hc-github-team-nomad-core@users.noreply.github.com>
Date: Thu, 7 Dec 2023 13:27:56 -0600
Subject: [PATCH] Backport of connect: deployments should wait for Connect
 sidecar checks into release/1.6.x #19365

Co-authored-by: Tim Gross <tgross@hashicorp.com>
---
 .changelog/19334.txt                          |  3 ++
 client/allochealth/tracker.go                 | 17 +++++++++
 client/allochealth/tracker_test.go            | 36 +++++++++++++++++++
 .../service_registration.go                   |  6 ++++
 command/agent/consul/service_client.go        |  9 +++++
 5 files changed, 71 insertions(+)
 create mode 100644 .changelog/19334.txt

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)
+					}
+				}
+			}
 		}
 	}