Bump openshift/api#2651
Conversation
|
/assign @bertinatto |
go.mod
Outdated
There was a problem hiding this comment.
I think there's a newer update (v0.0.0-20210629...`)
There was a problem hiding this comment.
@bertinatto I have picked the commit hash from your PR as per openshift/api#957
go.mod
Outdated
There was a problem hiding this comment.
Can you check if this is really necessary? When I bump openshift/api locally I get this:
diff --git a/go.mod b/go.mod
index 50523691..fba6adf5 100644
--- a/go.mod
+++ b/go.mod
@@ -35,7 +35,7 @@ require (
github.com/huandu/xstrings v1.2.0 // indirect
github.com/imdario/mergo v0.3.9
github.com/opencontainers/go-digest v1.0.0
- github.com/openshift/api v0.0.0-20210409143810-a99ffa1cac67
+ github.com/openshift/api v0.0.0-20210629145910-15a1cae1fca8
github.com/openshift/client-go v0.0.0-20210112165513-ebc401615f47
github.com/openshift/library-go v0.0.0-20210301154249-aa29957b8a9c
github.com/openshift/runtime-utils v0.0.0-20200415173359-c45d4ff3f912
@@ -51,11 +51,11 @@ require (
github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect
golang.org/x/net v0.0.0-20210224082022-3d97a244fca7
golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba
- k8s.io/api v0.21.0-rc.0
+ k8s.io/api v0.21.1
k8s.io/apiextensions-apiserver v0.21.0-rc.0
- k8s.io/apimachinery v0.21.0-rc.0
+ k8s.io/apimachinery v0.21.1
k8s.io/client-go v0.21.0-rc.0
- k8s.io/code-generator v0.21.0-rc.0
+ k8s.io/code-generator v0.21.1
k8s.io/kubectl v0.21.0-rc.0
k8s.io/kubelet v0.21.0-rc.0
k8s.io/utils v0.0.0-20201110183641-67b214c5f920
@@ -67,7 +67,7 @@ replace (
github.com/godbus/dbus => github.com/godbus/dbus v0.0.0-20190623212516-8a1682060722
github.com/googleapis/gnostic => github.com/googleapis/gnostic v0.4.1
github.com/opencontainers/runtime-spec => github.com/opencontainers/runtime-spec v0.1.2-0.20190408193819-a1b50f621a48
- github.com/openshift/api => github.com/openshift/api v0.0.0-20210409143810-a99ffa1cac67
+ github.com/openshift/api => github.com/openshift/api v0.0.0-20210629145910-15a1cae1fca8
github.com/openshift/cluster-api => github.com/openshift/cluster-api v0.0.0-20191129101638-b09907ac6668
github.com/securego/gosec => github.com/securego/gosec v0.0.0-20190709033609-4b59c948083c
k8s.io/api => k8s.io/api v0.21.0-rc.0
``
There was a problem hiding this comment.
+1 please bump only the minimum set of changes
There was a problem hiding this comment.
go mod vendor && go mod tidy from your PR produced this. If this is not what you are looking for, please guide me for the how
There was a problem hiding this comment.
@kikisdeliveryservice thanks a lot for your review, how to get the minimum set of changes, I applied go mod vendor && go mod tidy and this was the result
There was a problem hiding this comment.
go mod vendor && go mod tidyfrom your PR produced this. If this is not what you are looking for, please guide me for the how
cc: @bertinatto
There was a problem hiding this comment.
@kikisdeliveryservice @bertinatto fixed 👍🏽
There was a problem hiding this comment.
once you have updated required deps, you can simply run make go-deps
There was a problem hiding this comment.
@sinnykumari thanks for pointing out make go-deps.
I noticed that this target marks some scripts as executable. IMO the vendor directory should not be changed by anything other than go mod vendor. I think this should be handled in a different way, like whatever is calling those scripts should pass them as args to bash/sh or something else. Anyway, that's up to you folks.
There was a problem hiding this comment.
these executable mode change is there for a while now, it is a good idea to re-validate and see if we even need it now or not.
Elbehery
force-pushed
the
bump-openshift-api
branch
from
3f37f3f to
dfccfaf
Compare
kikisdeliveryservice
requested review from
sinnykumari
and removed request for
yuqi-zhang
|
/retest |
1 similar comment
|
/retest |
go.mod
Outdated
There was a problem hiding this comment.
@Elbehery please squash your commits and add a descriptive message stating why we're bumping openshift/api. You'll be able to re-use it in the other PRs.
The PR description is a good start 😉
There was a problem hiding this comment.
@Elbehery sorry for picking on this, but the commit message isn't quite correct: we are not enabling the feature gates here, the user is the one who enables them (through theTechPreviewNoUpgrade featureSet).
I meant the commit message could be something like this (for this PR specifically):
Bump openshift/api
The goal of this bump is to apply `CSIMigrationGCE` and `CSIMigrationAzureDisk`
feature gates to kubelet config when the `TechPreviewNoUpgrade` featureSet is
set by the user.
There was a problem hiding this comment.
when you get a chance could you update ^^ @Elbehery
Elbehery
force-pushed
the
bump-openshift-api
branch
from
1e6675f to
83c4fa5
Compare
|
/retest |
bertinatto
left a comment
bertinatto
left a comment
There was a problem hiding this comment.
Just one nit about the commit message, otherwise LGTM.
/assign @sinnykumari @kikisdeliveryservice
for approval
go.mod
Outdated
There was a problem hiding this comment.
@Elbehery sorry for picking on this, but the commit message isn't quite correct: we are not enabling the feature gates here, the user is the one who enables them (through theTechPreviewNoUpgrade featureSet).
I meant the commit message could be something like this (for this PR specifically):
Bump openshift/api
The goal of this bump is to apply `CSIMigrationGCE` and `CSIMigrationAzureDisk`
feature gates to kubelet config when the `TechPreviewNoUpgrade` featureSet is
set by the user.
|
lgtm. pending commit message update |
The goal of this bump is to apply `CSIMigrationGCE` and `CSIMigrationAzureDisk` feature gates to kubelet config when the `TechPreviewNoUpgrade` featureSet is set by the user.
Elbehery
force-pushed
the
bump-openshift-api
branch
from
83c4fa5 to
8dba95a
Compare
@bertinatto @kikisdeliveryservice @sinnykumari commit msg update accordingly 👍🏽 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bertinatto, Elbehery, sinnykumari The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@Elbehery: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
7 participants
This will allow us to get latest feature gates about CSI migration
Note: this is needed for CSI migration feature to work in OCP 4.9.
Goal of this PR is to apply --feature-gate= CSIMigrationGCE=true, CSIMigrationAzureDisk=true to kubelet when TechPreviewNoUpgrade featureSet is enabled, which was introduced in openshift/api#957