openstack: Support setting network UUID via terraform variable.

[russellb]

openstack: Support setting network UUID via terraform variable.

Technically, network names in OpenStack are not unique.  We would
expect almost every deployment to be using a unique name for its
external network, though.

In the rare case where you must disambiguate networks, it's possible
to specify the UUID via a terraform variable.

$ env TF_VAR_openstack_external_network_id="6a32627e-d98d-40d8-9324-5da7cf1452fc" \
> bin/openshift-install create cluster

[staebler]

We would expect almost every deployment to be using a unique name for its external network, though.

If this is truly the case, then we should not be adding this as an option in install-config.yml. The barrier for entry into that file should be high. The rare customer that needs to use the network UUID will need to bear the burden of manually modifying the assets generated in later stages. In the near future, there will be an installer target that creates the tfvars (JIRA 917), which the end user can modify to accomplish this.

[russellb]

Ok, that sounds good. I’ll drop the install config part and leave it as a tfvar that can be manually set in the rare case. Thanks.

On Fri, Dec 7, 2018 at 10:53 PM Matthew Staebler ***@***.***> wrote: We would expect almost every deployment to be using a unique name for its external network, though. If this is truly the case, then we should not be adding this as an option in install-config.yml. The barrier for entry into that file should be high. The rare customer that needs to use the network UUID will need to bear the burden of manually modifying the assets generated in later stages. In the near future, there will be an installer target that creates the tfvars (JIRA 917 <https://jira.coreos.com/browse/CORS-917>), which the end user can modify to accomplish this. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#794 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAS4Cun0h5jWYqBcJ8zftO0LR9CC-khdks5u2zeYgaJpZM4ZFH2j> .

-- Russell Bryant

[hardys]

I tested this today and it looks good - kinda unfortunate that we can't hide the name/id interface in the terraform provider or gophercloud but this seems reasonable assuming we want to enable some cases where an ID is preferred over the name.

Happy to approve but wasn't sure if we're waiting on a re-review from @staebler ?

[russellb]

I was going to rework this once the "tfvars" installer target was added. Then I'd drop this from install config, and we'd just have a terraform variable you can hack in the rare case it's needed. I should probably mark this as WIP or something.

[russellb]

/hold

[wking]

The PR and commit-message subjects seem stale since the shift to Terraform variables.

[russellb]

Thanks, I'll fix the PR and commit message.

I rebased just to keep the code current, but have the PR marked as blocked until the tfvars target is available which would make this usable in the way we discussed earlier in the PR.

[flaper87]

looks good, one question inline.

[flaper87]

What happens if someone "accidentally" sets both, name and id, and these 2 don't correspond to the same network? Will neutron fail or will it give precedence to the id?

[russellb]

I don't know for sure since I haven't tested it, but I think terraform will fail because it won't be able to find a network that matches that name + UUID. I do know that setting both works, if you set them properly.

[russellb]

I did test this. Terraform will fail because it can't find a matching network with that name + UUID.

[flaper87]

@russellb you planning to update this?

[russellb]

I was waiting on a tfvars installer target but maybe that’s not going to get added? There is already the TF_VAR style env variables that would make this usable so maybe we don’t need to wait?

On Tue, Feb 5, 2019 at 7:55 AM Flavio Percoco ***@***.***> wrote: @russellb <https://github.com/russellb> you planning to update this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#794 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAS4CrjS7MAjdDrNzWb8Yb9z48Zq23fHks5vKX8mgaJpZM4ZFH2j> .

-- Russell Bryant

[russellb]

/hold cancel

I think this is ready to go now. @flaper87

[flaper87]

@russellb sorry it took me long to get back to this. If you do another rebase, I'll send it through. it looks good

[russellb]

@flaper87 rebase done

[flaper87]

/lgtm

[flaper87]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flaper87, russellb

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 [flaper87,russellb]

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

[flaper87]

/test e2e-aws