Skip to content

terraform: ibmcloud: bump provider version #6210

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.

Sign up for GitHub

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
Merged
openshift-merge-robot merged 2 commits into from
Aug 14, 2022
Merged

terraform: ibmcloud: bump provider version #6210

openshift-merge-robot merged 2 commits into from
Aug 14, 2022

Conversation

@clnperez
Copy link
Contributor

@clnperez clnperez commented Aug 9, 2022

to get network_name from ibm_pi_dhcps for the Power VS provider

Signed-off-by: Christy Norman christy@linux.vnet.ibm.com

@clnperez
Copy link
Contributor Author

clnperez commented Aug 9, 2022

@cjschaef FYI

@openshift-ci openshift-ci bot requested review from jstuever and r4f4 August 9, 2022 21:11
r4f4
r4f4 suggested changes Aug 9, 2022
View reviewed changes
Copy link
Contributor

@r4f4 r4f4 left a comment

Choose a reason for hiding this comment

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

Vendoring changes should be in their own commit.

@clnperez
Copy link
Contributor Author

clnperez commented Aug 9, 2022

hmm. @r4f4 . this is all pretty much just one giant vendor. which files specifically should i split out? the two under terraform/?

  • terraform/providers/ibm/go.mod
  • terraform/providers/ibm/go.sum

@r4f4
Copy link
Contributor

r4f4 commented Aug 10, 2022

hmm. @r4f4 . this is all pretty much just one giant vendor. which files specifically should i split out? the two under terraform/?

* `terraform/providers/ibm/go.mod`

* `terraform/providers/ibm/go.sum`

Yes. go.mod and go.sum changes in one commit, vendor/* changes in another.

@sadasu
Copy link
Contributor

sadasu commented Aug 10, 2022

/retest-required

@clnperez clnperez force-pushed the ibmcloud-tf-bump-1.44.2 branch from 5372966 to 7b49c60 Compare August 10, 2022 17:21
@clnperez
Copy link
Contributor Author

clnperez commented Aug 10, 2022

@r4f4 commits split out

@r4f4
Copy link
Contributor

r4f4 commented Aug 10, 2022

@r4f4 commits split out

@clnperez thanks, that looks good.

I'm afraid I'll be pedantic again: to align the commit msg with the team guidelines [1], I'd suggest rewording the mod files commit to something like:

terraform: ibmcloud: bump provider version

to get network_name from ibm_pi_dhcps for the Power VS provider

But we can wait for the current ibmcloud e2e run to finish first.

[1] https://github.com/openshift/installer/blob/master/CONTRIBUTING.md#commit-message-format

@clnperez
terraform: ibmcloud: bump provider version
5845fe9 
to get network_name from ibm_pi_dhcps for the Power VS provider

Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>
@clnperez
vendor dir
a6aea18 
the new files under vendor

Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>
@clnperez clnperez force-pushed the ibmcloud-tf-bump-1.44.2 branch from 7b49c60 to a6aea18 Compare August 10, 2022 22:42
@clnperez
Copy link
Contributor Author

clnperez commented Aug 10, 2022 •
edited
Loading

@r4f4 fixed up the commit verbiage and pushed after the tests had all completed. the ibmcloud ci spins up a cluster and runs tests so i'm pretty confident that this is fine. but will wait for the ibmcloud folks approval

@clnperez clnperez changed the title update ibmcloud tf provider terraform: ibmcloud: bump provider version Aug 10, 2022
@clnperez clnperez mentioned this pull request Aug 10, 2022
Merged
@r4f4
Copy link
Contributor

r4f4 commented Aug 11, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2022
@r4f4 r4f4 removed their assignment Aug 11, 2022
@cjschaef
Copy link
Member

cjschaef commented Aug 11, 2022

I agree with #6210 (comment)

Did some local testing with these changes as well and x86_64 looks to be okay with them.

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 11, 2022

@cjschaef: changing LGTM is restricted to collaborators

Details

In response to this:

I agree with #6210 (comment)

Did some local testing with these changes as well and x86_64 looks to be okay with them.

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

@patrickdillon
Copy link
Contributor

patrickdillon commented Aug 12, 2022

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 12, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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

The pull request process is described here

Details 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2022
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 12, 2022

/retest-required

Remaining retests: 2 against base HEAD 977b2bb and 8 for PR HEAD a6aea18 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 12, 2022 •
edited
Loading

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

Test name Commit Details Required Rerun command
ci/prow/e2e-libvirt a6aea18 link false /test e2e-libvirt
ci/prow/e2e-aws-proxy a6aea18 link false /test e2e-aws-proxy
ci/prow/e2e-aws-single-node a6aea18 link false /test e2e-aws-single-node
ci/prow/e2e-ibmcloud a6aea18 link false /test e2e-ibmcloud

Full PR test history. Your PR dashboard.

Details

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.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 12, 2022

/retest-required

Remaining retests: 1 against base HEAD 977b2bb and 7 for PR HEAD a6aea18 in total

@r4f4
Copy link
Contributor

r4f4 commented Aug 13, 2022

/retest-required

@openshift-merge-robot openshift-merge-robot merged commit 3db9150 into openshift:master Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jstuever jstuever Awaiting requested review from jstuever

1 more reviewer

@r4f4 r4f4 r4f4 requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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.

7 participants

@clnperez @r4f4 @sadasu @cjschaef @patrickdillon @openshift-ci-robot @openshift-merge-robot