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.

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

Add useSNAT option to CloudProfile #165

Merged
merged 1 commit into from
Oct 22, 2020
Merged

Add useSNAT option to CloudProfile #165

merged 1 commit into from
Oct 22, 2020

Conversation

timuthy
Copy link
Member

@timuthy timuthy commented Oct 20, 2020 •
edited
Loading

How to categorize this PR?

/area networking
/kind enhancement
/priority normal
/platform openstack

What this PR does / why we need it:
This PR adds a new option useSNAT to the provider's specific CloudProfileConfig. If set to true, Gardener creates the OpenStack Router with the enable_snat option.

Which issue(s) this PR fixes:
Fixes parts of #122

Special notes for your reviewer:
/cc @schrodit @vasu1124 @vlerenc @markbgrdt

Release note:

The OpenStack extension now created OpenStack routers with [enable_snat](https://registry.terraform.io/providers/terraform-provider-openstack/openstack/latest/docs/resources/networking_router_v2#enable_snat) if the corresponding option `.useSNAT` is set to `true` in the provider's `CloudProfileConfig`.

@timuthy timuthy requested review from a team as code owners October 20, 2020 15:09
@gardener-robot gardener-robot added kind/api-change API change with impact on API users needs/second-opinion Needs second review by someone else labels Oct 20, 2020
@gardener-robot
Copy link

@timuthy Labels area/todo, kind/todo do not exist.

@gardener-robot gardener-robot added platform/openstack OpenStack platform/infrastructure priority/normal needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Oct 20, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 20, 2020
@timuthy timuthy mentioned this pull request Oct 20, 2020
Open
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 20, 2020
@gardener-robot gardener-robot added area/networking Networking related kind/enhancement Enhancement, improvement, extension labels Oct 20, 2020
MartinWeindel
MartinWeindel previously approved these changes Oct 21, 2020
View reviewed changes
Copy link
Member

@MartinWeindel MartinWeindel left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review needs/second-opinion Needs second review by someone else labels Oct 21, 2020
dkistner
dkistner previously approved these changes Oct 21, 2020
View reviewed changes
rfranzke
rfranzke reviewed Oct 21, 2020
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

What happens to existing clusters which already have a router if this is being enabled?

docs/usage-as-operator.md Show resolved Hide resolved
@timuthy
Copy link
Member Author

timuthy commented Oct 21, 2020

What happens to existing clusters which already have a router if this is being enabled?

Good question @rfranzke. Let me verify and get back in a bit.

@rfranzke
Copy link
Member

/status author-action

@gardener-robot gardener-robot added status/author-action Issue requires issue author action and removed reviewed/lgtm Has approval for merging labels Oct 21, 2020
@gardener-robot
Copy link

@timuthy The pull request was assigned to you under author-action. Please unassign yourself when you are done. Thank you.

@timuthy timuthy dismissed stale reviews from dkistner and MartinWeindel via 25df2cf October 21, 2020 15:27
@gardener-robot gardener-robot added the needs/second-opinion Needs second review by someone else label Oct 21, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 21, 2020
@timuthy
Copy link
Member Author

timuthy commented Oct 21, 2020

What happens to existing clusters which already have a router if this is being enabled?

Good question @rfranzke. Let me verify and get back in a bit.

I tested the changes with an existing cluster successfully.

rfranzke
rfranzke approved these changes Oct 22, 2020
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/second-opinion Needs second review by someone else status/author-action Issue requires issue author action labels Oct 22, 2020
@rfranzke rfranzke merged commit 88147e4 into gardener:master Oct 22, 2020
@timuthy timuthy deleted the feature.snat branch October 22, 2020 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfranzke rfranzke rfranzke approved these changes

@dkistner dkistner dkistner left review comments

@MartinWeindel MartinWeindel MartinWeindel left review comments

Assignees

@timuthy timuthy

Labels
area/networking Networking related kind/api-change API change with impact on API users kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/openstack OpenStack platform/infrastructure reviewed/lgtm Has approval for merging size/s Size of pull request is small (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@timuthy @gardener-robot @rfranzke @MartinWeindel @dkistner @gardener-robot-ci-1 @gardener-robot-ci-3