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.

Sign up for GitHub

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

🐛 Fix permanent change to Kubernetes version when running make generate #4612

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

🐛 Fix permanent change to Kubernetes version when running make generate #4612

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ help: ## Display this help

##@ Build

K8S_VERSION ?= $(shell go list -m -modfile=./testdata/project-v4/go.mod -f "{{ .Version }}" k8s.io/api | awk -F'[v.]' '{printf "1.%d.%d", $$3, $$4}')

LD_FLAGS=-ldflags " \
-X sigs.k8s.io/kubebuilder/v4/cmd.kubeBuilderVersion=$(shell git describe --tags --dirty --broken) \
-X sigs.k8s.io/kubebuilder/v4/cmd.kubernetesVendorVersion=$(K8S_VERSION) \
Copy link
Member

@camilamacedo86 camilamacedo86 Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @vitorfloriano,

Our goal is to ensure that the binary always uses the Kubernetes version defined in the go.mod file of the projects built with it (e.g., project-v4), so that we don't need to pass any additional values manually.

Given that, the change you made above makes sense for me, But this one will be called when we run make install right? So, we need to run make install and check the bin version after the change.

Copy link
Member

@camilamacedo86 camilamacedo86 Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Test for this one:

$ make install 
go build -ldflags " -X sigs.k8s.io/kubebuilder/v4/cmd.kubeBuilderVersion=v4.5.0-99-g4c0b8d641 -X sigs.k8s.io/kubebuilder/v4/cmd.goos=darwin -X sigs.k8s.io/kubebuilder/v4/cmd.goarch=arm64 -X sigs.k8s.io/kubebuilder/v4/cmd.gitCommit=4c0b8d64107a1c2beae6ae6c83ca63d5393cde22 -X sigs.k8s.io/kubebuilder/v4/cmd.buildDate=2025-03-17T11:25:36Z " -o bin/kubebuilder
rm -f /Users/camilam/go/bin/kubebuilder
cp ./bin/kubebuilder /Users/camilam/go/bin/kubebuilder

----- Now to check that all still fine ---

$ kubebuilder version
Version: main.version{KubeBuilderVersion:"4.5.1", KubernetesVendor:"1.32.1", GitCommit:"0ace7a8753c52b35014e43edc2a0b0454b78e769", BuildDate:"2025-02-21T20:16:18Z", GoOs:"darwin", GoArch:"arm64"}

The flags are called here:

kubebuilder/Makefile

Lines 56 to 58 in 4c0b8d6

build: ## Build the project locally
go build $(LD_FLAGS) -o bin/kubebuilder

-X sigs.k8s.io/kubebuilder/v4/cmd.goos=$(shell go env GOOS) \
-X sigs.k8s.io/kubebuilder/v4/cmd.goarch=$(shell go env GOARCH) \
-X sigs.k8s.io/kubebuilder/v4/cmd.gitCommit=$(shell git rev-parse HEAD) \
Expand Down Expand Up @@ -201,13 +204,10 @@ install-helm: ## Install the latest version of Helm locally
helm-lint: install-helm ## Lint the Helm chart in testdata
helm lint testdata/project-v4-with-plugins/dist/chart

K8S_VERSION ?= $(shell go list -m -modfile=./testdata/project-v4/go.mod -f "{{ .Version }}" k8s.io/api | awk -F'[v.]' '{printf "1.%d.%d", $$3, $$4}')
.PHONY: update-k8s-version
update-k8s-version: ## Update Kubernetes API version in version.go and .goreleaser.yml
update-k8s-version: ## Update Kubernetes API version in .goreleaser.yml
@if [ -z "$(K8S_VERSION)" ]; then echo "Error: K8S_VERSION is empty"; exit 1; fi
@echo "Updating Kubernetes version to $(K8S_VERSION)"
@# Update version.go
@sed -i.bak 's/kubernetesVendorVersion = .*/kubernetesVendorVersion = "$(K8S_VERSION)"/' cmd/version.go
@echo "Updating Kubernetes version to $(K8S_VERSION) in .goreleaser.yml"
@# Update .goreleaser.yml
@sed -i.bak 's/KUBERNETES_VERSION=.*/KUBERNETES_VERSION=$(K8S_VERSION)/' build/.goreleaser.yml
@# Clean up backup files
Comment on lines +210 to 213
Copy link
Member

@camilamacedo86 camilamacedo86 Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This target was originally created due to: https://github.com/kubernetes-sigs/kubebuilder/pull/4516/files#diff-7cb26babce207ed9c17e9dd3ead0b66d204da3cf4d11649412c197defbb3029cR31

I just reviewed this. If we can ensure that:

Then, we are good to go.

If you find a solution that meets all these requirements without needing to modify cmd/version.go, that would be even better.

Let me know your thoughts!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@camilamacedo86 I ran make install after the changes in this PR and the version was still there:

$ make install
go build -ldflags " -X sigs.k8s.io/kubebuilder/v4/cmd.kubeBuilderVersion=v4.5.1-39-g32e5d5522 -X sigs.k8s.io/kubebuilder/v4/cmd.kubernetesVendorVersion=1.32.1 -X sigs.k8s.io/kubebuilder/v4/cmd.goos=linux -X sigs.k8s.io/kubebuilder/v4/cmd.goarch=amd64 -X sigs.k8s.io/kubebuilder/v4/cmd.gitCommit=32e5d5522b12d71356a2ffbc230866b4ddd512cc -X sigs.k8s.io/kubebuilder/v4/cmd.buildDate=2025-03-18T04:29:06Z " -o bin/kubebuilder
rm -f /home/vitorfloriano/go/bin/kubebuilder
cp ./bin/kubebuilder /home/vitorfloriano/go/bin/kubebuilder

$ kubebuilder version 
Version: main.version{KubeBuilderVersion:"4.5.1", KubernetesVendor:"1.32.1", GitCommit:"0ace7a8753c52b35014e43edc2a0b0454b78e769", BuildDate:"2025-02-21T20:16:18Z", GoOs:"linux", GoArch:"amd64"}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also ran make generate then goreleaser again, to double-check and the version was there too:

$ goreleaser release --snapshot --skip=publish -f ./build/.goreleaser.yml
  • skipping announce, publish, and validate...
  • loading environment variables
  • getting and validating git state
    • git state                                      commit=32e5d5522b12d71356a2ffbc230866b4ddd512cc branch=bug/4611-make-generate-kubernetes-version current_tag=v4.5.1 previous_tag=v4.5.0 dirty=false
...
...
...
...
  • writing artifacts metadata
  • release succeeded after 21s
  • thanks for using GoReleaser!

$ ./dist/kubebuilder_linux_amd64_v1/kubebuilder version
Version: cmd.version{KubeBuilderVersion:"4.5.1-SNAPSHOT-32e5d5522", KubernetesVendor:"1.32.1", GitCommit:"32e5d5522b12d71356a2ffbc230866b4ddd512cc", BuildDate:"2025-03-18T04:40:38Z", GoOs:"linux", GoArch:"amd64"}

Copy link
Member

Choose a reason for hiding this comment

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

Great !!!
So, it seems that it is missing to be checked only for item (b) above. How does that work when we try to install the module using the latest commit? How that will be if we do not set the fixed version in the cmd/version.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@camilamacedo86 If I'm replicating this correctly, go install does not inject the version into the binary:

$ mkdir kb-test
$ cd kb-test/
~/kb-test$ git clone https://github.com/vitorfloriano/kubebuilder.git
Cloning into 'kubebuilder'...

~/kb-test$ cd kubebuilder/
~/kb-test/kubebuilder$ git checkout 32e5d5522b12d71356a2ffbc230866b4ddd512cc
Note: switching to '32e5d5522b12d71356a2ffbc230866b4ddd512cc'.
HEAD is now at 32e5d5522 🐛 Fix changes to Kubernetes version when running make generate

~/kb-test/kubebuilder$ go mod tidy
~/kb-test/kubebuilder$ go install .

~/kb-test/kubebuilder$ ~/go/bin/kubebuilder version
Version: cmd.version{KubeBuilderVersion:"(devel)", KubernetesVendor:"unknown", GitCommit:"$Format:%H$", BuildDate:"1970-01-01T00:00:00Z", GoOs:"unknown", GoArch:"unknown"}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also ran go build to test the binary and got the same result:

~/kb-test/kubebuilder$ go build .
go: downloading github.com/sirupsen/logrus v1.9.3
go: downloading github.com/spf13/afero v1.12.0
go: downloading github.com/spf13/cobra v1.9.1
go: downloading github.com/spf13/pflag v1.0.6
go: downloading sigs.k8s.io/yaml v1.4.0
go: downloading golang.org/x/text v0.23.0
go: downloading golang.org/x/tools v0.31.0
go: downloading golang.org/x/sys v0.31.0
go: downloading github.com/gobuffalo/flect v1.0.3
go: downloading golang.org/x/sync v0.12.0
go: downloading golang.org/x/mod v0.24.0

~/kb-test/kubebuilder$ ./kubebuilder version
Version: cmd.version{KubeBuilderVersion:"(devel)", KubernetesVendor:"unknown", GitCommit:"$Format:%H$", BuildDate:"1970-01-01T00:00:00Z", GoOs:"unknown", GoArch:"unknown"}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@camilamacedo86 I think I see what you mean now. It seems that go install and go build do not handle ldflags correctly, or don't even access them, like it happens when running make install, hence why we need to update the .cmd/version.go file. Shoot!

Copy link
Member

Choose a reason for hiding this comment

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

Can we revert the changes in: update-k8s-version
I think the build flag change is OK .. we can move forward with

Expand Down