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.

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

Jump to bottom

Vendor: update library-go for ibmcloud provider #2757

Merged
merged 1 commit into from Sep 15, 2021
Merged

Vendor: update library-go for ibmcloud provider #2757

merged 1 commit into from Sep 15, 2021

Conversation

jonesbr17
Copy link
Contributor

- What I did
Revendor dependencies

- How to verify it

- Description for the changelog
Pull in support for ibmcloud added in openshift/library-go#1161

@sinnykumari
Copy link
Contributor

This PR is also updating some of the kubernetes deps to 1.22.1. Can you please also update rest of the kube deps to 1.22.1 as well?
Also, since MCO uses replace directive to pin some kube and OpenShift deps, you will need to version match deps updated in require section into replace (https://github.com/openshift/machine-config-operator/blob/master/go.mod#L64) section, otherwise vendored content doesn't get updated with correct versions.

rtheis
rtheis reviewed Sep 8, 2021
View reviewed changes
Copy link

@rtheis rtheis left a comment

Choose a reason for hiding this comment

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

Thanks Bryan. No further comments beyond what was already provided.

Prashanth684
Prashanth684 reviewed Sep 8, 2021
View reviewed changes
go.mod Outdated Show resolved Hide resolved
rtheis
rtheis reviewed Sep 8, 2021
View reviewed changes
go.mod Outdated Show resolved Hide resolved
sinnykumari
sinnykumari reviewed Sep 9, 2021
View reviewed changes
go.mod Outdated Show resolved Hide resolved
sinnykumari
sinnykumari reviewed Sep 9, 2021
View reviewed changes
go.mod Outdated Show resolved Hide resolved
sinnykumari
sinnykumari reviewed Sep 9, 2021
View reviewed changes
go.mod
k8s.io/kubernetes => k8s.io/kubernetes v1.22.0-rc.0
k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.22.0-rc.0
k8s.io/metrics => k8s.io/metrics v0.22.0-rc.0
k8s.io/mount-utils => k8s.io/mount-utils v0.23.0-alpha.0
Copy link
Contributor

Choose a reason for hiding this comment

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

@umohnani8 Do we need to keep this version or we can keep it consistent with rest of the replace for kube deps i.e v0.22.1

sinnykumari
sinnykumari reviewed Sep 9, 2021
View reviewed changes
go.mod Outdated Show resolved Hide resolved
@sinnykumari
Copy link
Contributor

Latest change mostly looks good. One question:
@jonesbr17 did you run make go-deps (https://github.com/openshift/machine-config-operator/blob/master/Makefile#L54) after updating the go.mod file. Asking this because I don't see much changes in vendor directory with updated content.

rtheis
rtheis reviewed Sep 9, 2021
View reviewed changes
go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@jeffnowicki
Copy link

/retest-required

@sinnykumari
Copy link
Contributor

Please fix some final comments from @rtheis and then this should be good to get merged

@jeffnowicki
Copy link

/assign @kikisdeliveryservice

@jeffnowicki
Copy link

@sinnykumari can you add required label(s)... lgtm and/or approved? would like to get this merged and into release image. thank you

@sinnykumari
Copy link
Contributor

@sinnykumari can you add required label(s)... lgtm and/or approved? would like to get this merged and into release image. thank you

There are few outstanding comment that needs to get fixed merge #2757 (comment) and #2757 (comment)

@JoelSpeed
Copy link
Contributor

/lgtm

Thanks @jonesbr17

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 15, 2021
sinnykumari
sinnykumari approved these changes Sep 15, 2021
View reviewed changes
Copy link
Contributor

@sinnykumari sinnykumari left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and doing the final fixes. lgtm

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 15, 2021
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@sinnykumari
Copy link
Contributor

Skipping optional tests to save some ci resources.
/skip

rtheis
rtheis approved these changes Sep 15, 2021
View reviewed changes
Copy link

@rtheis rtheis left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 15, 2021

@rtheis: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Instructions 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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 15, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, jonesbr17, rtheis, sinnykumari

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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rtheis
Copy link

rtheis commented Sep 15, 2021

/retest-required

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 15, 2021 •
edited

@jonesbr17: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Rerun command
ci/prow/okd-e2e-aws 95ad345 link /test okd-e2e-aws
ci/prow/e2e-aws-workers-rhel7 95ad345 link /test e2e-aws-workers-rhel7
ci/prow/e2e-aws-workers-rhel8 95ad345 link /test e2e-aws-workers-rhel8
ci/prow/e2e-aws-upgrade-single-node 95ad345 link /test e2e-aws-upgrade-single-node
ci/prow/e2e-ovn-step-registry 95ad345 link /test e2e-ovn-step-registry
ci/prow/e2e-aws-single-node 95ad345 link /test e2e-aws-single-node
ci/prow/e2e-aws-disruptive 95ad345 link /test e2e-aws-disruptive

Full PR test history. Your PR dashboard.

Instructions 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.

@jonesbr17
Copy link
Contributor Author

/test e2e-agnostic-upgrade

@openshift-merge-robot openshift-merge-robot merged commit 19379f1 into openshift:master Sep 15, 2021
@jonesbr17 jonesbr17 deleted the ibm-cloud-provider-revendor branch September 20, 2021 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Prashanth684 Prashanth684 Prashanth684 left review comments

@clnperez clnperez clnperez left review comments

@sinnykumari sinnykumari sinnykumari approved these changes

@rtheis rtheis rtheis approved these changes

@kikisdeliveryservice kikisdeliveryservice Awaiting requested review from kikisdeliveryservice

Assignees

@kikisdeliveryservice kikisdeliveryservice

@JoelSpeed JoelSpeed

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@jonesbr17 @sinnykumari @jeffnowicki @JoelSpeed @openshift-bot @rtheis @Prashanth684 @clnperez @kikisdeliveryservice @openshift-merge-robot