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

Ability to specify subnets for NLB #1667

Merged
merged 3 commits into from Nov 18, 2020
Merged

Ability to specify subnets for NLB #1667

merged 3 commits into from Nov 18, 2020

Conversation

kishorj
Copy link
Collaborator

@kishorj kishorj commented Nov 18, 2020

Fixes #1576 Ability to specify subnets for NLB

Support new annotation service.beta.kubernetes.io/aws-load-balancer-subnets on service objects. Annotation is of type string list containing subnet ids or subnet names from one ore more AZs.

Tests run:

  • Create NLB IP service without subnets annotation, verify auto-discovery works as expected
  • Create NLB IP service with the following subnets annotation, and verify NLB got provisioned appropriately - 
     service.beta.kubernetes.io/aws-load-balancer-subnets: "subnet-xxx, eksctl-xyz-cluster/SubnetPublicUSWEST2C"
  • Added a new subnet from a different AZ to the service created earlier, and verified NLB got updated
  • Unable to specify subnets annotation after service creation unless the subnets in the annotatoin match the auto-discovered ones
  • unable to delete subnets due to AWS temporary limitations
  • subnets order in the annotation did not matter

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 18, 2020
M00nF1sh
M00nF1sh approved these changes Nov 18, 2020
View reviewed changes
Copy link
Collaborator

@M00nF1sh M00nF1sh left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kishorj, M00nF1sh
To complete the pull request process, please assign after the PR has been reviewed.
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found 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 removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2020
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@M00nF1sh M00nF1sh merged commit 72cef4c into kubernetes-sigs:main Nov 18, 2020
@kishorj kishorj deleted the main-nlb-subnets branch December 24, 2020 00:36
Timothy-Dougherty pushed a commit to adammw/aws-load-balancer-controller that referenced this pull request Nov 9, 2023
@kishorj @Timothy-Dougherty
Ability to specify subnets for NLB (kubernetes-sigs#1667)
f51ade8 
* ability to specify subnets for NLB

* update resolver function

* fix doc typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@M00nF1sh M00nF1sh M00nF1sh approved these changes

Assignees

@M00nF1sh M00nF1sh

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] Allow manual subnet specification for NLB
3 participants
@kishorj @k8s-ci-robot @M00nF1sh