clarify unmentioned constraint port.name equalness #11680

tedli · 2022-08-04T02:58:01Z

Please provide a description for what this PR is for.

In our multi cluster mesh, we found that, even we meet the namespace-sameness, cross cluster calls to remote cluster through eastwest gateway never happen in some cases. (calls always invoke to primary cluster)
After some digging, we found that the same kubernetes Service had different port.name, and the code that aggregating eds reads the port.name to match endpoint with service.
And if we correct the Service.spec.ports[_].name to be same as what in primary cluster (literally equal, both name no specified consider equal "" == "" ), cross cluster calls act as expect.
So as namespace-sameness, the addition constraint should be documented.

https://github.com/istio/istio/blob/93e4ec424b52da807228bd7784c59d646ec73733/pilot/pkg/xds/endpoint_builder.go#L240-L242

for _, ep := range endpoints {
			// TODO(nmittler): Consider merging discoverability policy with cluster-local
			if !ep.IsDiscoverableFromProxy(b.proxy) {
				continue
			}
			if svcPort.Name != ep.ServicePortName {
				continue
			}

And to help us figure out who should review this PR, please
put an X in all the areas that this PR affects.

In our multi cluster mesh, we found that, even we meet the `namespace-sameness`, cross cluster calls to remote cluster through eastwest gateway never happen in some cases. (calls always invoke to primary cluster) After some digging, we found that the `same` kubernetes `Service` had different `port.name`, and the code aggregating eds read port.name to match endpoint with service. And if correct `Service.spec.ports[_].name` to be same as what in primary cluster (literally equal, both name no specified consider equal "" == "" ), cross cluster calls act as expect. So as `namespace-sameness`, the addition constraint should be documented.

istio-policy-bot · 2022-08-04T02:58:04Z

😊 Welcome! This is either your first contribution to the Istio documentation repo, or
it's been awhile since you've been here. A few things you should know:

You can learn about how we write and maintain documentation, about our style guidelines,
and about all the available web site features by visiting Contributing to the Docs.
In the next few minutes, an automatic preview of your change will be
built as a full copy of the istio.io website. You can find this preview by clicking on
the Details link next to the deploy/netlify entry in the Status section of this
page.
We care about quality, so we've put in place a number of checks to ensure our documentation
is top notch. We do spell checking, we sanitize the markdown, we ensure all hyperlinks point
to valid location, and more. If your PR doesn't pass one of these checks, you'll see a red X in the
status section of the page. Click on the Details link to get a list of the problems with your PR.
Fix those problems and push an update to your PR. This will automatically rerun the tests and
hopefully this time everything will be perfect.
Once your changes are accepted and merged into the repository, they will initially show up
on https://preliminary.istio.io. The changes will be published to https://istio.io
the next time we do a major release (which typically happens every 3 months or so).

Thanks for contributing!

Courtesy of your friendly welcome wagon.

istio-testing · 2022-08-04T02:58:14Z

Hi @tedli. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tedli · 2022-08-05T03:34:08Z

/ok-to-test

istio-testing · 2022-08-05T03:34:32Z

@tedli: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/ok-to-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

zirain · 2022-08-05T06:08:06Z

/ok-to-test

tedli · 2022-08-05T07:07:56Z

/retest

content/en/docs/reference/glossary/namespace-sameness.md

craigbox · 2022-08-12T14:38:30Z

WG Network, PTAL

content/en/docs/reference/glossary/namespace-sameness.md

ericvn · 2022-08-31T19:10:57Z

This PR is targeted for the release-1.14 branch which will shortly be archived and likely not archived again. Thus this PR will not show up in the 1.14 version of the documentation. This PR should be rebased on the main branch (and cherry-picked to 1.15 if needed).

craigbox · 2022-09-12T02:29:07Z

@tedli could you please rebase this on master or release-1.15?

tedli · 2022-09-12T03:38:20Z

@tedli could you please rebase this on master or release-1.15?

Hi craigbox,

Thanks for reply. I cherry-picked this to master and release-1.15. Please have a look.

#11892
#11891

ericvn · 2022-09-12T13:45:28Z

@craigbox If we approve this one, it will be cp'd to the other releases. This PR won't matter too much, unless we rebuild the 1.14 archive. But at least the tooling should cherry-pick to the istio and preliminary istio branches.

Else, we could close this and approve the one in master and cherry-pick that to 1.15.

craigbox · 2022-10-08T04:48:06Z

content/en/docs/reference/glossary/namespace-sameness.md

+load-balanced across all clusters in the mesh for a given service.
+
+For one or many ports within the combined service, same port value should also have same port name.


Suggested change

For one or many ports within the combined service, same port value should also have same port name.

Ports that match on _number_ must also have the same port _name_ to be considered as a combined service port.```

… (#11891) * doc the port name should be same * namespace sameness, accept craigbox's suggestion

…#11680) (istio#11891) * doc the port name should be same * namespace sameness, accept craigbox's suggestion

tedli requested a review from a team as a code owner August 4, 2022 02:58

istio-policy-bot added the kind/docs label Aug 4, 2022

istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 4, 2022

istio-testing added the needs-ok-to-test label Aug 4, 2022

tedli mentioned this pull request Aug 5, 2022

fix empty endpoints weight for AUTO_PASSTHROUGH gateway istio/istio#40243

Merged

istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Aug 5, 2022

fix spelling errors reported from test

0ad7956

ericvn added the area/networking label Aug 8, 2022

craigbox reviewed Aug 12, 2022

View reviewed changes

content/en/docs/reference/glossary/namespace-sameness.md Show resolved Hide resolved

craigbox requested changes Aug 12, 2022

View reviewed changes

content/en/docs/reference/glossary/namespace-sameness.md Outdated Show resolved Hide resolved

craigbox requested a review from a team August 12, 2022 14:38

craigbox added cherrypick/master Set this label on a PR to auto-merge from a release branch to master cherrypick/release-1.15 labels Aug 12, 2022

howardjohn reviewed Aug 12, 2022

View reviewed changes

content/en/docs/reference/glossary/namespace-sameness.md Outdated Show resolved Hide resolved

doc the port name should be same

43dec6b

tedli force-pushed the patch-1 branch from 896a097 to 43dec6b Compare August 16, 2022 03:18

This was referenced Sep 12, 2022

clarify unmentioned constraint port.name equalness (cherry-pick #11680) #11891

Merged

clarify unmentioned constraint port.name equalness (cherry-pick #11680) #11892

Closed

craigbox requested changes Oct 8, 2022

View reviewed changes

tedli added 2 commits October 8, 2022 13:38

Merge branch 'patch-1' of ssh://github.com/tedli/istio.io into patch-1

01a89fe

namespace sameness, accept craigbox's suggestion

53e6d3f

craigbox approved these changes Nov 11, 2022

View reviewed changes

istio-testing merged commit d549481 into istio:release-1.14 Nov 11, 2022

istio-testing pushed a commit that referenced this pull request Nov 11, 2022

clarify unmentioned constraint port.name equalness (cherry-pick #11680)…

db3731f

… (#11891) * doc the port name should be same * namespace sameness, accept craigbox's suggestion

justedennnnn pushed a commit to justedennnnn/istio.io that referenced this pull request Jun 29, 2023

clarify unmentioned constraint port.name equalness (cherry-pick istio…

da95ef5

…#11680) (istio#11891) * doc the port name should be same * namespace sameness, accept craigbox's suggestion

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

clarify unmentioned constraint port.name equalness #11680

clarify unmentioned constraint port.name equalness #11680

Uh oh!

tedli commented Aug 4, 2022 •

edited

Loading

Uh oh!

istio-policy-bot commented Aug 4, 2022

Uh oh!

istio-testing commented Aug 4, 2022

Uh oh!

tedli commented Aug 5, 2022

Uh oh!

istio-testing commented Aug 5, 2022

Uh oh!

zirain commented Aug 5, 2022

Uh oh!

tedli commented Aug 5, 2022

Uh oh!

Uh oh!

Uh oh!

craigbox commented Aug 12, 2022

Uh oh!

Uh oh!

ericvn commented Aug 31, 2022

Uh oh!

craigbox commented Sep 12, 2022

Uh oh!

tedli commented Sep 12, 2022

Uh oh!

ericvn commented Sep 12, 2022 •

edited

Loading

Uh oh!

craigbox Oct 8, 2022

Uh oh!

Uh oh!

		load-balanced across all clusters in the mesh for a given service.

		For one or many ports within the combined service, same port value should also have same port name.

	For one or many ports within the combined service, same port value should also have same port name.
	Ports that match on _number_ must also have the same port _name_ to be considered as a combined service port.```

clarify unmentioned constraint port.name equalness #11680

clarify unmentioned constraint port.name equalness #11680

Uh oh!

Conversation

tedli commented Aug 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

istio-policy-bot commented Aug 4, 2022

Uh oh!

istio-testing commented Aug 4, 2022

Uh oh!

tedli commented Aug 5, 2022

Uh oh!

istio-testing commented Aug 5, 2022

Uh oh!

zirain commented Aug 5, 2022

Uh oh!

tedli commented Aug 5, 2022

Uh oh!

Uh oh!

Uh oh!

craigbox commented Aug 12, 2022

Uh oh!

Uh oh!

ericvn commented Aug 31, 2022

Uh oh!

craigbox commented Sep 12, 2022

Uh oh!

tedli commented Sep 12, 2022

Uh oh!

ericvn commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

craigbox Oct 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tedli commented Aug 4, 2022 •

edited

Loading

ericvn commented Sep 12, 2022 •

edited

Loading