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

Disable SrcAndDestCheck on NAT Instances #39

Merged
merged 2 commits into from
Aug 2, 2021
Merged

Disable SrcAndDestCheck on NAT Instances #39

merged 2 commits into from
Aug 2, 2021

Conversation

AxiomSamarth
Copy link
Collaborator

@AxiomSamarth AxiomSamarth commented Jul 28, 2021 •
edited by prashanth26
Loading

What this PR does / why we need it:
This PR is needed to support calico's "CrossSubnet" mode on gardener clusters for provider type aws the src and dst checks. This PR introduces a new boolean field in the providerSpec of the MachineClass for AWS called SrcAndDstChecksEnabled which when set to false disabled the SrcAndDestCheck on that AWS EC2 instance.

Which issue(s) this PR fixes:
Fixes #36

Special notes for your reviewer:

Release note:

A new boolean field called `SrcAndDstChecksEnabled` in the providerSpec of the MachineClass for AWS is introduced. The default value of this flag is `true` which retains current behavior. However, on setting this flag to `false` these checks are disabled.

@AxiomSamarth AxiomSamarth requested review from a team as code owners July 28, 2021 10:40
@gardener-robot gardener-robot added needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Jul 28, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) 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 Jul 28, 2021
@AxiomSamarth AxiomSamarth mentioned this pull request Jul 28, 2021
Open
@gardener-robot-ci-1 gardener-robot-ci-1 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 Jul 28, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 removed 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 Jul 28, 2021
@AxiomSamarth
Copy link
Collaborator Author

/invite @DockToFuture @ScheererJ

kon-angelo
kon-angelo approved these changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@kon-angelo kon-angelo 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 labels Jul 29, 2021
@dkistner
Copy link
Member

dkistner commented Jul 29, 2021 •
edited
Loading

/needs second-opinion
/squash

@gardener-robot gardener-robot added merge/squash Should be merged via 'Squash and merge' needs/second-opinion Needs second review by someone else and removed reviewed/lgtm Has approval for merging labels Jul 29, 2021
@gardener gardener deleted a comment from gardener-robot Jul 29, 2021
@dkistner
Copy link
Member

/assign @ScheererJ @DockToFuture

DockToFuture
DockToFuture approved these changes Jul 29, 2021
View reviewed changes
Copy link
Member

@DockToFuture DockToFuture 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 labels Jul 29, 2021
@kon-angelo kon-angelo mentioned this pull request Jul 30, 2021
Merged
prashanth26
prashanth26 approved these changes Jul 31, 2021
View reviewed changes
Copy link

@prashanth26 prashanth26 left a comment

Choose a reason for hiding this comment

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

Minor NIT.
/lgtm otherwise.
/squash

pkg/aws/core_util.go Outdated Show resolved Hide resolved
@gardener-robot
Copy link

@ScheererJ You have pull request review open invite, please check

@gardener-robot-ci-2 gardener-robot-ci-2 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 Aug 2, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 removed 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 Aug 2, 2021
@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 Aug 2, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 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 Aug 2, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 removed 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 Aug 2, 2021
@prashanth26 prashanth26 merged commit 2112bd8 into gardener:master Aug 2, 2021
@prashanth26 prashanth26 mentioned this pull request Aug 12, 2021
Closed
This was referenced Aug 2, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kon-angelo kon-angelo kon-angelo approved these changes

@prashanth26 prashanth26 prashanth26 approved these changes

@DockToFuture DockToFuture DockToFuture approved these changes

@ScheererJ ScheererJ Awaiting requested review from ScheererJ

Assignees

@DockToFuture DockToFuture

@ScheererJ ScheererJ

Labels
merge/squash Should be merged via 'Squash and merge' needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) 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.

Make src/dst checks configurable on awsmachineclass
10 participants
@AxiomSamarth @dkistner @gardener-robot @DockToFuture @prashanth26 @kon-angelo @ScheererJ @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3