Skip to content

feat(nodeadm): support for nodeadm version display#2321

Open
guessi wants to merge 2 commits intoawslabs:mainfrom
guessi:nodeadm-version-info
Open

feat(nodeadm): support for nodeadm version display#2321
guessi wants to merge 2 commits intoawslabs:mainfrom
guessi:nodeadm-version-info

Conversation

@guessi
Copy link
Copy Markdown
Member

@guessi guessi commented Jul 7, 2025

Issue #, if available:

Description of changes:

By current design, there have no version support for nodeadm, so it would inherit the setup from integrii/flaggy

// defaultVersion is applied to parsers when they are created
const defaultVersion = "0.0.0"

With this PR change, not only make it possible to identify which version it is about, but also help customer to identify which nodeadm it is running with.

Cross ref: aws/eks-hybrid#639 and aws/eks-hybrid#640

Testing Done

Before change:

$ /usr/bin/nodeadm version
    Version: 0.0.0

After change:

$ ./_bin/nodeadm --version
    Version: v20250627-7-ged80709a

Copy link
Copy Markdown
Contributor

@ndbaker1 ndbaker1 left a comment

Choose a reason for hiding this comment

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

that's a nice idea, ill see if its plumbed through correctly in today's process

@guessi guessi force-pushed the nodeadm-version-info branch from ed80709 to 8dfbd18 Compare July 8, 2025 00:13
@guessi guessi requested a review from ndbaker1 July 8, 2025 00:14
@guessi
Copy link
Copy Markdown
Member Author

guessi commented Jul 17, 2025

Notice merge conflicts, let me fix it.

@guessi guessi force-pushed the nodeadm-version-info branch from 8dfbd18 to 6534ca0 Compare July 17, 2025 00:23
@guessi
Copy link
Copy Markdown
Member Author

guessi commented Jul 17, 2025

Test after rebased

With no tag

% make build
GOOS= go build -ldflags="-X 'main.gitTagVersion=v20250715-6-g6534ca01'" -o <redacted>/_bin/ ./cmd/...
% ./_bin/nodeadm --version
Version: v20250715-6-g6534ca01

With tag (for test only, not actually a release)

% git tag v20250717
% git describe --exact-match --tags
v20250717
% make build
GOOS= go build -ldflags="-X 'main.gitTagVersion=v20250717'" -o <redacted>/_bin/ ./cmd/...
% ./_bin/nodeadm --version
Version: v20250717

nodeadm/Makefile Outdated
endif

# Version from git tags
VERSION ?= $(shell git describe --tags --abbrev=8 --always)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unfortunately, this won't work with our current setup. The nodeadm build happens in a container as a part of the AMI build, and there is no .git directory in that workspace. Additionally, the git tag for an release does not actually exist when the AMI build happens. We could wire something up, but it would have to happen in our internal release tooling.

We could do this as a pre-req without changing behavior, and then set the version in a future release:

Suggested change
VERSION ?= $(shell git describe --tags --abbrev=8 --always)
VERSION ?= 0.0.0

Copy link
Copy Markdown
Member Author

@guessi guessi Jul 17, 2025

Choose a reason for hiding this comment

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

Umm... interesting, sounds like we need somethings like sed/awk/...

Never mind, I just made updates, and I can see could be done by...

% VERSION=1.2.3 make build
GOOS= go build -ldflags="-X 'main.version=1.2.3'" -o <redacted>/_bin/ ./cmd/...`
% ./_bin/nodeadm --version
Version: 1.2.3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah exactly, we can pass the correct version with our internal tooling

@guessi guessi force-pushed the nodeadm-version-info branch from 6534ca0 to 8caa01d Compare July 17, 2025 00:45
@guessi guessi requested a review from cartermckinnon July 17, 2025 00:49
@guessi
Copy link
Copy Markdown
Member Author

guessi commented Jul 17, 2025

Just made an update to support both nodeadm and nodeadm-internal.

% VERSION=v1.99.88 make build
% ./_bin/nodeadm --version
Version: v1.99.88
% ./_bin/nodeadm-internal --version
Version: v1.99.88

@github-actions
Copy link
Copy Markdown
Contributor

This pull request is stale because it has been open for 60 days with no activity. Remove the stale label or comment to avoid closure in 14 days

@github-actions github-actions bot added the Stale label Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants