connect: enable automatic expose paths for TG service checks

[shoenig]

Part of #6120

Building on the support for enabling connect proxy paths in #7323, this change
adds the ability to configure the 'service.check.expose' flag on group-level
service check definitions for services that are connect-enabled. This is a slight
deviation from the "magic" that Consul provides. With Consul, the 'expose' flag
exists on the connect.proxy stanza, which will then auto-generate expose paths
for every HTTP and gRPC service check associated with that connect-enabled
service.

A first attempt at providing similar magic for Nomad's Consul Connect integration
followed that pattern exactly, as seen in #7396. However, on reviewing the PR
we realized having the expose flag on the proxy stanza inseperably ties together
the automatic path generation with every HTTP/gRPC defined on the service. This
makes sense in Consul's context, because a service definition is reasonably
associated with a single "task". With Nomad's group level service definitions
however, there is a reasonable expectation that a service definition is more
abstractly representative of multiple services within the task group. In this
case, one would want to define checks of that service which concretely make HTTP
or gRPC requests to different underlying tasks. Such a model is not possible
with the course proxy.expose flag.

Instead, we now have the flag made available within the check definitions themselves.
By making the expose feature resolute to each check, it is possible to have
some HTTP/gRPC checks which make use of the envoy exposed paths, as well as
some HTTP/gRPC checks which make use of some orthongonal port-mapping to do
checks on some other task (or even some other bound port of the same task)
within the task group.

Given this example,

group "server-group" {
  network {
    mode = "bridge"
    port "forchecks" {
      to = -1
    }
  }

  service {
    name = "myserver"
    port = 2000

    connect {
      sidecar_service {
      }
    }

    check {
      name     = "mycheck-myserver"
      type     = "http"
      port     = "forchecks"
      interval = "3s"
      timeout  = "2s"
      method   = "GET"
      path     = "/classic/responder/health"
      expose   = true
    }
  }
}

Nomad will automatically inject (via job endpoint mutator) the
extrapolated expose path configuration, i.e.

expose {
  path {
    path            = "/classic/responder/health"
    protocol        = "http"
    local_path_port = 2000
    listener_port   = "forchecks"
  }
}

Documentation is coming in #7440 (needs updating, doing next)

Modifications to the countdash examples in hashicorp/demo-consul-101#6
which will make the examples in the documentation actually runnable.

Will add some e2e tests based on the above when it becomes available.

[schmichael]

LGTM! I like this fine grained approach a lot.

[schmichael]

Maybe switch this to if !c.Expose { continue } to save at least one level of indentation for readability.

[nickethier]

I've had to do this a few times and groan each time. What do you folks think about adding helper funcs where fitting to make this a bit more readable. Something like:

job.WalkTGServiceChecks(func(tg *structs.TaskGroup, s *struct.Service, c *struct.ServiceCheck){
	if !c.Expose { return }
	...
})

[shoenig]

I was having similar thoughts about having a walk func - if we go that route we'd probably want to enable some err control similar to what filepath.Walk provides

[schmichael]

Since the Check.Expose field only exists in Nomad do we need to include it in the hash which is used for change detection? Aren't the generated Connect stanza changes sufficient to trigger the update?

I don't think it matters functionally either way, just curious if I'm understanding things.

[shoenig]

Ah that's a great point; yeah the Hash is only used for diffing against Consul. Within Nomad we use Equals and diff.go. I'll comment this out and leave a reminder.

[nickethier]

Maybe remove this line to catch testing that expose defaults to false

[nickethier]

I've had to do this a few times and groan each time. What do you folks think about adding helper funcs where fitting to make this a bit more readable. Something like:

job.WalkTGServiceChecks(func(tg *structs.TaskGroup, s *struct.Service, c *struct.ServiceCheck){
	if !c.Expose { return }
	...
})

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.