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

🌱 refactor route discover/creation flow #4900

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

mtulio
Copy link
Contributor

@mtulio mtulio commented Apr 3, 2024

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

The flow for route discover and creation is reviewed to provide flexibility of route entry inputs for routes discovered by each subnet.

This change is a subset of Wavelength zone (#4874 , #4901) feature which will introduce requirements when discovering gateways of public and private subnets.

The refact should not change the existing flow.

Which issue(s) this PR fixes None

Special notes for your reviewer:

Checklist:

  • squashed commits
  • includes documentation (N/A)
  • includes emojis
  • adds unit tests
  • adds or updates e2e tests (N/A)

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 3, 2024
@k8s-ci-robot k8s-ci-robot added needs-priority size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 3, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @mtulio. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Apr 3, 2024
@mtulio mtulio marked this pull request as ready for review April 3, 2024 06:14
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 3, 2024
@mtulio
Copy link
Contributor Author

mtulio commented Apr 3, 2024

For reviewers: this is ready for review. Could you please take a look?

@nrb
Copy link
Contributor

nrb commented Apr 3, 2024

/retitle 🌱 refactor route discover/creation flow
/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Apr 3, 2024
@k8s-ci-robot k8s-ci-robot changed the title 🌱 refact/network/routetable: review route discover/creation flow 🌱 refactor route discover/creation flow Apr 3, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 3, 2024
@mtulio
Copy link
Contributor Author

mtulio commented Apr 3, 2024

/test pull-cluster-api-provider-aws-e2e

The flow for route discover and creation is reviewed to provide flexibility
of route entry inputs for routes discovered by each subnet.

This change is a subset of Wavelength zone (kubernetes-sigs#4874) feature which will
introduce requirements when discovering gateways of public and private subnets.

The refact should not change the existing flow.
@mtulio
Copy link
Contributor Author

mtulio commented Apr 12, 2024

This PR is ready for review. PTAL?
/assign @nrb @damdo

mtulio added a commit to mtulio/cluster-api-provider-aws that referenced this pull request Apr 12, 2024
Isolate the route table lookup into dedicated methods for private and
public subnets to allow more complex requirements for edge zones, as
well introduce unit tests for each scenario to cover edge cases.

There is no change for private and public subnets for regular
zones (standard flow), and the routes will be assigned accordainly
the existing flow: private subnets uses nat gateways per public zone,
and internet gateway for public zones's tables.

For private and public subnets in edge zones, the following changes is
introduced according to each rule:

General:

- IPv6 subnets is not be supported in AWS Local Zones,
  zone, consequently no ip6 routes will be created
- nat gateways is not supported, default gateway's route for private
  subnets will use nat gateways from the zones in the Region
(availability-zone's zone type)
- one route table by zone's role by zone (standard flow)

Private tables for Local Zones:
- default route's gateways is assigned using nat gateway created in
  the region (availability-zones).

Public tables for Local Zones:
- default route's gateway is assigned using internet gateway

The changes in the standard flow (without edge subnets' support) was
isolated in the PR kubernetes-sigs#4900
@nrb
Copy link
Contributor

nrb commented Apr 18, 2024

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 18, 2024
@richardcase
Copy link
Member

/test ?

@k8s-ci-robot
Copy link
Contributor

@richardcase: The following commands are available to trigger required jobs:

  • /test pull-cluster-api-provider-aws-build
  • /test pull-cluster-api-provider-aws-build-docker
  • /test pull-cluster-api-provider-aws-test
  • /test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-provider-aws-apidiff-main
  • /test pull-cluster-api-provider-aws-e2e
  • /test pull-cluster-api-provider-aws-e2e-blocking
  • /test pull-cluster-api-provider-aws-e2e-clusterclass
  • /test pull-cluster-api-provider-aws-e2e-conformance
  • /test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
  • /test pull-cluster-api-provider-aws-e2e-eks
  • /test pull-cluster-api-provider-aws-e2e-eks-gc
  • /test pull-cluster-api-provider-aws-e2e-eks-testing

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-provider-aws-apidiff-main
  • pull-cluster-api-provider-aws-build
  • pull-cluster-api-provider-aws-build-docker
  • pull-cluster-api-provider-aws-test
  • pull-cluster-api-provider-aws-verify

In response to this:

/test ?

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.

@richardcase
Copy link
Member

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-ek

@richardcase
Copy link
Member

/test pull-cluster-api-provider-aws-e2e-eks

@mtulio
Copy link
Contributor Author

mtulio commented Apr 19, 2024

Hey @nrb @richardcase e2e* are passing now! 🚀

@mtulio
Copy link
Contributor Author

mtulio commented Apr 20, 2024

/assign richardcase

@richardcase
Copy link
Member

The e2e passed (both eks and non-eks), from my side:

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 22, 2024
@k8s-ci-robot k8s-ci-robot merged commit 810bbf4 into kubernetes-sigs:main Apr 22, 2024
31 checks passed
mtulio added a commit to mtulio/cluster-api-provider-aws that referenced this pull request Apr 22, 2024
✨ edge subnets/routes: supporting custom routes for Local Zones

Isolate the route table lookup into dedicated methods for private and
public subnets to allow more complex requirements for edge zones, as
well introduce unit tests for each scenario to cover edge cases.

There is no change for private and public subnets for regular
zones (standard flow), and the routes will be assigned accordainly
the existing flow: private subnets uses nat gateways per public zone,
and internet gateway for public zones's tables.

For private and public subnets in edge zones, the following changes is
introduced according to each rule:

General:

- IPv6 subnets is not be supported in AWS Local Zones,
  zone, consequently no ip6 routes will be created
- nat gateways is not supported, default gateway's route for private
  subnets will use nat gateways from the zones in the Region
(availability-zone's zone type)
- one route table by zone's role by zone (standard flow)

Private tables for Local Zones:
- default route's gateways is assigned using nat gateway created in
  the region (availability-zones).

Public tables for Local Zones:
- default route's gateway is assigned using internet gateway

The changes in the standard flow (without edge subnets' support) was
isolated in the PR kubernetes-sigs#4900

✨ edge subnets/nat-gw: support private routing in Local Zones

Introduce the support to lookup a nat gateway for edge zones when
creating private subnets.

Currently CAPA requires a NAT Gateway in the public subnet for each zone
which requires private subnets to define default nat gateway in the
private route table for each zone.

NAT Gateway resource isn't globally supported by Local Zones, thus
private subnets in Local Zones are created with default route gateway
using a nat gateway selected in the Region (regular availability zones)
based in the Parent Zone* for the edge subnet.

*each edge zone is "tied" to a zone named "Parent Zone", a zone type
availability-zone (regular zones) in the region.
@nrb nrb added this to the v2.5.0 milestone Apr 22, 2024
mtulio added a commit to mtulio/cluster-api-provider-aws that referenced this pull request Apr 24, 2024
✨ edge subnets/cagw: carrier gateway for public subnets in Wavelength

Introduce Carrier Gateway resource reconciliator in the network service.

Carrier Gateway is the gateway responsible to route ingress and egress
traffic **in/out the Wavelength Zone**, located in the Carrier
Infrastructure - communications service providers’ (CSP) 5G networks.

Carrier Gateway is similar Internet Gatewat resource, responsible for
the network border groups in the Region and Local Zones for public
subnets.

✨ edge subnets/routes: supporting custom routes for Wavelength

For private and public subnets in edge zones, the following changes is
introduced according to each rule:

General:

- IPv6 subnets is not be supported in AWS Local Zones and Wavelength
  zone, consequently no ip6 routes will be created
- nat gateways is not supported, default gateway's route for private
  subnets will use nat gateways from the zones in the Region
(availability-zone's zone type)
- one route table by zone's role by zone (standard flow)

Private tables for Local Zones and Wavelength:
- default route's gateways is assigned using nat gateway created in
  the region (availability-zones).

Public tables for Wavelength zones:
- default route's gateways is assigned using Carrier Gateway, resource
  introduced in the edge zone's feature.

The changes in the standard flow (without edge subnets' support) was
isolated in the PR kubernetes-sigs#4900
fad3t pushed a commit to fad3t/cluster-api-provider-aws that referenced this pull request Jul 25, 2024
✨ edge subnets/routes: supporting custom routes for Local Zones

Isolate the route table lookup into dedicated methods for private and
public subnets to allow more complex requirements for edge zones, as
well introduce unit tests for each scenario to cover edge cases.

There is no change for private and public subnets for regular
zones (standard flow), and the routes will be assigned accordainly
the existing flow: private subnets uses nat gateways per public zone,
and internet gateway for public zones's tables.

For private and public subnets in edge zones, the following changes is
introduced according to each rule:

General:

- IPv6 subnets is not be supported in AWS Local Zones,
  zone, consequently no ip6 routes will be created
- nat gateways is not supported, default gateway's route for private
  subnets will use nat gateways from the zones in the Region
(availability-zone's zone type)
- one route table by zone's role by zone (standard flow)

Private tables for Local Zones:
- default route's gateways is assigned using nat gateway created in
  the region (availability-zones).

Public tables for Local Zones:
- default route's gateway is assigned using internet gateway

The changes in the standard flow (without edge subnets' support) was
isolated in the PR kubernetes-sigs#4900

✨ edge subnets/nat-gw: support private routing in Local Zones

Introduce the support to lookup a nat gateway for edge zones when
creating private subnets.

Currently CAPA requires a NAT Gateway in the public subnet for each zone
which requires private subnets to define default nat gateway in the
private route table for each zone.

NAT Gateway resource isn't globally supported by Local Zones, thus
private subnets in Local Zones are created with default route gateway
using a nat gateway selected in the Region (regular availability zones)
based in the Parent Zone* for the edge subnet.

*each edge zone is "tied" to a zone named "Parent Zone", a zone type
availability-zone (regular zones) in the region.
fad3t pushed a commit to fad3t/cluster-api-provider-aws that referenced this pull request Jul 25, 2024
✨ edge subnets/cagw: carrier gateway for public subnets in Wavelength

Introduce Carrier Gateway resource reconciliator in the network service.

Carrier Gateway is the gateway responsible to route ingress and egress
traffic **in/out the Wavelength Zone**, located in the Carrier
Infrastructure - communications service providers’ (CSP) 5G networks.

Carrier Gateway is similar Internet Gatewat resource, responsible for
the network border groups in the Region and Local Zones for public
subnets.

✨ edge subnets/routes: supporting custom routes for Wavelength

For private and public subnets in edge zones, the following changes is
introduced according to each rule:

General:

- IPv6 subnets is not be supported in AWS Local Zones and Wavelength
  zone, consequently no ip6 routes will be created
- nat gateways is not supported, default gateway's route for private
  subnets will use nat gateways from the zones in the Region
(availability-zone's zone type)
- one route table by zone's role by zone (standard flow)

Private tables for Local Zones and Wavelength:
- default route's gateways is assigned using nat gateway created in
  the region (availability-zones).

Public tables for Wavelength zones:
- default route's gateways is assigned using Carrier Gateway, resource
  introduced in the edge zone's feature.

The changes in the standard flow (without edge subnets' support) was
isolated in the PR kubernetes-sigs#4900
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants