Skip to content

shared control plane multicluster fixes #22173

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
Merged
merged 6 commits into from
Mar 17, 2020
Merged

shared control plane multicluster fixes #22173

merged 6 commits into from
Mar 17, 2020

Conversation

ayj
Copy link
Contributor

@ayj ayj commented Mar 13, 2020 •
edited
Loading

  • Rename the remote istiod service and endpoint to istiod-remote to
    avoid conflicts with the real local istiod service.

  • Use the istiod-remote.<namespace>.svc hostname for the sidecar and
    ingress proxies discoveryAddress. This address needs to match the
    SAN in istiod's cert. The istiod-remote headless service will
    resolve the hostname to the remote IP address.

  • Add the istiod-remote hostname to istiod's SANs. Also use istiod's
    namespace to construct the legacy service names instead of
    hardcoding them to istio-system.

  • Simplify the remote profile by removing redundant and unused values.

Manually backport #21912 from master.

@ayj ayj requested a review from a team March 13, 2020 21:47
@googlebot
Copy link
Collaborator

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. label Mar 13, 2020
@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR needs to be rebased before being merged size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 13, 2020
@ayj
Copy link
Contributor Author

ayj commented Mar 13, 2020

cc @linsun

@ayj ayj force-pushed the ws1-release-1.5-backport-multicluster-fixes branch from 07e13ca to 18771db Compare March 13, 2020 22:55
@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels Mar 13, 2020
@istio-testing istio-testing 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 Mar 13, 2020
@ayj
shared control plane multicluster fixes
036fc76 
* Rename the remote istiod service and endpoint to `istiod-remote` to
  avoid conflicts with the real local istiod service.

* Use the `istiod-remote.<namespace>.svc` hostname for the sidecar and
  ingress proxies discoveryAddress. This address needs to match the
  SAN in istiod's cert. The `istiod-remote` headless service will
  resolve the hostname to the remote IP address.

* Add the `istiod-remote` hostname to istiod's SANs. Also use istiod's
  namespace to construct the legacy service names instead of
  hardcoding them to `istio-system`.

* Simplify the remote profile by removing redundant and unused values.

* clone LbEndpoint to prevent data race (istio#22023)

* fix meshexpansion ports for non-istiod deployments
@ayj ayj force-pushed the ws1-release-1.5-backport-multicluster-fixes branch from 18771db to 036fc76 Compare March 13, 2020 23:26
@ayj
Copy link
Contributor Author

ayj commented Mar 15, 2020

/retest

@ayj ayj added the do-not-merge/hold Block automatic merging of a PR. label Mar 15, 2020
@ayj ayj removed the do-not-merge/hold Block automatic merging of a PR. label Mar 15, 2020
@ayj
Copy link
Contributor Author

ayj commented Mar 15, 2020

/retest

linsun
linsun reviewed Mar 16, 2020
View reviewed changes
istioctl/pkg/multicluster/remote_secret.go

# Create a secret access a remote cluster with an auth plugin
istioctl --Kubeconfig=c0.yaml x create-remote-secret --name c0 --auth-type=plugin --auth-plugin-name=gcp \
| kubectl -n istio-system --Kubeconfig=c1.yaml apply -f -
| kubectl --Kubeconfig=c1.yaml apply -f -
Copy link
Member

@linsun linsun Mar 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@ayj what would be the cmd if users don't install istio to istio-system ns? use -n {namespace}?

Copy link
Member

Choose a reason for hiding this comment

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

Also, where can I find docs for this auth-plugin, auth-type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, user's can use -n <namespace> if they install in a different namespace.

There aren't any additional docs yet for auth->{plugin,type}.

linsun
linsun reviewed Mar 16, 2020
View reviewed changes
@ayj
Copy link
Contributor Author

ayj commented Mar 16, 2020

This should be ready for review. This is mostly a backport of #21912. Charts between 1.5 and master have diverged so its worth taking a second look at the legacy helm charts in particular.

linsun
linsun approved these changes Mar 17, 2020
View reviewed changes
Copy link
Member

@linsun linsun left a comment

Choose a reason for hiding this comment

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

@fpesce @dgn pls review/approve

@istio-testing istio-testing merged commit 3218efd into istio:release-1.5 Mar 17, 2020
@ayj ayj deleted the ws1-release-1.5-backport-multicluster-fixes branch March 17, 2020 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linsun linsun linsun approved these changes

+1 more reviewer

@fpesce fpesce fpesce approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ayj @googlebot @linsun @fpesce @istio-testing