-
Notifications
You must be signed in to change notification settings - Fork 28
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
build: update rook and k8s api version #285
Conversation
go.mod
Outdated
go 1.21 | ||
go 1.22.0 | ||
|
||
toolchain go1.22.1 |
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.
Why was the toochain needed as a separate version from go above? It's nice if we can just declare 1.22
and it automatically picks up the latest patch.
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.
Let's discuss this in huddle,
the k8s 0.30 api are bringing those changes either we have to mention toolchain go.*
or we have to use explicit version of go in go.mod like go 1.22.1
go.mod
Outdated
@@ -1,19 +1,21 @@ | |||
module github.com/rook/kubectl-rook-ceph | |||
|
|||
go 1.21 | |||
go 1.22.0 |
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.
lets use 1.22 not stick to x.y.z so that we always use latest released version and also we will get build problem in downstream if we use x.y.z
here
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.
@Madhu-1 if we use go 1.22
then we need to use toolchain go1.22.1
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.
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.
toolchain is automatically picked up based on the version right? i think that should be fine.
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.
yes it is auto picked by running go mod tidy
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.
The go toolchain doc says:
"That is, the go
line sets the minimum required Go version necessary to use a module or workspace."
Sounds fine to have 1.22.0 as the min version. But do we need to be opinionated about the toolchain that is chosen? Can we remove the toolchain
line?
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.
we can remove the toolchain
we just need to use the same go toolchain version go *
. See the changes now
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.
we will get to problems like red-hat-storage/odf-multicluster-orchestrator#197, sadly that PR doesn't contain details but that was done to fix the build issue in Downstream, for some reason we don't get x.y.z specific go version in downstream , and also this doesn't allow developers to use lower version of 1.22 to build the repo? just ensure that we are covering all the cases
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 think we can discuss this in today's meeting, but anyways we need to use specific version in go.mod either with/without toolchain
go 1.22.0
toolchain go1.22.1
is added when ran go mod tidy
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.
This does allow multiple versions of Go to be used locally at least during development. If I build this branch then run go version I see:
% go version
go version go1.22.1 darwin/arm64
Which is interesting because I don't recall installing 1.22, it must have pulled it automatically. Then if I go back to the rook repo, I still see go 1.21:
% go version
go version go1.21.5 darwin/arm64
So the main requirement I assume is that the downstream build would need to use 1.22 for this repo
this commit updates rook and k8s api version Signed-off-by: subhamkrai <[email protected]>
ccca1e6
to
88fc4ca
Compare
CLosing this one as this has been fixed in another PR. |
this commit updates rook and k8s api version
Description of your changes:
Which issue is resolved by this Pull Request:
Resolves #
Checklist: