Skip to content
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

cli: report server version #9159

Conversation

drewgonzales360
Copy link
Contributor

@drewgonzales360 drewgonzales360 commented Oct 22, 2020

Issue #1040

This is my first PR here 😄

Summary

Originally, I was going to set a X-Nomad-Version on every request, then check the header on every request to any API, but I didn't think it was very clean and it would be a big changeset. I also thought it might get annoying for users to see the warning for several commands.

Instead, I just added the revision to nomad server members -detailed.

Name          Address    Port  Tags
linux.global  127.0.0.1  4648  port=4647,raft_vsn=3,role=nomad,revision=3bf31ce64e762641787d8cc2c6a46a36a90816b6+CHANGES,bootstrap=1,expect=1,dc=dc1,build=1.2.6-dev,region=global,rpc_addr=127.0.0.1,id=94139a45-1000-023c-c809-8e9434ace143

setVersion adds a header X-Nomad-Version to a wrapper function. There is
a wrapper function in http.go that takes a normal implementation for an
endpoint, and adds other code that should be there. As an example, let's
say that an endpoint wants to search for all the jobs running. The
function would implement the logic to look for jobs. Then you take that
function, pass it to the wrapper, and the wrapper will add all the
headers that should be on that response.

It's nice because all of the logic that you'd have to add version
headers can get put in one place
Also fix the build. You need to call go mod vendor when changing the
api directory
@hashicorp-cla
Copy link

hashicorp-cla commented Oct 22, 2020

CLA assistant check
All committers have signed the CLA.

@drewgonzales360
Copy link
Contributor Author

@nickethier 👋 Could you take a quick look at this? I've still got this as a draft, but I wanted to get some early feedback before I dive into this more. Thanks!

@vercel vercel bot temporarily deployed to Preview – nomad October 17, 2021 04:50 Inactive
Copy link
Contributor Author

@drewgonzales360 drewgonzales360 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okidokes, I think I understand more of it now 😸. I'm adding a bunch of comments to check my understanding of stuff.

command/agent/agent.go Show resolved Hide resolved
nomad/server.go Show resolved Hide resolved
nomad/config.go Show resolved Hide resolved
command/version.go Outdated Show resolved Hide resolved
@drewgonzales360
Copy link
Contributor Author

Shoot, I forgot I left this open. @tgross does this make sense?

@tgross
Copy link
Member

tgross commented Nov 7, 2022

Shoot, I forgot I left this open. @tgross does this make sense?

Hi @drewgonzales360! I think so. Let me build it and take it for a spin to make sure that I'm clear on all places this gets picked up.

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I tested this out locally with a build of 1.4.2 and then did a rolling update with this patch, just to verify nothing unexpected during the upgrade process:

Before:

$ nomad server members -detailed
Name            Address        Port  Status  Leader  Protocol  Raft Version  Build  Datacenter  Region  Tags
server0.global  192.168.1.151  4648  alive   true    2         3             1.4.2  dc1         global  rpc_addr=192.168.1.151,dc=dc1,role=nomad,build=1.4.2,raft_vsn=3,region=global,expect=3,id=b762289c-b200-7017-e6ea-bde963599fa0,port=4647,vsn=1
server1.global  192.168.1.151  4658  alive   false   2         3             1.4.2  dc1         global  port=4657,role=nomad,region=global,id=a778c12c-d259-df16-8ae6-f7b2a58a929b,expect=3,build=1.4.2,vsn=1,raft_vsn=3,dc=dc1,rpc_addr=192.168.1.151
server2.global  192.168.1.151  4668  alive   false   2         3             1.4.2  dc1         global  dc=dc1,vsn=1,raft_vsn=3,role=nomad,region=global,id=ef79077f-e492-e20c-a80e-ea9e0b6ee43f,expect=3,port=4667,build=1.4.2,rpc_addr=192.168.1.151

Upgrade a follower:

$ nomad server members -detailed
Name            Address        Port  Status  Leader  Protocol  Raft Version  Build      Datacenter  Region  Tags
server0.global  192.168.1.151  4648  alive   true    2         3             1.4.2      dc1         global  raft_vsn=3,expect=3,port=4647,vsn=1,build=1.4.2,dc=dc1,id=b762289c-b200-7017-e6ea-bde963599fa0,role=nomad,region=global,rpc_addr=192.168.1.151
server1.global  192.168.1.151  4658  alive   false   2         3             1.4.2      dc1         global  vsn=1,expect=3,build=1.4.2,role=nomad,region=global,id=a778c12c-d259-df16-8ae6-f7b2a58a929b,dc=dc1,raft_vsn=3,rpc_addr=192.168.1.151,port=4657
server2.global  192.168.1.151  4668  alive   false   2         3             1.4.1-dev  dc1         global  raft_vsn=3,id=ef79077f-e492-e20c-a80e-ea9e0b6ee43f,region=global,build=1.4.1-dev,port=4667,role=nomad,dc=dc1,expect=3,revision=2f7ef2de9935f4e36f676da17f787740fca82dc7,vsn=1,rpc_addr=192.168.1.151

Upgrade the leader:

$ nomad server members -detailed
Name            Address        Port  Status  Leader  Protocol  Raft Version  Build      Datacenter  Region  Tags
server0.global  192.168.1.151  4648  alive   false   2         3             1.4.1-dev  dc1         global  dc=dc1,revision=2f7ef2de9935f4e36f676da17f787740fca82dc7,id=b762289c-b200-7017-e6ea-bde963599fa0,build=1.4.1-dev,vsn=1,raft_vsn=3,expect=3,role=nomad,port=4647,rpc_addr=192.168.1.151,region=global
server1.global  192.168.1.151  4658  alive   false   2         3             1.4.2      dc1         global  id=a778c12c-d259-df16-8ae6-f7b2a58a929b,port=4657,raft_vsn=3,expect=3,region=global,vsn=1,role=nomad,dc=dc1,build=1.4.2,rpc_addr=192.168.1.151
server2.global  192.168.1.151  4668  alive   true    2         3             1.4.1-dev  dc1         global  dc=dc1,build=1.4.1-dev,port=4667,raft_vsn=3,id=ef79077f-e492-e20c-a80e-ea9e0b6ee43f,region=global,revision=2f7ef2de9935f4e36f676da17f787740fca82dc7,vsn=1,rpc_addr=192.168.1.151,role=nomad,expect=3

Upgrade the remaining follower:

$ nomad server members -detailed
Name            Address        Port  Status  Leader  Protocol  Raft Version  Build      Datacenter  Region  Tags
server0.global  192.168.1.151  4648  alive   false   2         3             1.4.1-dev  dc1         global  rpc_addr=192.168.1.151,region=global,build=1.4.1-dev,role=nomad,id=b762289c-b200-7017-e6ea-bde963599fa0,revision=2f7ef2de9935f4e36f676da17f787740fca82dc7,port=4647,vsn=1,raft_vsn=3,expect=3,dc=dc1
server1.global  192.168.1.151  4658  alive   false   2         3             1.4.1-dev  dc1         global  build=1.4.1-dev,vsn=1,rpc_addr=192.168.1.151,role=nomad,raft_vsn=3,region=global,revision=2f7ef2de9935f4e36f676da17f787740fca82dc7,dc=dc1,port=4657,id=a778c12c-d259-df16-8ae6-f7b2a58a929b,expect=3
server2.global  192.168.1.151  4668  alive   true    2         3             1.4.1-dev  dc1         global  role=nomad,dc=dc1,region=global,expect=3,rpc_addr=192.168.1.151,build=1.4.1-dev,port=4667,raft_vsn=3,revision=2f7ef2de9935f4e36f676da17f787740fca82dc7,vsn=1,id=ef79077f-e492-e20c-a80e-ea9e0b6ee43f

I've tweaked the changelog entry slightly and pushed that. Will merge once CI is done and then this will ship in the next regular release of Nomad (most likely 1.4.3). Thanks so much for your patience on this one @drewgonzales360!

@tgross tgross added this to the 1.4.3 milestone Nov 7, 2022
@tgross tgross merged commit 1a1ce36 into hashicorp:main Nov 7, 2022
Nomad - Community Issues Triage automation moved this from In Progress to Done Nov 7, 2022
@tgross tgross added the backport/1.4.x backport to 1.4.x release line label Nov 7, 2022
@tgross
Copy link
Member

tgross commented Nov 7, 2022

The automated backport to the 1.4.x branch failed, but that's just internal process nonsense so I'll get that fixed up and merged.

tgross pushed a commit that referenced this pull request Nov 7, 2022
tgross pushed a commit that referenced this pull request Nov 7, 2022
Co-authored-by: Drew Gonzales <6912711+drewgonzales360@users.noreply.github.com>
@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.4.x backport to 1.4.x release line
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants