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.

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

add a remote cluster check step to "verify the installation" multicluster setup docs #15923

Merged
merged 5 commits into from
Nov 13, 2024

Conversation

rsalmond
Copy link
Contributor

Description

Add a step to the verify the installation multicluster setup docs for executing istioctl remote clusters, and some notes about interpreting the results.

Fixes #15911

Reviewers

  • Ambient
  • Docs
  • Installation
  • Networking
  • Performance and Scalability
  • Extensions and Telemetry
  • Security
  • Test and Release
  • User Experience
  • Developer Infrastructure
  • Localization/Translation

@rsalmond rsalmond requested a review from a team as a code owner November 11, 2024 18:34
@istio-policy-bot
Copy link

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

  • You can learn about how we write and maintain documentation, our style guidelines,
    and 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 with
    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, sanitize the Markdown, ensure all hyperlinks point to a
    valid location, and more. If your PR doesn't pass one of these checks, you'll see a red X in the
    lint_istio.io entry in the status section. Click on the Details link to get a list of the
    problems with your PR. Fix those problems and push an update; this will automatically re-run the
    tests. 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).
    To publish them sooner, add a cherrypick/release-x.xx label, where x.xx is the current
    release of Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 11, 2024
@istio-testing istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 11, 2024
cluster1 synced istiod-a5jg5df5bd-2dfa9
cluster2 istio-system/istio-remote-secret synced istiod-a5jg5df5bd-2dfa9
ENDSNIP

Copy link
Member

Choose a reason for hiding this comment

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

This is a generated file as per the comments on previous lines. You can skip these changes.

####################################################################################################
# WARNING: THIS IS AN AUTO-GENERATED FILE, DO NOT EDIT. PLEASE MODIFY THE ORIGINAL MARKDOWN FILE:
#          docs/setup/install/multicluster/verify/index.md
####################################################################################################

@@ -20,6 +20,16 @@
# docs/setup/install/multicluster/verify/index.md
####################################################################################################

snip_verify_multicluster_1() {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add these verification steps to the test ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I definitely underestimated how complex the docs tests are. I think I added it to the right place, but I don't see anything obvious in the multicluster test output that suggests this test was executed.

@kfaseela
Copy link
Member

/ok-to-test

@istio-testing istio-testing added the ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. label Nov 12, 2024
@rsalmond
Copy link
Contributor Author

@kfaseela @m4ver1k thanks for the feedback but I feel like I'm getting mixed messages. Should I, or should I not, make some sort of change to snips.sh? Note that at present the changes to that file are the result of running make gen.

@kfaseela
Copy link
Member

@kfaseela @m4ver1k thanks for the feedback but I feel like I'm getting mixed messages. Should I, or should I not, make some sort of change to snips.sh? Note that at present the changes to that file are the result of running make gen.

Changes are fine. I understand it is auto generated. Those snips we should be using in the test.sh if it makes sense.

@m4ver1k
Copy link
Member

m4ver1k commented Nov 12, 2024

@kfaseela @m4ver1k thanks for the feedback but I feel like I'm getting mixed messages. Should I, or should I not, make some sort of change to snips.sh? Note that at present the changes to that file are the result of running make gen.

Apologies I mistook them to be edited and not auto-generated. Please ignore my previous comment.

@@ -101,6 +101,9 @@ function cleanup_cluster2
# between CLUSTER1 and CLUSTER2.
function verify_load_balancing
{
# Verify istiod is synced
snip_verify_multicluster_1
Copy link
Member

Choose a reason for hiding this comment

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

simply calling verify_multicluster wont be enough.. you will have to compare the output of the command to the expected output which is there in snip_verify_multicluster_1_out . Take a look at the usages of _verify_like() in other similar tests

@istio-testing istio-testing merged commit 93c28e5 into istio:master Nov 13, 2024
13 checks passed
wilsonwu added a commit to wilsonwu/istio.io that referenced this pull request Nov 14, 2024
…llation" multicluster setup docs into Chinese
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/docs ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

call out istioctl create-remote-secret interaction with rancher proxy
5 participants