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

add blog for feature LoadBalancerIPMode #43904

Merged
merged 1 commit into from
Dec 13, 2023
Merged

Conversation

RyanAoh
Copy link
Member

@RyanAoh RyanAoh commented Nov 13, 2023

Blog for new feature LoadBalancerIPMode.

PR link: kubernetes/kubernetes#119937
KEP issue: kubernetes/enhancements#1860

@k8s-ci-robot k8s-ci-robot added this to the 1.29 milestone Nov 13, 2023
Copy link

netlify bot commented Nov 13, 2023

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit a2326a4
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/65705d3e1efceb000816b31e

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 13, 2023
@k8s-ci-robot k8s-ci-robot added area/blog Issues or PRs related to the Kubernetes Blog subproject language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 13, 2023
@RyanAoh RyanAoh force-pushed the dev-1.29 branch 2 times, most recently from c06030f to ba7ce92 Compare November 13, 2023 12:01
@dipesh-rawat
Copy link
Member

/sig network

@k8s-ci-robot k8s-ci-robot added the sig/network Categorizes an issue or PR as relevant to SIG Network. label Nov 13, 2023
@sftim
Copy link
Contributor

sftim commented Nov 13, 2023

Hi.

I'm afraid this PR should target main, not dev-1.29. Even if the announcement is related to Kubernetes v1.29

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 13, 2023
@RyanAoh RyanAoh changed the base branch from dev-1.29 to main November 13, 2023 23:17
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 13, 2023
@RyanAoh RyanAoh changed the base branch from main to dev-1.29 November 13, 2023 23:17
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 13, 2023
@k8s-ci-robot k8s-ci-robot added language/de Issues or PRs related to German language language/fr Issues or PRs related to French language language/id Issues or PRs related to Indonesian language language/ru Issues or PRs related to Russian language language/vi Issues or PRs related to Vietnamese language language/zh Issues or PRs related to Chinese language size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 13, 2023
@RyanAoh RyanAoh force-pushed the dev-1.29 branch 2 times, most recently from 7574d18 to 24d0461 Compare November 15, 2023 13:39
@krol3
Copy link
Contributor

krol3 commented Nov 22, 2023

Hi @RyanAoh , here Communication Team 1.29, the deadline to the feature blog be ready to review was this Friday, Nov 17th, the proposal publish date will be Dec 18th. cc: @a-mccarthy @kcmartin @James-Quigley

@kubernetes/sig-docs-blog-owners: Blog scheduled: Dec 18th, Publication Order Nro:6

Copy link
Contributor

Choose a reason for hiding this comment

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

OK to unhold once the date (check the path and the front matter) matches the assigned date, 2023-12-18.

Copy link
Member Author

Choose a reason for hiding this comment

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

The date both in the file path and front matter have been updated to 2023-12-18.

@RyanAoh RyanAoh force-pushed the dev-1.29 branch 2 times, most recently from 7a698c8 to a061e7f Compare November 23, 2023 13:45
@RyanAoh RyanAoh requested a review from sftim November 23, 2023 14:04
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Some feedback.

Comment on lines 34 to 35
transmitting packets to the node. In the ipvs mode of kube-proxy,
there is a problem that health checks from the load balancer never return as the IP is bound to an interface.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
transmitting packets to the node. In the ipvs mode of kube-proxy,
there is a problem that health checks from the load balancer never return as the IP is bound to an interface.
transmitting packets to the node. In the ipvs mode of kube-proxy,
there is a problem that health checks from the load balancer never return (the health checks fail
as the associated listening socket is bound to an interface that the load balancer cannot reach).

Copy link
Member Author

Choose a reason for hiding this comment

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

The load balancer can reach the listening socket but the reply packets can not out the node. I chane the description here. PTAL.

Comment on lines 10 to 13
This blog introduces `LoadBalancerIPMode`, a new alpha feature in Kubernetes 1.29.
It provides a configurable approach to define how service implementations,
exemplified in this blog by kube-proxy,
handle traffic from pods to the `service.status.loadbalancer.ingress.ip` within the cluster.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This blog introduces `LoadBalancerIPMode`, a new alpha feature in Kubernetes 1.29.
It provides a configurable approach to define how service implementations,
exemplified in this blog by kube-proxy,
handle traffic from pods to the `service.status.loadbalancer.ingress.ip` within the cluster.
This blog introduces a new alpha feature in Kubernetes 1.29.
It provides a configurable approach to define how Service implementations,
exemplified in this blog by kube-proxy,
handle traffic from pods to the Service, within the cluster.

Comment on lines 65 to 68
Given that `EnsureLoadBalancer` returns a `LoadBalancerStatus`,
the `ipMode` field can be set by the cloud-controller-manager before returning the status.
It is more appropriate to delegate this decision to cloud providers through the cloud-controller-manager
rather than relying on end users, who may not be familiar with these technical details.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd skip this:

Suggested change
Given that `EnsureLoadBalancer` returns a `LoadBalancerStatus`,
the `ipMode` field can be set by the cloud-controller-manager before returning the status.
It is more appropriate to delegate this decision to cloud providers through the cloud-controller-manager
rather than relying on end users, who may not be familiar with these technical details.

sftim
sftim previously requested changes Dec 4, 2023
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks for proposing this article.

This PR has missed the deadline for PRs to be ready for review; you should promptly address the pending feedback. This article is at risk of being omitted from post-release comms.

@RyanAoh
Copy link
Member Author

RyanAoh commented Dec 5, 2023

@sftim Sorry for not responding promptly. I have made a new commit incorporating your feedback. PTAL, thanks.

@RyanAoh RyanAoh force-pushed the dev-1.29 branch 2 times, most recently from c0b3bbb to 7226012 Compare December 5, 2023 02:22
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks. Here's some feedback.

The big change to make is to write this article as if Kubernetes v1.29 has been released; we won't publish the article until the release has happened.

The default value is "VIP", meaning that traffic delivered to the node
with the destination set to the load balancer's IP and port will be redirected to the backend service by kube-proxy.
This preserves the existing behavior of kube-proxy.
The "Proxy" value is intended to prevent kube-proxy from binding the Load Balancer IP to the node in both ipvs and iptables modes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The "Proxy" value is intended to prevent kube-proxy from binding the Load Balancer IP to the node in both ipvs and iptables modes.
The "Proxy" value is intended to prevent kube-proxy from binding the load balancer's IP address to the node
any mode (iptables, ipvs, nftables).

?

What about Windows behavior? There's another operating mode for kube-proxy and the article as written makes it sound like there isn't.

Copy link
Member Author

@RyanAoh RyanAoh Dec 6, 2023

Choose a reason for hiding this comment

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

Oops, the mode for Windows was completely overlooked. Upon checking the code, I found that it hasn't been implemented for Windows at all. This is a mistake. As the code for v1.29.0 has already been frozen. I will patch this after its release.

Copy link
Contributor

Choose a reason for hiding this comment

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

Windows doesn't have nftables, but there is a kube-proxy mode for Windows: kernelspace

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a k/k issue tracked?

Copy link
Member Author

@RyanAoh RyanAoh Dec 6, 2023

Choose a reason for hiding this comment

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

I created a new one(kubernetes/kubernetes#122202).

@sftim sftim dismissed their stale review December 5, 2023 17:38

Superseded

@sftim
Copy link
Contributor

sftim commented Dec 6, 2023

/lgtm
/approve

We can publish this article as it stands.

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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
Copy link
Contributor

LGTM label has been added.

Git tree hash: afa66e8f70367f93702e35f607382128bc43c36a

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2023
@sftim
Copy link
Contributor

sftim commented Dec 13, 2023

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 13, 2023
@k8s-ci-robot k8s-ci-robot merged commit cd76d21 into kubernetes:main Dec 13, 2023
6 checks passed
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. area/blog Issues or PRs related to the Kubernetes Blog subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/network Categorizes an issue or PR as relevant to SIG Network. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: Published
Development

Successfully merging this pull request may close these issues.

5 participants