This repository was archived by the owner on Jul 11, 2023. It is now read-only.
[backport] traffic-split: update root service selector & targetPort usage (#4902) #4905
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ervicemesh#4902) This change does the following: 1. Fixes the incorrect legacy behavior where traffic directed to a root service specified in a TrafficSplit resource can direct traffic to pods that do not match the root service's selector. Not only was this behavior confusing, it also significantly complicated code paths that required special handling of this scenario that is unintuitive. Going forward, the root service selector must match pods for traffic splitting to those pods to function. Existing e2e tests relying on this unsupported behavior have been updated to correctly configure selectors and labels on services and pods backing them. A redundant test explicitly testing the only supported scenario after this change has been removed. The automated demo has also been updated to correctly configure the selector and labels. 2. Fixes openservicemesh#4894, where the TargetPort on the root service was expected to match the ContainerPort on the pod backing the service. Per SMI's TrafficSplit API, the TargetPort on the root does not need to match the ContainerPort on the pod, thus allowing newer application versions to change the ports they listen on without needing to update the root service definition. Signed-off-by: Shashank Ram <shashr2204@gmail.com>
Adds a note regarding the change to drop support to split traffic from a root service to pods that do not match the selector on the root service. Signed-off-by: Shashank Ram <shashr2204@gmail.com>
Codecov Report
@@ Coverage Diff @@
## release-v1.2 #4905 +/- ##
================================================
- Coverage 68.68% 68.63% -0.06%
================================================
Files 220 220
Lines 15914 15918 +4
================================================
- Hits 10931 10925 -6
- Misses 4931 4941 +10
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
keithmattix
approved these changes
Jul 16, 2022
trstringer
approved these changes
Jul 18, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backports 5f54056 to release-v1.2
This change does the following:
Fixes the incorrect legacy behavior where traffic
directed to a root service specified in a TrafficSplit
resource can direct traffic to pods that do not match
the root service's selector. Not only was this behavior
confusing, it also significantly complicated code paths
that required special handling of this scenario that is
unintuitive. Going forward, the root service selector
must match pods for traffic splitting to those pods to
function. Existing e2e tests relying on this unsupported
behavior have been updated to correctly configure selectors
and labels on services and pods backing them. A redundant
test explicitly testing the only supported scenario after
this change has been removed. The automated demo has
also been updated to correctly configure the selector and
labels.
Fixes Specifying root service port to a port other than containerPort/TargetPort of the leaf service doesn't not work #4894, where the TargetPort on the root service was
expected to match the ContainerPort on the pod backing
the service. Per SMI's TrafficSplit API, the TargetPort
on the root does not need to match the ContainerPort
on the pod, thus allowing newer application versions
to change the ports they listen on without needing
to update the root service definition.
Please answer the following questions with yes/no.
Does this change contain code from or inspired by another project?
noIs this a breaking change?
yesHas documentation corresponding to this change been updated in the osm-docs repo (if applicable)?
not yet