hashicorp · thisisnotashwin · Jan 23, 2023 · Jan 19, 2023
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,7 @@ IMPROVEMENTS:
 * Helm:
   * Add a `global.extraLabels` stanza to allow setting global Kubernetes labels for all components deployed by the `consul-k8s` Helm chart. [[GH-1778](https://github.com/hashicorp/consul-k8s/pull/1778)]
 * Control-Plane
-  * Add support for the annotation `consul.hashicorp.com/use-proxy-health-check`. [[GH-1824](https://github.com/hashicorp/consul-k8s/pull/1824)]
+  * Add support for the annotation `consul.hashicorp.com/use-proxy-health-check`. When this annotation is used by a service, it configures a readiness endpoint on Consul Dataplane and queries it instead of the proxy's inbound port which forwards requests to the application. [[GH-1824](https://github.com/hashicorp/consul-k8s/pull/1824)], [[GH-1843](https://github.com/hashicorp/consul-k8s/pull/1843)]
   * Add health check for synced services based on the status of the Kubernetes readiness probe on synced pod. [[GH-1821](https://github.com/hashicorp/consul-k8s/pull/1821)]
 
 BUG FIXES:

diff --git a/control-plane/connect-inject/container_init.go b/control-plane/connect-inject/container_init.go
@@ -56,6 +56,12 @@ type initContainerCommandData struct {
 	// EnvoyUID is the Linux user id that will be used when tproxy is enabled.
 	EnvoyUID int
 
+	// EnableProxyHealthChecks configures a readiness endpoint on the envoy sidecar.
+	EnableProxyHealthChecks bool
+	// EnableProxyHealthChecks is the port on which the readiness endpoint is configured
+	// on the envoy sidecar.
+	EnvoyHealthCheckPort int
+
 	// EnableTransparentProxy configures this init container to run in transparent proxy mode,
 	// i.e. run consul connect redirect-traffic command and add the required privileges to the
 	// container to do that.
@@ -166,6 +172,8 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod,
 		ConsulCACert:               w.ConsulCACert,
 		EnableTransparentProxy:     tproxyEnabled,
 		EnableCNI:                  w.EnableCNI,
+		EnableProxyHealthChecks:    useProxyHealthCheck(pod),
+		EnvoyHealthCheckPort:       proxyDefaultHealthPort + mpi.serviceIndex,
 		TProxyExcludeInboundPorts:  splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeInboundPorts, pod),
 		TProxyExcludeOutboundPorts: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeOutboundPorts, pod),
 		TProxyExcludeOutboundCIDRs: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeOutboundCIDRs, pod),
@@ -436,6 +444,10 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
   {{- else }}
   -proxy-id="$(cat /consul/connect-inject/proxyid)" \
   {{- end }}
+  {{- if .EnableProxyHealthChecks }}
+  -envoy-ready-bind-address="${POD_IP}" \
+  -envoy-ready-bind-port={{ .EnvoyHealthCheckPort }} \
+  {{- end }}
   {{- if .PrometheusScrapePath }}
   -prometheus-scrape-path="{{ .PrometheusScrapePath }}" \
   {{- end }}
@@ -498,6 +510,9 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
   {{- range .TProxyExcludeOutboundPorts }}
   -exclude-outbound-port="{{ . }}" \
   {{- end }}
+  {{- if .EnableProxyHealthChecks }}
+  -exclude-inbound-port={{ .EnvoyHealthCheckPort }} \
+  {{- end }}
   {{- range .TProxyExcludeOutboundCIDRs }}
   -exclude-outbound-cidr="{{ . }}" \
   {{- end }}

diff --git a/control-plane/connect-inject/container_init_test.go b/control-plane/connect-inject/container_init_test.go
@@ -74,7 +74,29 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
 			"",
 			"",
 		},
+		{
+			"Proxy Health Checks",
+			func(pod *corev1.Pod) *corev1.Pod {
+				pod.Annotations[annotationService] = "web"
+				pod.Annotations[annotationUseProxyHealthCheck] = "true"
+				return pod
+			},
+			MeshWebhook{},
+			`/bin/sh -ec 
+export CONSUL_HTTP_ADDR="${HOST_IP}:8500"
+export CONSUL_GRPC_ADDR="${HOST_IP}:8502"
+consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \
+  -consul-api-timeout=0s \
 
+# Generate the envoy bootstrap code
+/consul/connect-inject/consul connect envoy \
+  -proxy-id="$(cat /consul/connect-inject/proxyid)" \
+  -envoy-ready-bind-address="${POD_IP}" \
+  -envoy-ready-bind-port=21000 \
+  -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`,
+			"",
+			"",
+		},
 		{
 			"When auth method is set -service-account-name and -service-name are passed in",
 			func(pod *corev1.Pod) *corev1.Pod {
@@ -384,7 +406,6 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
   -proxy-uid=5995`,
 			map[string]string{keyTransparentProxy: "true"},
 		},
-
 		"enabled globally, ns not set, annotation not set, cni enabled": {
 			true,
 			true,
@@ -395,6 +416,17 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
   -proxy-uid=5995`,
 			nil,
 		},
+		"enabled globally, ns not set, annotation is true, cni disabled, proxy health checks": {
+			true,
+			false,
+			map[string]string{keyTransparentProxy: "true", annotationUseProxyHealthCheck: "true"},
+			`/consul/connect-inject/consul connect redirect-traffic \
+  -exclude-inbound-port=21000 \
+  -proxy-id="$(cat /consul/connect-inject/proxyid)" \
+  -proxy-uid=5995`,
+			"",
+			nil,
+		},
 	}
 	for name, c := range cases {
 		t.Run(name, func(t *testing.T) {
@@ -995,6 +1027,60 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
 /consul/connect-inject/consul connect envoy \
   -proxy-id="$(cat /consul/connect-inject/proxyid-web-admin)" \
   -admin-bind=127.0.0.1:19001 \
+  -bootstrap > /consul/connect-inject/envoy-bootstrap-web-admin.yaml`,
+			},
+		},
+		{
+			"Whole template, multiport, proxy health check",
+			func(pod *corev1.Pod) *corev1.Pod {
+				pod.Annotations[annotationUseProxyHealthCheck] = "true"
+				return pod
+			},
+			MeshWebhook{ConsulAPITimeout: 5 * time.Second},
+			2,
+			[]multiPortInfo{
+				{
+					serviceIndex: 0,
+					serviceName:  "web",
+				},
+				{
+					serviceIndex: 1,
+					serviceName:  "web-admin",
+				},
+			},
+			[]string{
+				`/bin/sh -ec 
+export CONSUL_HTTP_ADDR="${HOST_IP}:8500"
+export CONSUL_GRPC_ADDR="${HOST_IP}:8502"
+consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \
+  -consul-api-timeout=5s \
+  -multiport=true \
+  -proxy-id-file=/consul/connect-inject/proxyid-web \
+  -service-name="web" \
+
+# Generate the envoy bootstrap code
+/consul/connect-inject/consul connect envoy \
+  -proxy-id="$(cat /consul/connect-inject/proxyid-web)" \
+  -envoy-ready-bind-address="${POD_IP}" \
+  -envoy-ready-bind-port=21000 \
+  -admin-bind=127.0.0.1:19000 \
+  -bootstrap > /consul/connect-inject/envoy-bootstrap-web.yaml`,
+
+				`/bin/sh -ec 
+export CONSUL_HTTP_ADDR="${HOST_IP}:8500"
+export CONSUL_GRPC_ADDR="${HOST_IP}:8502"
+consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \
+  -consul-api-timeout=5s \
+  -multiport=true \
+  -proxy-id-file=/consul/connect-inject/proxyid-web-admin \
+  -service-name="web-admin" \
+
+# Generate the envoy bootstrap code
+/consul/connect-inject/consul connect envoy \
+  -proxy-id="$(cat /consul/connect-inject/proxyid-web-admin)" \
+  -envoy-ready-bind-address="${POD_IP}" \
+  -envoy-ready-bind-port=21001 \
+  -admin-bind=127.0.0.1:19001 \
   -bootstrap > /consul/connect-inject/envoy-bootstrap-web-admin.yaml`,
 			},
 		},

diff --git a/control-plane/connect-inject/endpoints_controller.go b/control-plane/connect-inject/endpoints_controller.go
@@ -61,6 +61,9 @@ const (
 
 	// proxyDefaultInboundPort is the default inbound port for the proxy.
 	proxyDefaultInboundPort = 20000
+
+	// proxyDefaultHealthPort is the default health check port for the proxy.
+	proxyDefaultHealthPort = 21000
 )
 
 type EndpointsController struct {
@@ -501,6 +504,31 @@ func (r *EndpointsController) createServiceRegistrations(pod corev1.Pod, service
 	if idx := getMultiPortIdx(pod, serviceEndpoints); idx >= 0 {
 		proxyPort += idx
 	}
+	var publicListenerCheck api.AgentServiceCheck
+	if useProxyHealthCheck(pod) {
+		// When using the proxy's health check, create an HTTP check on the ready endpoint
+		// that will be configured on the proxy sidecar container.
+		healthCheckPort := proxyDefaultHealthPort
+		if idx := getMultiPortIdx(pod, serviceEndpoints); idx >= 0 {
+			healthCheckPort += idx
+		}
+		publicListenerCheck = api.AgentServiceCheck{
+			Name:                           "Proxy Public Listener",
+			HTTP:                           fmt.Sprintf("http://%s:%d/ready", pod.Status.PodIP, healthCheckPort),
+			TLSSkipVerify:                  true,
+			Interval:                       "10s",
+			DeregisterCriticalServiceAfter: "10m",
+		}
+	} else {
+		// Configure the default application health check.
+		publicListenerCheck = api.AgentServiceCheck{
+			Name:                           "Proxy Public Listener",
+			TCP:                            fmt.Sprintf("%s:%d", pod.Status.PodIP, proxyPort),
+			Interval:                       "10s",
+			DeregisterCriticalServiceAfter: "10m",
+		}
+	}
+
 	proxyService := &api.AgentServiceRegistration{
 		Kind:      api.ServiceKindConnectProxy,
 		ID:        proxyServiceID,
@@ -511,12 +539,7 @@ func (r *EndpointsController) createServiceRegistrations(pod corev1.Pod, service
 		Namespace: r.consulNamespace(pod.Namespace),
 		Proxy:     proxyConfig,
 		Checks: api.AgentServiceChecks{
-			{
-				Name:                           "Proxy Public Listener",
-				TCP:                            fmt.Sprintf("%s:%d", pod.Status.PodIP, proxyPort),
-				Interval:                       "10s",
-				DeregisterCriticalServiceAfter: "10m",
-			},
+			&publicListenerCheck,
 			{
 				Name:         "Destination Alias",
 				AliasService: serviceID,

diff --git a/control-plane/connect-inject/endpoints_controller_test.go b/control-plane/connect-inject/endpoints_controller_test.go
@@ -1187,6 +1187,7 @@ func TestReconcileCreateEndpoint(t *testing.T) {
 		expectedProxySvcInstances  []*api.CatalogService
 		expectedAgentHealthChecks  []*api.AgentCheck
 		expErr                     string
+		useProxyHealthChecks       bool
 	}{
 		{
 			name:          "Empty endpoints",
@@ -1279,6 +1280,76 @@ func TestReconcileCreateEndpoint(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:                 "Basic endpoints with proxy healthchecks",
+			useProxyHealthChecks: true,
+			consulSvcName:        "service-created",
+			k8sObjects: func() []runtime.Object {
+				pod1 := createPod("pod1", "1.2.3.4", true, true)
+				pod1.Annotations[annotationUseProxyHealthCheck] = "true"
+				endpoint := &corev1.Endpoints{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "service-created",
+						Namespace: "default",
+					},
+					Subsets: []corev1.EndpointSubset{
+						{
+							Addresses: []corev1.EndpointAddress{
+								{
+									IP:       "1.2.3.4",
+									NodeName: &nodeName,
+									TargetRef: &corev1.ObjectReference{
+										Kind:      "Pod",
+										Name:      "pod1",
+										Namespace: "default",
+									},
+								},
+							},
+						},
+					},
+				}
+				return []runtime.Object{pod1, endpoint}
+			},
+			initialConsulSvcs:       []*api.AgentServiceRegistration{},
+			expectedNumSvcInstances: 1,
+			expectedConsulSvcInstances: []*api.CatalogService{
+				{
+					ServiceID:      "pod1-service-created",
+					ServiceName:    "service-created",
+					ServiceAddress: "1.2.3.4",
+					ServicePort:    0,
+					ServiceMeta:    map[string]string{MetaKeyPodName: "pod1", MetaKeyKubeServiceName: "service-created", MetaKeyKubeNS: "default", MetaKeyManagedBy: managedByValue},
+					ServiceTags:    []string{},
+				},
+			},
+			expectedProxySvcInstances: []*api.CatalogService{
+				{
+					ServiceID:      "pod1-service-created-sidecar-proxy",
+					ServiceName:    "service-created-sidecar-proxy",
+					ServiceAddress: "1.2.3.4",
+					ServicePort:    20000,
+					ServiceProxy: &api.AgentServiceConnectProxyConfig{
+						DestinationServiceName: "service-created",
+						DestinationServiceID:   "pod1-service-created",
+						LocalServiceAddress:    "",
+						LocalServicePort:       0,
+					},
+					ServiceMeta: map[string]string{MetaKeyPodName: "pod1", MetaKeyKubeServiceName: "service-created", MetaKeyKubeNS: "default", MetaKeyManagedBy: managedByValue},
+					ServiceTags: []string{},
+				},
+			},
+			expectedAgentHealthChecks: []*api.AgentCheck{
+				{
+					CheckID:     "default/pod1-service-created/kubernetes-health-check",
+					ServiceName: "service-created",
+					ServiceID:   "pod1-service-created",
+					Name:        "Kubernetes Health Check",
+					Status:      api.HealthPassing,
+					Output:      kubernetesSuccessReasonMsg,
+					Type:        ttl,
+				},
+			},
+		},
 		{
 			name:          "Endpoints with multiple addresses",
 			consulSvcName: "service-created",
@@ -1811,6 +1882,17 @@ func TestReconcileCreateEndpoint(t *testing.T) {
 				require.Contains(t, expectedChecks, checks[0].Name)
 				require.Contains(t, expectedChecks, checks[1].Name)
 			}
+			agentChecks, err := consulClient.Agent().Checks()
+			require.NoError(t, err)
+			for _, check := range agentChecks {
+				if check.Name == "Proxy Public Listener" {
+					if tt.useProxyHealthChecks {
+						require.Equal(t, "http", check.Type)
+					} else {
+						require.Equal(t, "tcp", check.Type)
+					}
+				}
+			}
 
 			// Check that the Consul health check was created for the k8s pod.
 			if tt.expectedAgentHealthChecks != nil {

diff --git a/control-plane/connect-inject/envoy_sidecar.go b/control-plane/connect-inject/envoy_sidecar.go
@@ -50,6 +50,14 @@ func (w *MeshWebhook) envoySidecar(namespace corev1.Namespace, pod corev1.Pod, m
 		Command: cmd,
 	}
 
+	if useProxyHealthCheck(pod) {
+		// Add a port on the sidecar where the sidecar proxy will be queried for its health.
+		container.Ports = append(container.Ports, corev1.ContainerPort{
+			Name:          fmt.Sprintf("%s-%d", "proxy-health", mpi.serviceIndex),
+			ContainerPort: int32(proxyDefaultHealthPort + mpi.serviceIndex),
+		})
+	}
+
 	// Add any extra Envoy VolumeMounts.
 	if _, ok := pod.Annotations[annotationConsulSidecarUserVolumeMount]; ok {
 		var volumeMount []corev1.VolumeMount
@@ -105,7 +113,7 @@ func (w *MeshWebhook) getContainerSidecarCommand(pod corev1.Pod, multiPortSvcNam
 	}
 	if multiPortSvcName != "" {
 		// --base-id is needed so multiple Envoy proxies can run on the same host.
-		cmd = append(cmd, "--base-id", fmt.Sprintf("%d", multiPortSvcIdx))
+		cmd = append(cmd, "--base-id", strconv.Itoa(multiPortSvcIdx))
 	}
 
 	// Check to see if the user has overriden concurrency via an annotation.
@@ -215,3 +223,16 @@ func (w *MeshWebhook) envoySidecarResources(pod corev1.Pod) (corev1.ResourceRequ
 
 	return resources, nil
 }
+
+// useProxyHealthCheck returns true if the pod has the annotation 'consul.hashicorp.com/use-proxy-health-check'
+// set to truthy values.
+func useProxyHealthCheck(pod corev1.Pod) bool {
+	if v, ok := pod.Annotations[annotationUseProxyHealthCheck]; ok {
+		useProxyHealthCheck, err := strconv.ParseBool(v)
+		if err != nil {
+			return false
+		}
+		return useProxyHealthCheck
+	}
+	return false
+}