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

Fix bastion for multi subnet #518

Merged
merged 3 commits into from
Jun 28, 2022
Merged

Conversation

kon-angelo
Copy link
Contributor

@kon-angelo kon-angelo commented Jun 8, 2022

How to categorize this PR?

/area control-plane
/kind enhancement
/platform azure

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

Bastion now correctly computes network information (vnet, subnet) from infrastructure status.
Add bastion support for multi-subnet deployments

@gardener-robot gardener-robot added the platform/azure Microsoft Azure platform/infrastructure label Jun 8, 2022
@gardener-robot
Copy link

@kon-angelo Labels area/todo, kind/todo do not exist.

@gardener-robot gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Jun 8, 2022
@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 Jun 8, 2022
@kon-angelo
Copy link
Contributor Author

/test

@testmachinery
Copy link

testmachinery bot commented Jun 8, 2022

Testrun: e2e-2kzl2
Workflow: e2e-2kzl2-wf
Phase: Succeeded

+---------------------+---------------------+-----------+----------+
|        NAME         |        STEP         |   PHASE   | DURATION |
+---------------------+---------------------+-----------+----------+
| infrastructure-test | infrastructure-test | Succeeded | 28m42s   |
| bastion-test        | bastion-test        | Succeeded | 9m51s    |
+---------------------+---------------------+-----------+----------+

@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 Jun 8, 2022
@kon-angelo kon-angelo marked this pull request as ready for review June 10, 2022 08:16
@kon-angelo kon-angelo requested review from a team as code owners June 10, 2022 08:16
@gardener-robot gardener-robot added area/control-plane Control plane related kind/enhancement Enhancement, improvement, extension labels Jun 10, 2022
@tedteng
Copy link
Contributor

tedteng commented Jun 21, 2022

@kon-angelo Should we process this PR?

pkg/controller/bastion/actuator_reconcile.go Outdated Show resolved Hide resolved
@@ -62,7 +70,15 @@ func (a *actuator) Reconcile(ctx context.Context, bastion *extensionsv1alpha1.Ba
return err
}

nic, err := ensureNic(ctx, factory, opt, publicIP)
if infrastructureStatus.Networks.Layout != "SingleSubnet" {
return fmt.Errorf("unsupported network layout %s", infrastructureStatus.Networks.Layout)
Copy link
Member

Choose a reason for hiding this comment

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

Why is the singleSubnet layout not supported?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess forget cherry-pick 042d16b @kon-angelo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed I did. I will push an update

@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 Jun 21, 2022
@kon-angelo
Copy link
Contributor Author

/cla

@gardener-robot
Copy link

@kon-angelo I reached out successfully to the cla-assistant to recheck this pull request.

Copy link
Member

@dkistner dkistner left a comment

Choose a reason for hiding this comment

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

/lgtm

@dkistner dkistner merged commit f79f4af into gardener:master Jun 28, 2022
@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging status/closed Issue is closed (either delivered or triaged) and removed needs/review Needs review labels Jun 28, 2022
@kon-angelo kon-angelo deleted the fix-bastion2 branch December 12, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/control-plane Control plane related 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/azure Microsoft Azure platform/infrastructure reviewed/lgtm Has approval for merging size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants