Bug 1732858: unhardcode the cloud name for openstack #2122

Fedosin · 2019-07-30T16:51:38Z

To allow users to provide custom cloud names in the install config we need to remove hardcoded 'openstack' name.

To do so and to keep the compatibility with the current behavior, we read cloud name from the install config first, and if it's empty, set it to the value of OS_CLOUD env variable. If both values are not specified, then cloud name defaults to "openstack".

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1732858

Fedosin · 2019-07-31T09:07:22Z

/test e2e-openstack

Fedosin · 2019-07-31T23:22:29Z

/test e2e-openstack

tomassedovic · 2019-08-05T09:29:36Z

/assign

tomassedovic · 2019-08-05T10:14:56Z

/lgtm
/hold

Looks good to me, thanks! I'm putting it on /hold until the e2e-openstack job passes. All OpenStack are currently broken due to openshift/library-go#500. The fix (openshift/cluster-kube-controller-manager-operator#275) is merged, but we need to wait for it to appear in the release image and retest.

Feel free to remove the hold once e2e-openstack is green.

tomassedovic · 2019-08-05T11:18:34Z

/retest

mandre · 2019-08-05T12:05:24Z

/test e2e-openstack

mandre · 2019-08-05T12:23:27Z

/retest

tomassedovic · 2019-08-05T13:31:16Z

/retest

mandre · 2019-08-05T15:41:53Z

/retest

Fedosin · 2019-08-06T09:56:38Z

/test e2e-openstack

Fedosin · 2019-08-06T11:03:04Z

/test e2e-openstack

Fedosin · 2019-08-06T15:22:12Z

/hold cancel

abhinavdahiya · 2019-08-12T20:26:20Z

pkg/tfvars/openstack/openstack.go

why is the cloud name on master machine objects different from the the one that should actually be used??

Yeah, it's not related here at all... I changed this.
Now, when the installer wants to read a local clouds.yaml file, it looks at "cloud" value from the install-config.yaml first, and, if there is nothing, it tries the default value "openstack".

But when we create a system secret with a generated "clouds.yaml" item, it always sets the name as "openstack" there, which is good for consistency.

abhinavdahiya · 2019-08-13T22:46:27Z

pkg/tfvars/openstack/openstack.go

this should part of the defaults.. not when passing those to terraform.

To allow users to provide custom cloud names in the install config we need to remove the hardcoded 'openstack' name. To do so and to keep the compatibility with the current behavior, we read cloud name from the install config first, and if it's empty, set it to the value of OS_CLOUD env variable. If both values are not specified, then cloud name defaults to "openstack".

openshift-ci-robot · 2019-08-14T19:23:28Z

@Fedosin: This pull request references a valid Bugzilla bug. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1732858: unhardcode the cloud name for openstack

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.

abhinavdahiya · 2019-08-14T19:34:13Z

/approve

Fedosin · 2019-08-15T11:44:22Z

/test e2e-openstack

Fedosin · 2019-08-15T11:54:32Z

/test e2e-openstack

Fedosin · 2019-08-15T22:06:43Z

/test e2e-openstack

dprince · 2019-08-16T01:02:33Z

/lgtm

openshift-ci-robot · 2019-08-16T01:02:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, dprince, Fedosin, mandre, tomassedovic

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:

~~OWNERS~~ [abhinavdahiya]

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

openshift-bot · 2019-08-16T02:27:17Z

/retest

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

openshift-ci-robot · 2019-08-16T03:47:18Z

@Fedosin: All pull requests linked via external trackers have merged. The Bugzilla bug has been moved to the MODIFIED state.

Details

In response to this:

Bug 1732858: unhardcode the cloud name for openstack

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-robot requested review from jcpowermac and mtnbikenc July 30, 2019 16:51

openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 30, 2019

Fedosin force-pushed the unhardcode_openstack branch from 77fa8de to 3b3c07a Compare July 30, 2019 17:08

openshift-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 30, 2019

Fedosin force-pushed the unhardcode_openstack branch from 3b3c07a to 7db337b Compare July 30, 2019 20:09

Fedosin force-pushed the unhardcode_openstack branch from 7db337b to c042f68 Compare July 31, 2019 11:08

openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 31, 2019

openshift-ci-robot assigned tomassedovic Aug 5, 2019

tomassedovic mentioned this pull request Aug 5, 2019

[OPENSTACK] platform.openstack.cloud parameter doesn't have an effect #1404

Closed

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Aug 5, 2019

Fedosin force-pushed the unhardcode_openstack branch from c042f68 to 1e6c781 Compare August 6, 2019 13:03

openshift-ci-robot removed lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 6, 2019

abhinavdahiya reviewed Aug 12, 2019

View reviewed changes

Fedosin force-pushed the unhardcode_openstack branch from 7962a4e to 065efa8 Compare August 13, 2019 22:29

openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 13, 2019

abhinavdahiya reviewed Aug 13, 2019

View reviewed changes

pkg/tfvars/openstack/openstack.go Outdated

Copy link

Contributor

abhinavdahiya Aug 13, 2019

Choose a reason for hiding this comment

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

this should part of the defaults.. not when passing those to terraform.

Fedosin force-pushed the unhardcode_openstack branch from 065efa8 to ff6f1fa Compare August 14, 2019 10:22

openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 14, 2019

Fedosin force-pushed the unhardcode_openstack branch from ff6f1fa to 37f7816 Compare August 14, 2019 11:36

openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 14, 2019

Fedosin force-pushed the unhardcode_openstack branch from 37f7816 to b4de924 Compare August 14, 2019 16:14

Fedosin force-pushed the unhardcode_openstack branch from b4de924 to b920e6c Compare August 14, 2019 19:16

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 14, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 14, 2019

openshift-ci-robot assigned dprince Aug 16, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 16, 2019

openshift-merge-robot merged commit c7a87b5 into openshift:master Aug 16, 2019

Fedosin deleted the unhardcode_openstack branch November 1, 2019 11:18

Bug 1732858: unhardcode the cloud name for openstack #2122

Bug 1732858: unhardcode the cloud name for openstack #2122

Uh oh!

Conversation

Fedosin commented Jul 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fedosin commented Jul 31, 2019

Uh oh!

Fedosin commented Jul 31, 2019

Uh oh!

tomassedovic commented Aug 5, 2019

Uh oh!

tomassedovic commented Aug 5, 2019

Uh oh!

tomassedovic commented Aug 5, 2019

Uh oh!

mandre commented Aug 5, 2019

Uh oh!

mandre commented Aug 5, 2019

Uh oh!

tomassedovic commented Aug 5, 2019

Uh oh!

mandre commented Aug 5, 2019

Uh oh!

Fedosin commented Aug 6, 2019

Uh oh!

Fedosin commented Aug 6, 2019

Uh oh!

Fedosin commented Aug 6, 2019

Uh oh!

abhinavdahiya Aug 12, 2019

Choose a reason for hiding this comment

Uh oh!

Fedosin Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

Fedosin Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

abhinavdahiya Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Aug 14, 2019

Uh oh!

abhinavdahiya commented Aug 14, 2019

Uh oh!

Fedosin commented Aug 15, 2019

Uh oh!

Fedosin commented Aug 15, 2019

Uh oh!

Fedosin commented Aug 15, 2019

Uh oh!

dprince commented Aug 16, 2019

Uh oh!

openshift-ci-robot commented Aug 16, 2019

Uh oh!

openshift-bot commented Aug 16, 2019

Uh oh!

openshift-ci-robot commented Aug 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Fedosin commented Jul 30, 2019 •

edited

Loading