-
Notifications
You must be signed in to change notification settings - Fork 549
add go.mod for operator #3735
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
add go.mod for operator #3735
Conversation
Signed-off-by: Troy Chiu <y.troychiu@gmail.com>
@@ -5,6 +5,8 @@ WORKDIR /workspace | |||
|
|||
# Copy the Go Modules manifests | |||
COPY go.mod go.sum ./ | |||
COPY ray-operator/go.mod ray-operator/go.mod |
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.
Can you help me understand why ray-operator/go.mod
is needed 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.
Since we have replace github.com/ray-project/kuberay/ray-operator => ./ray-operator
in kuberay/go.mod
, go knows /ray-operator is another module and expects /ray-operator
contains go.mod
.
go.mod
Outdated
github.com/pkg/errors v0.9.1 | ||
github.com/prometheus/client_golang v1.22.0 | ||
github.com/ray-project/kuberay/ray-operator v1.3.2 |
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.
Was this added manually? In addition, if we use v1.3.2, we’ll need to manually update this for each release.
What's the best practice? Is it OK to use something like v0.0.0
so that we don't need to manually update it for each release or v0.0.0
is not a good practice.
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.
cc @rueian for inputs
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 am sure that using v0.0.0
is not a good practice. Users need a correct version specified in the go.mod for go get
and go mod tidy
to work properly.
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.
v1.3.2, which points to the wrong version, is also incorrect. What happens if users run go get with v1.3.2? I assume it will pull the v1.3.2 package from the internet, which means the API server will end up using a different KubeRay commit for ray-operator.
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.
https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/go.mod#L51
K8s also uses v0.0.0 and replace
.
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.
What happens if users run go get with v1.3.2? I assume it will pull the v1.3.2 package from the internet, which means the API server will end up using a different KubeRay commit for ray-operator.
Is this statement correct?
Yes. If we have an apiserver/v1.3.2
tag on the same release commit and a user runs
8000
go get github.com/ray-project/kuberay/apiserver
, then it will depend on the 3e7749d17400
commit.
In our case, can we use v0.0.0 on the master branch and v1.X.Y on the v1.X.Y release branch? Or is our case different?
Yes, we can.
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.
Yeah I can confirm simply using a dummy version such as v0.0.0 won't work. But
In our case, can we use v0.0.0 on the master branch and v1.X.Y on the v1.X.Y release branch? Or is our case different?
sounds a good solution
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.
@troychiu if you run go mod tidy
with version v0.0.0, go.mod
should be update it a valid version like v0.0.0-20220703232803-3e7749d17400
. This is what we had in the old go.mod file for apiserver: https://github.com/ray-project/kuberay/blob/release-1.3/apiserver/go.mod#L12C46-L12C80. The actual version doesn't matter due to the replace directive
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 did run go mod tidy
before pushing but go.mod was not updated. 🤔
I can manually update to that if that's preferred.
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.
ah, maybe you need to set the version to a branch name and it will auto resolve
cc @andrewsykim @rueian for review |
Signed-off-by: Troy Chiu <y.troychiu@gmail.com>
@andrewsykim Do you have any other concerns? We expect to merge this PR tomorrow. Thanks. |
LGTM, we may need to follow-up on #3735 (comment) |
* add go.mod for operator Signed-off-by: Troy Chiu <y.troychiu@gmail.com> * use v0.0.0 Signed-off-by: Troy Chiu <y.troychiu@gmail.com> --------- Signed-off-by: Troy Chiu <y.troychiu@gmail.com>
Why are these changes needed?
This PR add go.mod for ray-operator. For more details, see #3730
Related issue number
Closes #3730
Checks