search

kubeflow / common

Common APIs and libraries shared by other Kubeflow operator repositories.
Apache License 2.0
51 stars 73 forks source link

update Volcano CRD API to v1.16 #123

Closed Thor-wl closed 3 years ago

Thor-wl commented 3 years ago

Signed-off-by: Thor-wl 13164644535@163.com

Thor-wl commented 3 years ago

I've got the root reason for CI failed. Before executing go build, go get volcano.sh/apis@v1.16.15 works and pull the code from volcano.sh/apis. Indeed, it pulls the right version. But what confused me is that all zz_generated.xx,go files under each CRD diretory were not pulled. Do you have met this problem? Any advice?

Thor-wl commented 3 years ago

Well, what were pulled to local is not from the repository(not the up-to-date code) but looks like a old version. There seems to be some cache code. Do you have some ideas about that?

shinytang6 commented 3 years ago

Well, what were pulled to local is not from the repository(not the up-to-date code) but looks like a old version. There seems to be some cache code. Do you have some ideas about that?

lt seems that you didn't release a tag for the latest update?

Jeffwan commented 3 years ago

Well, what were pulled to local is not from the repository(not the up-to-date code) but looks like a old version. There seems to be some cache code. Do you have some ideas about that?

After checking logs, I notice volcano api uses 1.19 changes which already has context support in client operations. However, in kubeflow/common, we replace dependencies to 1.16 which doesn't have it. That's the reason build failed.

Let me file a PR to bump the version and you can rebase the changes, then change PR title to "Use Volca CRD API" (Remove version information)

Thor-wl commented 3 years ago

Well, what were pulled to local is not from the repository(not the up-to-date code) but looks like a old version. There seems to be some cache code. Do you have some ideas about that?

After checking logs, I notice volcano api uses 1.19 changes which already has context support in client operations. However, in kubeflow/common, we replace dependencies to 1.16 which doesn't have it. That's the reason build failed.

Let me file a PR to bump the version and you can rebase the changes, then change PR title to "Use Volca CRD API" (Remove version information)

I've taken some tests about it. It's caused by the reuse of tag v1.16.15. Namely, i've had taged v1.16.15 ,deleted it, made some changes and taged again. But the go repository cached the original version and will not be refreshed again. So we can only get the old version from cache. I'll make a new tag.

Jeffwan commented 3 years ago

Please have a check on https://github.com/kubeflow/common/pull/126. Once it's merged, this PR should be unblocked (need minor changes)

Thor-wl commented 3 years ago

I've update the tag and it's ok now.

Jeffwan commented 3 years ago

/approve

/cc @gaocegege @terrytangyuan @shinytang6 for double confirmation

google-oss-robot commented 3 years ago

@Jeffwan: GitHub didn't allow me to request PR reviews from the following users: double, confirmation, shinytang6, for.

Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to [this](https://github.com/kubeflow/common/pull/123#issuecomment-809992206): >/approve > >/cc @gaocegege @shinytang6 for double confirmation Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
shinytang6 commented 3 years ago

great, LGTM :) /approve

google-oss-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jeffwan, shinytang6, Thor-wl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubeflow/common/blob/master/OWNERS)~~ [Jeffwan] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
gaocegege commented 3 years ago

/lgtm

Thanks for your contribution! :tada: :+1: