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

Update document antrea-proxy.md #6274

Merged
merged 1 commit into from
May 8, 2024
Merged

Update document antrea-proxy.md #6274

merged 1 commit into from
May 8, 2024

Conversation

hongliangl
Copy link
Contributor

Resolve #6263

@hongliangl hongliangl added the kind/documentation Categorizes issue or PR as related to a documentation. label Apr 29, 2024
antoninbas
antoninbas reviewed Apr 30, 2024
View reviewed changes
docs/antrea-proxy.md Outdated Show resolved Hide resolved
docs/antrea-proxy.md Outdated
@@ -253,6 +253,24 @@ data:
proxyLoadBalancerIPs: false
```

It's worth noting that AntreaProxy will bypass all the external `loadBalancerIP`s
with the above configuration. After K8s v1.29, field `LoadBalancerIPMode` was
introduced to `LoadBalancerIngress` in Service status, providing users with a
Copy link
Contributor

Choose a reason for hiding this comment

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

What's LoadBalancerIngress? I don't think it's the name of a field or a feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You could find it in this link https://github.com/kubernetes/kubernetes/blob/3f7a50f38688eb332e2a1b013678c6435d539ae6/staging/src/k8s.io/api/core/v1/types.go#L4867. I will rephrase the sentences containing the word LoadBalancerIngress that confuses readers.

docs/antrea-proxy.md Outdated Show resolved Hide resolved
docs/antrea-proxy.md Outdated Show resolved Hide resolved
docs/antrea-proxy.md Outdated Show resolved Hide resolved
docs/antrea-proxy.md Outdated Show resolved Hide resolved
docs/antrea-proxy.md Outdated Show resolved Hide resolved
docs/antrea-proxy.md Outdated Show resolved Hide resolved
docs/antrea-proxy.md Outdated

After Antrea v2.0, AntreaProxy respects `LoadBalancerIPMode` in Service status when
`proxyLoadBalancerIPs` is set to `true`. In this case, AntreaProxy serves only the
external `LoadBalancerIP`s configured with `LoadBalancerIPModeVIP`, bypassing those
Copy link
Contributor

Choose a reason for hiding this comment

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

s/bypassing/ignoring

same below

The usage is not correct here IMO, because it is the Service traffic that bypasses AntreaProxy in this case, not AntreaProxy that bypasses the Service traffic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. The external LoadBalancerIPs configured with LoadBalancerIPModeProxy will not be consumed by AntreaProxy, and AntreaProxy never has a chance to bypass them. For those configured with LoadBalancerIPModeVIP , they are consumed by AntreaProxy, and AntreaProxy can be able to decide to bypass all of them depending on the option proxyLoadBalancerIPs.

@hongliangl hongliangl requested a review from antoninbas May 6, 2024 02:48
luolanzone
luolanzone reviewed May 6, 2024
View reviewed changes
docs/antrea-proxy.md
@@ -253,6 +253,26 @@ data:
proxyLoadBalancerIPs: false
```

With the above configuration, AntreaProxy will ignore all external `loadBalancerIP`s.
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose it should be loadBalancerIPs.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used to be consistent with others in this document. See L240.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the current version is fine, but you have to be consistent between loadBalancerIPs and LoadBalancerIPs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

docs/antrea-proxy.md Outdated
the default behavior and get load-balanced at the source Node.
- If the value of `LoadBalancerIPMode` is `LoadBalancerIPModeProxy`, the traffic
destined for the corresponding external `LoadBalancerIP` should be sent to the
external loadBalancer.
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
external loadBalancer.
external load balancer.

or LoadBalancer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! It should be external LoadBalancer.

docs/antrea-proxy.md Outdated

Starting with Antrea v2.0, AntreaProxy will respect `LoadBalancerIPMode` in LoadBalancer
Services when the configuration option `proxyLoadBalancerIPs` is set to `true`
(default). In this case, AntreaProxy will serve only the external `LoadBalancerIP`s
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto, is LoadBalancerIPs or loadBalancerIPs? keep them consistent.

docs/antrea-proxy.md Outdated
(default). In this case, AntreaProxy will serve only the external `LoadBalancerIP`s
configured with `LoadBalancerIPModeVIP`, as those configured with `LoadBalancerIPMode`
bypass AntreaProxy. If the configuration option `proxyLoadBalancerIPs` is set to
`false`, AntreaProxy will ignore the external `LoadBalancerIP`s even if configured with
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

antoninbas
antoninbas reviewed May 6, 2024
View reviewed changes
docs/antrea-proxy.md Outdated Show resolved Hide resolved
docs/antrea-proxy.md
@@ -253,6 +253,26 @@ data:
proxyLoadBalancerIPs: false
```

With the above configuration, AntreaProxy will ignore all external `loadBalancerIP`s.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the current version is fine, but you have to be consistent between loadBalancerIPs and LoadBalancerIPs

antoninbas
antoninbas reviewed May 7, 2024
View reviewed changes
docs/antrea-proxy.md Outdated
Starting with Antrea v2.0, AntreaProxy will respect `LoadBalancerIPMode` in LoadBalancer
Services when the configuration option `proxyLoadBalancerIPs` is set to `true`
(default). In this case, AntreaProxy will serve only the external `loadBalancerIP`s
configured with `LoadBalancerIPModeVIP` and those configured with
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
configured with `LoadBalancerIPModeVIP` and those configured with
configured with `LoadBalancerIPModeVIP`, and those configured with

You need to keep the comma, otherwise the sentence reads a bit differently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I should notice that your review comment #6274 (comment) doesn't include the comma.

@hongliangl
Update document antrea-proxy.md
7bb8bc2 
Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
@hongliangl hongliangl requested a review from antoninbas May 8, 2024 00:21
@antoninbas antoninbas added the action/backport Indicates a PR that requires backports. label May 8, 2024
@antoninbas
Copy link
Contributor

/skip-all

@antoninbas
Copy link
Contributor

@hongliangl please backport to release-2.0

@antoninbas antoninbas merged commit 5251502 into antrea-io:main May 8, 2024
51 of 54 checks passed
hongliangl added a commit to hongliangl/antrea that referenced this pull request May 8, 2024
@hongliangl
Mention LoadbalancerIPMode support in AntreaProxy doc(antrea-io#6274)
19ff0a4 
In particular, we document how it interacts with the Antrea-specific
configuration parameter `proxyLoadBalancerIPs`.

Fixes antrea-io#6263

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
@hongliangl hongliangl mentioned this pull request May 8, 2024
Merged
@hongliangl hongliangl deleted the 20240429-antrea-proxy-doc branch May 8, 2024 02:33
antoninbas pushed a commit that referenced this pull request May 8, 2024
@hongliangl
Mention LoadbalancerIPMode support in AntreaProxy doc(#6274) (#6306)
5e24bca 
In particular, we document how it interacts with the Antrea-specific
configuration parameter `proxyLoadBalancerIPs`.

Fixes #6263

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoninbas antoninbas antoninbas approved these changes

@luolanzone luolanzone Awaiting requested review from luolanzone

Assignees
No one assigned
Labels
action/backport Indicates a PR that requires backports. kind/documentation Categorizes issue or PR as related to a documentation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Mention LoadBalancerIPMode support in AntreaProxy documentation
3 participants
@hongliangl @antoninbas @luolanzone