This repository was archived by the owner on Jan 8, 2024. It is now read-only.
Server install: Nomad service discovery #3500
Merged
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
paladin-devops
force-pushed
the
b-nomad-install-service-discovery
branch
from
0975d23 to
cc407d1
Compare
paladin-devops
force-pushed
the
b-nomad-install-service-discovery
branch
from
cc407d1 to
3b7ed16
Compare
briancain
suggested changes
Jun 28, 2022
evanphx
reviewed
Jun 29, 2022
paladin-devops
force-pushed
the
f-runner-install
branch
from
43444e2 to
9829fb8
Compare
paladin-devops
force-pushed
the
b-nomad-install-service-discovery
branch
from
3b7ed16 to
f158d5d
Compare
paladin-devops
force-pushed
the
b-nomad-install-service-discovery
branch
from
3fbc16c to
5ca33df
Compare
paladin-devops
added
the
backport/0.9.x
paladin-devops
force-pushed
the
b-nomad-install-service-discovery
branch
from
0a3218a to
410f082
Compare
briancain
approved these changes
Jul 26, 2022
izaaklauer
approved these changes
Jul 26, 2022
| @@ -286,19 +255,18 @@ func (i *NomadInstaller) Install( | |||
| s.Update("Waypoint server ready") | |||
| s.Done() | |||
|
|
|||
| if i.config.consulService { | |||
| s = sg.Add("The CLI has been configured to automatically install a Consul service for\n" + | |||
| if i.config.serviceProvider == "consul" && i.config.consulService { | |||
There was a problem hiding this comment.
I recall from a code review meeting that this was supposed to be an "or" instead of an "and"?
There was a problem hiding this comment.
@xiaolin-ninja since the default values are "consul" and true for serviceProvider and consulService, respectively, we want this to be "and" because if the user sets something other than "consul" for serviceProvider, or sets consulService to false, we then do not want to create the job with a Consul service.
catsby
reviewed
Jul 26, 2022
|
You might need to rebase off |
…onsul as a service provider.
…nfo on Nomad install.
…e old and new flags to configure service discovery.
…installs and upgrades.
… and prefer Consul tag flags for backwards compatibility.
This prompt is added so that a user doesn't inadvertently remove service discovery from their Waypoint installation.
paladin-devops
force-pushed
the
b-nomad-install-service-discovery
branch
from
dcf17fc to
888d138
Compare
|
@briancain so rebased! 😄 |
catsby
approved these changes
Aug 5, 2022
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.
This PR updates the server installation for Nomad to support service discovery for Nomad or Consul by specifying the service discovery provider as a flag for the
waypoint installcommand.Closes #3376.
NOTE: The old config,
-nomad-consul-service, is still supported with this PR update, but it should be phased out eventually. The new-nomad-service-providerconfig supports pre- and post-1.3.0 Waypoint installs to Nomad.