-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: improve some connect examples #7891
Conversation
This change replaces the top example for expose path configuration with two new runnable examples. Users should be able to copy and paste those jobs into a job file and run them against a basic connect enabled nomad setup. The example presented first demonstrates use of the service check expose parameter with no dynamic port explicitly defined (new to 0.11.2). This is expected to be the "90%" use case of users, and so we should try to emphasise this pattern as best practice. The example presented second demonstrates achieving the same goal as the first exmaple, but utilizing the full plumbing available through the `connect.proxy.expose` stanza. This should help readers comprehend what is happening "under the hood".
Replace the existing top example with something that is directly runnable on a `-dev-connect` nomad setup. Add the _complete_ `countdash` example at the bottom in the examples section, so that people do not need to go guide-hunting to find a complete example. The hope is people will see more runnable examples and be less afraid of connect.
Promote the Connect ACLs guide on the jobspec connect stanza docs page. This was suggested in a ticket after someone got stuck not realizing they needed to enable Consul Intentions for their connect enabled services, which is covered in the guide.
A few connect examples reference a fake 'test/test' image. By replacing those with `hashicorpnomad/counter-api` we can turn them into runnable examples.
} | ||
``` | ||
|
||
For uses other than Consul service checks, use the `expose` configuration in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this text is suggesting that there's some non-check related use case but then the example is just another health check. Might someone use expose
for allowing ingress traffic to the workload? If so, maybe we could call that out specifically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think ingressing traffic is the target usecase. Usually you would expose things like a prometheus endpoint so that the host agent could scrape metrics for example.
network { | ||
mode = "bridge" | ||
port "expose" { | ||
|
||
port "api_expose_healthcheck" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we planning to publish these docs prior to 0.11.2? Would be nice to drop this since #7800 has been merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was kind of trying to make this example serve double-duty
- show what the generated plumbing for service checks look like
- show how a user would manually reference the port for an exposed path
The usefulness of 2 is dubious, certainly in the context of countdash. I'll update this to also use auto dynamic ports.
Wait no sorry, the generated port only works for check.expose = true
. Since this example demonstrates expose.path
, it still needs the manually defined port for expose.path.listener_port
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. |
Make some examples around connect/expose more complete (i.e. runnable).
Take advantage of the automatic dynamic ports for exposed service checks.
Tweak some wording to be more clear.
Run everything through
hclfmt
.