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

helm: Allow node port allocation for Ingress LB service #26502

Merged
merged 1 commit into from Jul 12, 2023
Merged

helm: Allow node port allocation for Ingress LB service #26502

merged 1 commit into from Jul 12, 2023

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Jun 27, 2023 •
edited

Description

This commit is to make sure that users can have the option to disable node port allocation for LB service used by Ingress.

https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation

Testing

Testing was done locally for below cases:

  • Default case i.e. no helm option specified
  • Set helm flag ingressController.service.allocateLoadBalancerNodePorts=false
  • Set helm flag ingressController.service.allocateLoadBalancerNodePorts=true

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 27, 2023
@sayboras sayboras added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/helm Impacts helm charts and user deployment experience feature/k8s-ingress labels Jun 27, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 27, 2023
@sayboras sayboras force-pushed the tam/ingress-allocate-lb-node-port branch from d28666c to e1bde04 Compare June 27, 2023 10:46
@sayboras sayboras added the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Jun 27, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.5 Jun 27, 2023
@sayboras
Copy link
Member Author

/test

@sayboras sayboras force-pushed the tam/ingress-allocate-lb-node-port branch from e1bde04 to f4e39a2 Compare June 27, 2023 12:42
@sayboras
Copy link
Member Author

/test

@sayboras sayboras marked this pull request as ready for review June 27, 2023 12:42
@sayboras sayboras requested review from a team as code owners June 27, 2023 12:42
@sayboras sayboras force-pushed the tam/ingress-allocate-lb-node-port branch 2 times, most recently from ed3acb4 to d61ab56 Compare June 28, 2023 11:27
@sayboras sayboras requested a review from gandro June 28, 2023 11:29
learnitall
learnitall requested changes Jun 29, 2023
View reviewed changes
install/kubernetes/cilium/README.md Outdated Show resolved Hide resolved
@gandro
Copy link
Member

gandro commented Jul 3, 2023

Does this need the needs-backport/1.14 label?

@sayboras sayboras force-pushed the tam/ingress-allocate-lb-node-port branch from d61ab56 to 0af7cb1 Compare July 10, 2023 13:32
@sayboras
Copy link
Member Author

/test

learnitall
learnitall approved these changes Jul 10, 2023
View reviewed changes
Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

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

Looks good thanks!

gandro
gandro approved these changes Jul 10, 2023
View reviewed changes
Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

Thanks!

@sayboras sayboras added the release-blocker/1.13 This issue will prevent the release of the next version of Cilium. label Jul 12, 2023
@sayboras sayboras force-pushed the tam/ingress-allocate-lb-node-port branch from 0af7cb1 to 8b850e5 Compare July 12, 2023 08:36
@sayboras
Copy link
Member Author

/test

@sayboras sayboras force-pushed the tam/ingress-allocate-lb-node-port branch from 8b850e5 to 2c9481c Compare July 12, 2023 09:10
@sayboras
Copy link
Member Author

/test

@sayboras
Copy link
Member Author

Docs failure will be fixed in #26780

@sayboras
helm: Allow nodeport allocation for Ingress LB svc
908ceeb 
This commit is to make sure that users can have option to disable node
port allocation for LB service used by Ingress.

https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras sayboras force-pushed the tam/ingress-allocate-lb-node-port branch from 2c9481c to 908ceeb Compare July 12, 2023 11:09
@sayboras
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 12, 2023
@sayboras sayboras merged commit bbdad5d into cilium:main Jul 12, 2023
65 checks passed
@sayboras sayboras deleted the tam/ingress-allocate-lb-node-port branch July 12, 2023 21:42
@jibi jibi mentioned this pull request Jul 13, 2023
Merged
13 tasks
@jibi jibi added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 13, 2023
@aanm aanm added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Jul 14, 2023
@joestringer joestringer mentioned this pull request Jul 17, 2023
Merged
@gandro gandro mentioned this pull request Jul 17, 2023
Merged
6 tasks
@gandro gandro added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Jul 17, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport done to v1.14 in 1.14.0 Jul 17, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.13 in 1.13.5 Jul 17, 2023
@gandro gandro added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Jul 19, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.5 Jul 19, 2023
@gentoo-root gentoo-root mentioned this pull request Jul 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gandro gandro gandro approved these changes

@nathanjsweet nathanjsweet nathanjsweet approved these changes

@learnitall learnitall learnitall approved these changes

Assignees
No one assigned
Labels
area/helm Impacts helm charts and user deployment experience backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/k8s-ingress ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.13 This issue will prevent the release of the next version of Cilium. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.13.5
Backport done to v1.13
1.14.0
Backport done to v1.14
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@sayboras @gandro @nathanjsweet @learnitall @jibi @aanm