docs: add documentation for proxy.expose configuration #7440
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shoenig
added a commit
that referenced
this pull request
Mar 26, 2020
…hecks 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.
shoenig
force-pushed
the
docs-connect-expose
branch
2 times, most recently
from
f739083
to
dcbfa67
Compare
shoenig
changed the base branch from
f-connect-expose-checks-auto
to
f-connect-expose-checks-percheck
shoenig
added a commit
that referenced
this pull request
Mar 30, 2020
…hecks 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.
shoenig
force-pushed
the
f-connect-expose-checks-percheck
branch
from
2b43a44
to
3223686
Compare
shoenig
force-pushed
the
docs-connect-expose
branch
from
dcbfa67
to
a1a49d4
Compare
shoenig
added a commit
that referenced
this pull request
Mar 31, 2020
…hecks 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.
shoenig
force-pushed
the
f-connect-expose-checks-percheck
branch
from
0722229
to
df417f7
Compare
|
@shoenig I think the diff got wonky after merging 7515, mind taking a look? I'm guessing changing the base branch back to master should fix. |
This changeset adds documentation changes for the new `proxy.expose` stanza as well as the `check.expose` parameter. The examples are centered around proposed changes for the "countdash" `dashboard-service` in [demo-consul-101](github.com/hashicorp/demo-consul-101/pull/6). The dashboard service will now serve two additonal endpoints - `/health/api` - `/metrics` which should serve nicely as expose-able paths.
shoenig
force-pushed
the
docs-connect-expose
branch
from
a1a49d4
to
44f884e
Compare
|
Ah sorry @nickethier , it should be fixed now! |
nickethier
approved these changes
Apr 1, 2020
| network { | ||
| mode = "bridge" | ||
| port "expose" { | ||
| to = -1 |
There was a problem hiding this comment.
❓ Should we just make it such that if mode != "host" then any not set to port will automatically be the port value? Maybe not as part of this PR but just a thought, I don't love exposing (sorry bad pun) users to the -1 special value.
There was a problem hiding this comment.
Ha, @schmichael and I chatted about this and figured maybe you knew the reason it was done this way =\
I'll add this to our fortnightly to reconsider
| Usually the same as the parent service's port, it is useful to customize in clusters with mixed | ||
| Connect and non-Connect services | ||
| - `upstreams` <code>([upstreams][]: nil)</code> - Used to configure details of each upstream service that | ||
| this sidecar proxy communicates with. | ||
| - `config` <code>(map: nil)</code> - Proxy configuration that's opaque to Nomad and | ||
| - `expose` <code>([expose]: nil)</code> - Used to configure expose path configuration for Envoy. |
There was a problem hiding this comment.
Suggested change
| - `expose` <code>([expose]: nil)</code> - Used to configure expose path configuration for Envoy. | |
| - `expose` `([expose]: nil)` - Used to configure expose path configuration for Envoy. |
There was a problem hiding this comment.
Without the <code> tags, the expose name isn't rendered as a link to the expose doc page like we want it to. Similar story with upstreams above. I think it has something to do with the brackets, but /shrug
| information about Expose Path configurations for Envoy can be found in Consul's | ||
| [Expose Paths Configuration Reference](https://www.consul.io/docs/connect/registration/service-registration.html#expose-paths-configuration-reference). | ||
|
|
||
| Service [check](https://nomadproject.io/docs/job-specification/service/#check-parameters) |
There was a problem hiding this comment.
I think we should add a check to the example below to help call attention to this.
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #6120
This changeset adds documentation changes for
proxy.exposestanzaas well as the
check.exposeparameter.The examples are centered around proposed changes for the "countdash"
dashboard-servicein demo-consul-101.The dashboard service will now serve two additional endpoints
/health/api/metricswhich should serve nicely as expose-able paths.