-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Install guide for ambient multi network #16709
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
Install guide for ambient multi network #16709
Conversation
Skipping CI for Draft Pull Request. |
😊 Welcome! This is either your first contribution to the Istio documentation repo, or
Thanks for contributing! Courtesy of your friendly welcome wagon. |
content/en/docs/ambient/install/multicluster/multi-primary_multi-network/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/ambient/install/multicluster/multi-primary_multi-network/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/ambient/install/multicluster/multi-primary_multi-network/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/ambient/install/multicluster/multi-primary_multi-network/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/ambient/install/multicluster/multi-primary_multi-network/index.md
Outdated
Show resolved
Hide resolved
|
||
Currently, ambient multicluster **only supports**: | ||
- **Multi-network topologies** with multiple primary clusters | ||
- **Double HBONE encapsulation** for cross-cluster traffic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I'm a bit late to the party.
- Multi-network topologies with multiple primary clusters
More accurately, each cluster has to be a primary cluster (have its own istiod). If you have 10 clusters but only two primary cluster, then you would have "multiply primary clusters" but this topology would not be supported.
- Double HBONE encapsulation for cross-cluster traffic
Can we configure the this at all? is there any way for someone to try any other inner/outer tunnel protocol configuration. If not, then I think we can omit this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More accurately, each cluster has to be a primary cluster (have its own istiod). If you have 10 clusters but only two primary cluster, then you would have "multiply primary clusters" but this topology would not be supported.
Totally agree - good clarification
Can we configure the this at all? is there any way for someone to try any other inner/outer tunnel protocol configuration. If not, then I think we can omit this.
Even if we don't support other configurations, it might be good to explicitly clarify we currently don't support protocols other than double hbone. I could be convinced either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a stab at rewording; let me know your thoughts
Currently, ambient multicluster **only supports**: | ||
- **Multi-network topologies** with multiple primary clusters | ||
- **Double HBONE encapsulation** for cross-cluster traffic | ||
- **Universal waypoint deployments** across all clusters with identical names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
waypoint deployment or configuration? You don't have to deploy waypoints in all clusters, but maybe we do want to require this to avoid confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a service is using a waypoint does it have to have one locally available (as well as globally)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the services also have the same waypoints across clusters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we talk about it later
- Waypoint configurations must be synchronized manually across clusters | ||
- Traffic routing relies on consistent waypoint naming conventions | ||
|
||
#### Service Visibility and Scoping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The locality of services with waypoints is also confusing and worth mentioning. Like if service is local, but its waypoint service is labeled as global, then it is global.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this makes sense when I think of the implementation, but it's probably something to make an adjustment to in beta (/cc @therealmitchconnors @krinkinmu)
|
||
## Deploy the `HelloWorld` Service | ||
|
||
In order to make the `HelloWorld` service callable from any cluster, the DNS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TECHNICALLY, kubernetes also allows you to do service discovery though environment variables (crazy). But I think that mentioning DNS distracts from the point a bit. I think its more like, we want to create a helloworld service that we're eventually going to use to csend cross-cluster traffic. To start, we need to create the service in each cluster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the reference to the DNS doc though because it's important consideration for anyone trying to do multicluster (I remember stumbling through this when I first gave it a try). The k8s env var service disovery approach only works within single-cluster on the namespace, so I think this belongs
content/en/docs/ambient/install/multicluster/multi-primary_multi-network/index.md
Outdated
Show resolved
Hide resolved
214cd73
to
c16c515
Compare
Before you begin a multicluster installation, review the | ||
[deployment models guide](/docs/ops/deployment/deployment-models) | ||
which describes the foundational concepts used throughout this guide. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will sending people to the sidecar version of the doc confuse them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, the link isn't a sidecar specific doc; in theory, ambient can support all of the deployment models listed there.
scripts/lint_site.sh
Outdated
@@ -95,7 +95,7 @@ check_content() { | |||
FAILED=1 | |||
fi | |||
|
|||
if grep -nrP --include "*.md" -e "\(https://istio.io/(?!v[0-9]\.[0-9]/|archive/)" .; then | |||
if grep -nrP --include "*.md" -e "\(https://istio.io/(?!v[0-9]\.[0-9]/|archive|news/)" .; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this trying to do?
We shouldn't have any content at https://istio.io/news/ any more, which I think is what is being added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do link to news in one of these new docs to tell people to check there for updates on the feature status of ambient multi cluster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be /latest/news, then, instead of /news.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't /news redirect to /latest/news? I can update the regex and reference though if that's preferred
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please do yes; everything without a /latest/ is a leftover from before we put versioning in the site, rather than having archives on archive.istio.io. Which I still don't understand why we did, but I assume David had a really good reason :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, updated!
content/en/docs/ambient/install/multicluster/before-you-begin/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/ambient/install/multicluster/multi-primary_multi-network/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/ambient/install/multicluster/multi-primary_multi-network/index.md
Outdated
Show resolved
Hide resolved
|
||
Before proceeding, be sure to complete the steps under | ||
[before you begin](/docs/setup/install/multicluster/before-you-begin) as well as | ||
choosing and following one of the multicluster installation guides. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some links missing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One was incorrect and one was missing. Should be fixed, but I'll try and look through the Netlify preview
Signed-off-by: Jackie Elliott <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
34932f4
to
e6b080d
Compare
Signed-off-by: Keith Mattix II <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
/test doc.test.profile-default |
Signed-off-by: Keith Mattix II <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
Signed-off-by: Keith Mattix II <[email protected]>
In response to a cherrypick label: new pull request created: #16776 |
Description
@keithmattix and Mitch Conners are the current owners of this PR
PR is now ready for review
Reviewers