connect: use deterministic injected dynamic exposed port

This PR uses the checksum of the check for which a dynamic exposed
port is being generated (instead of a UUID prefix) so that the
generated port label is deterministic.

This fixes 2 bugs:
 - 'job plan' output is now idempotent for jobs making use of injected ports
 - tasks will no longer be destructively updated when jobs making use of
   injected ports are re-run without changing any user specified part of
   job config.

Closes: https://github.com/hashicorp/nomad/issues/10099

CHANGELOG.md

@@ -52,6 +52,7 @@ BUG FIXES:
  * cli: Remove extra linefeeds in monitor.log files written by `nomad operator debug`. [[GH-10252](https://github.com/hashicorp/nomad/issues/10252)]
  * client: Fixed log formatting when killing tasks. [[GH-10135](https://github.com/hashicorp/nomad/issues/10135)]
  * client: Fixed a bug where small files would be assigned the wrong content type. [[GH-10348](https://github.com/hashicorp/nomad/pull/10348)]
+ * consul/connect: Fixed a bug where job plan always different when using expose checks. [[GH-10492](https://github.com/hashicorp/nomad/pull/10492)]
  * consul/connect: Fixed a bug where HTTP ingress gateways could not use wildcard names. [[GH-10457](https://github.com/hashicorp/nomad/pull/10457)]
  * csi: Fixed a bug where volume with IDs that are a substring prefix of another volume could use the wrong volume for feasibility checking. [[GH-10158](https://github.com/hashicorp/nomad/issues/10158)]
  * scheduler: Fixed a bug where Nomad reports negative or incorrect running children counts for periodic jobs. [[GH-10145](https://github.com/hashicorp/nomad/issues/10145)]

nomad/job_endpoint_hook_expose_check.go

@@ -5,7 +5,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/pkg/errors"
 )
@@ -22,7 +21,7 @@ func (jobExposeCheckHook) Name() string {
 func (jobExposeCheckHook) Mutate(job *structs.Job) (_ *structs.Job, warnings []error, err error) {
 	for _, tg := range job.TaskGroups {
 		for _, s := range tg.Services {
-			for _, c := range s.Checks {
+			for i, c := range s.Checks {
 				if c.Expose {
 					// TG isn't validated yet, but validation
 					// may depend on mutation results.
@@ -33,7 +32,7 @@ func (jobExposeCheckHook) Mutate(job *structs.Job) (_ *structs.Job, warnings []e
 						continue
 					}
 
-					if exposePath, err := exposePathForCheck(tg, s, c); err != nil {
+					if exposePath, err := exposePathForCheck(tg, s, c, i); err != nil {
 						return nil, nil, err
 					} else if exposePath != nil {
 						serviceExposeConfig := serviceExposeConfig(s)
@@ -180,7 +179,7 @@ func checkIsExposable(check *structs.ServiceCheck) bool {
 // exposePathForCheck extrapolates the necessary expose path configuration for
 // the given consul service check. If the check is not compatible, nil is
 // returned.
-func exposePathForCheck(tg *structs.TaskGroup, s *structs.Service, check *structs.ServiceCheck) (*structs.ConsulExposePath, error) {
+func exposePathForCheck(tg *structs.TaskGroup, s *structs.Service, check *structs.ServiceCheck, i int) (*structs.ConsulExposePath, error) {
 	if !checkIsExposable(check) {
 		return nil, nil
 	}
@@ -197,8 +196,15 @@ func exposePathForCheck(tg *structs.TaskGroup, s *structs.Service, check *struct
 	//
 	// This lets PortLabel be optional for any exposed check.
 	if check.PortLabel == "" {
+
+		// Note: because the check label is not set yet, and we want to create a
+		// deterministic label based on the check itself, use the index of the check
+		// on the service as part of the service name as input into Hash, ensuring
+		// the hash for the check is unique.
+		suffix := check.Hash(fmt.Sprintf("%s_%d", s.Name, i))[:6]
 		port := structs.Port{
-			Label: fmt.Sprintf("svc_%s_ck_%s", s.Name, uuid.Generate()[:6]),
+			HostNetwork: "default",
+			Label:       fmt.Sprintf("svc_%s_ck_%s", s.Name, suffix),
 			To:          -1,
 		}
 

nomad/job_endpoint_hook_expose_check_test.go

@@ -226,6 +226,8 @@ func TestJobExposeCheckHook_Validate(t *testing.T) {
 func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
 	t.Parallel()
 
+	const checkIdx = 0
+
 	t.Run("not expose compatible", func(t *testing.T) {
 		c := &structs.ServiceCheck{
 			Type: "tcp", // not expose compatible
@@ -235,7 +237,7 @@ func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
 		}
 		ePath, err := exposePathForCheck(&structs.TaskGroup{
 			Services: []*structs.Service{s},
-		}, s, c)
+		}, s, c, checkIdx)
 		require.NoError(t, err)
 		require.Nil(t, ePath)
 	})
@@ -255,7 +257,7 @@ func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
 		ePath, err := exposePathForCheck(&structs.TaskGroup{
 			Name:     "group1",
 			Services: []*structs.Service{s},
-		}, s, c)
+		}, s, c, checkIdx)
 		require.NoError(t, err)
 		require.Equal(t, &structs.ConsulExposePath{
 			Path:          "/health",
@@ -286,7 +288,7 @@ func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
 					{Label: "sPort", Value: 4000},
 				},
 			}},
-		}, s, c)
+		}, s, c, checkIdx)
 		require.NoError(t, err)
 		require.Equal(t, &structs.ConsulExposePath{
 			Path:          "/health",
@@ -317,11 +319,12 @@ func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
 					// service declares "sPort", but does not exist
 				},
 			}},
-		}, s, c)
+		}, s, c, checkIdx)
 		require.EqualError(t, err, `unable to determine local service port for service check group1->service1->check1`)
 	})
 
 	t.Run("empty check port", func(t *testing.T) {
+		setup := func() (*structs.TaskGroup, *structs.Service, *structs.ServiceCheck) {
 			c := &structs.ServiceCheck{
 				Name: "check1",
 				Type: "http",
@@ -340,15 +343,35 @@ func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
 					DynamicPorts: []structs.Port{},
 				}},
 			}
-		ePath, err := exposePathForCheck(tg, s, c)
+			return tg, s, c
+		}
+
+		tg, s, c := setup()
+		ePath, err := exposePathForCheck(tg, s, c, checkIdx)
 		require.NoError(t, err)
 		require.Len(t, tg.Networks[0].DynamicPorts, 1)
+		require.Equal(t, "default", tg.Networks[0].DynamicPorts[0].HostNetwork)
+		require.Equal(t, "svc_", tg.Networks[0].DynamicPorts[0].Label[0:4])
 		require.Equal(t, &structs.ConsulExposePath{
 			Path:          "/health",
 			Protocol:      "",
 			LocalPathPort: 9999,
 			ListenerPort:  tg.Networks[0].DynamicPorts[0].Label,
 		}, ePath)
+
+		t.Run("deterministic generated port label", func(t *testing.T) {
+			tg2, s2, c2 := setup()
+			ePath2, err2 := exposePathForCheck(tg2, s2, c2, checkIdx)
+			require.NoError(t, err2)
+			require.Equal(t, ePath, ePath2)
+		})
+
+		t.Run("unique on check index", func(t *testing.T) {
+			tg3, s3, c3 := setup()
+			ePath3, err3 := exposePathForCheck(tg3, s3, c3, checkIdx+1)
+			require.NoError(t, err3)
+			require.NotEqual(t, ePath.ListenerPort, ePath3.ListenerPort)
+		})
 	})
 
 	t.Run("missing network with no service check port label", func(t *testing.T) {
@@ -370,7 +393,7 @@ func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
 			Services: []*structs.Service{s},
 			Networks: nil, // not set, should cause validation error
 		}
-		ePath, err := exposePathForCheck(tg, s, c)
+		ePath, err := exposePathForCheck(tg, s, c, checkIdx)
 		require.EqualError(t, err, `group "group1" must specify one bridge network for exposing service check(s)`)
 		require.Nil(t, ePath)
 	})