Merge pull request #9995 from hashicorp/f-expose-custom

consul/connect: enable custom sidecars to use expose checks

CHANGELOG.md

@@ -5,6 +5,7 @@ FEATURES:
 
 IMPROVEMENTS:
  * cli: Improved `scaling policy` commands with -verbose, auto-completion, and prefix-matching [[GH-9964](https://github.com/hashicorp/nomad/issues/9964)]
+ * consul/connect: Enable custom sidecar tasks to use connect expose checks [[GH-9995](https://github.com/hashicorp/nomad/pull/9995)]
  * consul/connect: Made handling of sidecar task container image URLs consistent with the `docker` task driver. [[GH-9580](https://github.com/hashicorp/nomad/issues/9580)]
  * drivers/exec+java: Added client plugin configuration to re-enable previous PID/IPC namespace behavior [[GH-9982](https://github.com/hashicorp/nomad/pull/9982)]
 

e2e/connect/connect.go

@@ -15,6 +15,10 @@ const (
 	// demoConnectJob is the example connect enabled job useful for testing
 	demoConnectJob = "connect/input/demo.nomad"
 
+	// demoConnectCustomProxyExposed is a connect job with custom sidecar_task
+	// that also uses the expose check feature.
+	demoConnectCustomProxyExposed = "connect/input/expose-custom.nomad"
+
 	// demoConnectNativeJob is the example connect native enabled job useful for testing
 	demoConnectNativeJob = "connect/input/native-demo.nomad"
 
@@ -34,7 +38,7 @@ type ConnectE2ETest struct {
 }
 
 func init() {
-	// connect tests without Consul ACLs enabled
+	// Connect tests without Consul ACLs enabled.
 	framework.AddSuites(&framework.TestSuite{
 		Component:   "Connect",
 		CanRunLocal: true,
@@ -45,16 +49,22 @@ func init() {
 		},
 	})
 
-	// connect tests with Consul ACLs enabled
-	framework.AddSuites(&framework.TestSuite{
-		Component:   "ConnectACLs",
-		CanRunLocal: false,
-		Consul:      true,
-		Parallel:    false,
-		Cases: []framework.TestCase{
-			new(ConnectACLsE2ETest),
-		},
-	})
+	// Connect tests with Consul ACLs enabled. These are now gated behind the
+	// NOMAD_TEST_CONNECT_ACLS environment variable, because they cause lots of
+	// problems for e2e test flakiness (due to restarting consul, nomad, etc.).
+	//
+	// Run these tests locally when working on Connect.
+	if os.Getenv("NOMAD_TEST_CONNECT_ACLS") == "1" {
+		framework.AddSuites(&framework.TestSuite{
+			Component:   "ConnectACLs",
+			CanRunLocal: false,
+			Consul:      true,
+			Parallel:    false,
+			Cases: []framework.TestCase{
+				new(ConnectACLsE2ETest),
+			},
+		})
+	}
 }
 
 func (tc *ConnectE2ETest) BeforeAll(f *framework.F) {
@@ -90,6 +100,19 @@ func (tc *ConnectE2ETest) TestConnectDemo(f *framework.F) {
 	e2eutil.WaitForAllocsRunning(t, tc.Nomad(), allocIDs)
 }
 
+// TestConnectCustomSidecarExposed tests that a connect sidecar with custom task
+// definition can also make use of the expose service check feature.
+func (tc *ConnectE2ETest) TestConnectCustomSidecarExposed(f *framework.F) {
+	t := f.T()
+
+	jobID := connectJobID()
+	tc.jobIds = append(tc.jobIds, jobID)
+
+	allocs := e2eutil.RegisterAndWaitForAllocs(t, tc.Nomad(), demoConnectCustomProxyExposed, jobID, "")
+	allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocs)
+	e2eutil.WaitForAllocsRunning(t, tc.Nomad(), allocIDs)
+}
+
 // TestConnectNativeDemo tests the demo job file used in Connect Native Integration examples.
 func (tc *ConnectE2ETest) TestConnectNativeDemo(f *framework.F) {
 	t := f.T()

e2e/connect/input/demo.nomad

@@ -33,7 +33,7 @@ job "countdash" {
       driver = "docker"
 
       config {
-        image = "hashicorpnomad/counter-api:v2"
+        image = "hashicorpnomad/counter-api:v3"
       }
     }
   }
@@ -72,7 +72,7 @@ job "countdash" {
       }
 
       config {
-        image = "hashicorpnomad/counter-dashboard:v2"
+        image = "hashicorpnomad/counter-dashboard:v3"
       }
     }
   }

e2e/connect/input/expose-custom.nomad

@@ -0,0 +1,46 @@
+job "expose-custom" {
+  datacenters = ["dc1"]
+
+  constraint {
+    attribute = "${attr.kernel.name}"
+    value     = "linux"
+  }
+
+  group "api" {
+    network {
+      mode = "bridge"
+    }
+
+    service {
+      name = "count-api"
+      port = "9001"
+
+      connect {
+        sidecar_service {}
+        sidecar_task {
+          resources {
+            cpu    = 133
+            memory = 63
+          }
+        }
+      }
+
+      check {
+        expose   = true
+        name     = "api-health"
+        type     = "http"
+        path     = "/health"
+        interval = "10s"
+        timeout  = "3s"
+      }
+    }
+
+    task "web" {
+      driver = "docker"
+
+      config {
+        image = "hashicorpnomad/counter-api:v3"
+      }
+    }
+  }
+}

e2e/connect/input/native-demo.nomad

@@ -25,7 +25,7 @@ job "cn-demo" {
       driver = "docker"
 
       config {
-        image        = "hashicorpnomad/uuid-api:v1"
+        image        = "hashicorpnomad/uuid-api:v5"
         network_mode = "host"
       }
 
@@ -57,7 +57,7 @@ job "cn-demo" {
       driver = "docker"
 
       config {
-        image        = "hashicorpnomad/uuid-fe:v1"
+        image        = "hashicorpnomad/uuid-fe:v5"
         network_mode = "host"
       }
 

e2e/e2e_test.go

@@ -11,7 +11,7 @@ import (
 	_ "github.com/hashicorp/nomad/e2e/affinities"
 	_ "github.com/hashicorp/nomad/e2e/clientstate"
 
-	// _ "github.com/hashicorp/nomad/e2e/connect"
+	_ "github.com/hashicorp/nomad/e2e/connect"
 	_ "github.com/hashicorp/nomad/e2e/consul"
 	_ "github.com/hashicorp/nomad/e2e/consultemplate"
 	_ "github.com/hashicorp/nomad/e2e/csi"

nomad/job_endpoint_hook_expose_check.go

@@ -102,9 +102,9 @@ func tgValidateUseOfCheckExpose(tg *structs.TaskGroup) error {
 	// validation for group services (which must use built-in connect proxy)
 	for _, s := range tg.Services {
 		for _, check := range s.Checks {
-			if check.Expose && !serviceUsesConnectEnvoy(s) {
+			if check.Expose && !s.Connect.HasSidecar() {
 				return errors.Errorf(
-					"exposed service check %s->%s->%s requires use of Nomad's builtin Connect proxy",
+					"exposed service check %s->%s->%s requires use of sidecar_proxy",
 					tg.Name, s.Name, check.Name,
 				)
 			}
@@ -155,29 +155,6 @@ func tgUsesExposeCheck(tg *structs.TaskGroup) bool {
 	return false
 }
 
-// serviceUsesConnectEnvoy returns true if the service is going to end up using
-// the built-in envoy proxy.
-//
-// This implementation is kind of reading tea leaves - firstly Connect
-// must be enabled, and second the sidecar_task must not be overridden. If these
-// conditions are met, the preceding connect hook will have injected a Connect
-// sidecar task, the configuration of which is interpolated at runtime.
-func serviceUsesConnectEnvoy(s *structs.Service) bool {
-	// A non-nil connect stanza implies this service isn't connect enabled in
-	// the first place.
-	if s.Connect == nil {
-		return false
-	}
-
-	// A non-nil connect.sidecar_task stanza implies the sidecar task is being
-	// overridden (i.e. the default Envoy is not being used).
-	if s.Connect.SidecarTask != nil {
-		return false
-	}
-
-	return true
-}
-
 // checkIsExposable returns true if check is qualified for automatic generation
 // of connect proxy expose path configuration based on configured consul checks.
 // To qualify, the check must be of type "http" or "grpc", and must have a Path

nomad/job_endpoint_hook_expose_check_test.go

@@ -13,34 +13,6 @@ func TestJobExposeCheckHook_Name(t *testing.T) {
 	require.Equal(t, "expose-check", new(jobExposeCheckHook).Name())
 }
 
-func TestJobExposeCheckHook_serviceUsesConnectEnvoy(t *testing.T) {
-	t.Parallel()
-
-	t.Run("connect is nil", func(t *testing.T) {
-		require.False(t, serviceUsesConnectEnvoy(&structs.Service{
-			Connect: nil,
-		}))
-	})
-
-	t.Run("sidecar-task is overridden", func(t *testing.T) {
-		require.False(t, serviceUsesConnectEnvoy(&structs.Service{
-			Connect: &structs.ConsulConnect{
-				SidecarTask: &structs.SidecarTask{
-					Name: "my-sidecar",
-				},
-			},
-		}))
-	})
-
-	t.Run("sidecar-task is nil", func(t *testing.T) {
-		require.True(t, serviceUsesConnectEnvoy(&structs.Service{
-			Connect: &structs.ConsulConnect{
-				SidecarTask: nil,
-			},
-		}))
-	})
-}
-
 func TestJobExposeCheckHook_tgUsesExposeCheck(t *testing.T) {
 	t.Parallel()
 
@@ -135,7 +107,7 @@ func TestJobExposeCheckHook_tgValidateUseOfCheckExpose(t *testing.T) {
 		require.EqualError(t, tgValidateUseOfCheckExpose(&structs.TaskGroup{
 			Name:     "g1",
 			Services: []*structs.Service{withCustomProxyTask},
-		}), `exposed service check g1->s1->s1-check1 requires use of Nomad's builtin Connect proxy`)
+		}), `exposed service check g1->s1->s1-check1 requires use of sidecar_proxy`)
 	})
 
 	t.Run("group-service uses custom proxy but no expose", func(t *testing.T) {
@@ -223,8 +195,10 @@ func TestJobExposeCheckHook_Validate(t *testing.T) {
 					Mode: "bridge",
 				}},
 				Services: []*structs.Service{{
-					Name:    "s1",
-					Connect: &structs.ConsulConnect{},
+					Name: "s1",
+					Connect: &structs.ConsulConnect{
+						SidecarService: &structs.ConsulSidecarService{},
+					},
 					Checks: []*structs.ServiceCheck{{
 						Name:   "check1",
 						Type:   "http",

website/content/docs/job-specification/expose.mdx

@@ -65,7 +65,7 @@ job "expose-check-example" {
       driver = "docker"
 
       config {
-        image = "hashicorpnomad/counter-api:v2"
+        image = "hashicorpnomad/counter-api:v3"
       }
     }
   }
@@ -123,7 +123,7 @@ job "expose-example" {
       driver = "docker"
 
       config {
-        image = "hashicorpnomad/counter-api:v2"
+        image = "hashicorpnomad/counter-api:v3"
       }
 
       # e.g. reference ${NOMAD_PORT_api_expose_healthcheck} for other uses