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.

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

Explicitly document how to template manifests with both helm and istioctl and remove references to istioctl manifest diff #16057

Merged
merged 13 commits into from
Dec 12, 2024
Merged

Explicitly document how to template manifests with both helm and istioctl and remove references to istioctl manifest diff #16057

merged 13 commits into from
Dec 12, 2024

Conversation

bleggett
Copy link
Contributor

@bleggett bleggett commented Dec 5, 2024 •
edited
Loading

Description

#16056 is fixed in 1.25 but indicates the need for

  • More details in the istioctl manifest generate section about --cluster-specific
  • remove manifest diff stuff, as that is gone in 1.24+
  • Add a section in the Helm install guide about how to generate manifests with that tool as well.

Reviewers

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

@bleggett bleggett requested a review from a team as a code owner December 5, 2024 17:58
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 5, 2024
@bleggett bleggett added cherrypick/release-1.24 Set this label on a PR to auto-merge it to the release-1.24 branch and removed area/environments kind/docs size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 5, 2024
@bleggett bleggett force-pushed the bleggett/fixup-manifest-template-stuff branch from ba7293d to 63f1b27 Compare December 5, 2024 18:03
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 5, 2024
@bleggett bleggett force-pushed the bleggett/fixup-manifest-template-stuff branch from 63f1b27 to 388f6e6 Compare December 5, 2024 18:05
@bleggett
Explicitly document how to template manifests with both helm and `i…
886eec8 
…stioctl` and remove references to `istioctl manifest diff`

Signed-off-by: Benjamin Leggett <[email protected]>
@bleggett bleggett force-pushed the bleggett/fixup-manifest-template-stuff branch from 388f6e6 to 886eec8 Compare December 5, 2024 18:08
@bleggett
Lintfix
d74f00d 
Signed-off-by: Benjamin Leggett <[email protected]>
@bleggett bleggett mentioned this pull request Dec 5, 2024
Closed
2 tasks
craigbox
craigbox reviewed Dec 6, 2024
View reviewed changes
content/en/about/faq/setup/install-method-selection.md Show resolved Hide resolved
content/en/about/faq/setup/install-method-selection.md Outdated Show resolved Hide resolved
content/en/about/faq/setup/install-method-selection.md Outdated Show resolved Hide resolved
content/en/about/faq/setup/install-method-selection.md Outdated Show resolved Hide resolved
content/en/docs/setup/install/helm/index.md Outdated Show resolved Hide resolved
content/en/docs/setup/install/helm/index.md Outdated Show resolved Hide resolved
content/en/docs/setup/install/helm/index.md Outdated Show resolved Hide resolved
bleggett and others added 8 commits December 6, 2024 11:21
@bleggett @craigbox
Update content/en/about/faq/setup/install-method-selection.md
7f5d473 
Co-authored-by: Craig Box <[email protected]>
@bleggett @craigbox
Update content/en/about/faq/setup/install-method-selection.md
8978fda 
Co-authored-by: Craig Box <[email protected]>
@bleggett @craigbox
Update content/en/docs/setup/install/helm/index.md
21329fe 
Co-authored-by: Craig Box <[email protected]>
@bleggett @craigbox
Update content/en/docs/setup/install/helm/index.md
0b80e70 
Co-authored-by: Craig Box <[email protected]>
@bleggett @craigbox
Update content/en/docs/setup/install/helm/index.md
8b6f14e 
Co-authored-by: Craig Box <[email protected]>
@bleggett @craigbox
Update content/en/docs/setup/install/istioctl/index.md
d012e15 
Co-authored-by: Craig Box <[email protected]>
@bleggett
Addl fixes
17fe298 
Signed-off-by: Benjamin Leggett <[email protected]>
@bleggett
Formatting fixes
bd178d0 
Signed-off-by: Benjamin Leggett <[email protected]>
@craigbox
Copy link
Contributor

craigbox commented Dec 9, 2024

(I dunno why GitHub doesn't send me email for these sorry, I have to chance back upon them later)

@craigbox craigbox added the do-not-merge/hold Block automatic merging of a PR. label Dec 12, 2024
craigbox
craigbox approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@craigbox craigbox left a comment

Choose a reason for hiding this comment

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

Approving with hold, feel free to make the final suggested changes or ignore and merge.

@bleggett
Copy link
Contributor Author

bleggett commented Dec 12, 2024 •
edited
Loading

@craigbox suggested changes applied ty

  • I also duplicated the istioctl/helm templating sections to the respective ambient install guides as well, since that seems correct. If you object to this LMK and I'll back it out.

@craigbox
Copy link
Contributor

I don't see it in this PR?

It feels like, given the FAQ lists three methods, this could maybe be a third page ("Install with templated manifest") and it could maybe be shared between ambient and sidecar? Not sure. Maybe we just have to have a bunch of boilerplates we share between them.

Anyway, lets get this in and iterate. Un-hold at will.

@bleggett
Review comments
adacca1 
Signed-off-by: Benjamin Leggett <[email protected]>
@bleggett
Gen
b079ca5 
Signed-off-by: Benjamin Leggett <[email protected]>
@bleggett bleggett removed the do-not-merge/hold Block automatic merging of a PR. label Dec 12, 2024
@istio-testing istio-testing merged commit fe9599e into istio:master Dec 12, 2024
6 checks passed
@istio-testing
Copy link
Contributor

In response to a cherrypick label: new pull request created: #16086

@istio-testing istio-testing mentioned this pull request Dec 12, 2024
Merged
wilsonwu added a commit to wilsonwu/istio.io that referenced this pull request Dec 13, 2024
@wilsonwu
Sync istio#16057 faq install and ambient install docs into Chinese
fb9821a
@wilsonwu wilsonwu mentioned this pull request Dec 13, 2024
Merged
11 tasks
wilsonwu added a commit to wilsonwu/istio.io that referenced this pull request Dec 13, 2024
@wilsonwu
Sync istio#16057 helm and istioctl setup docs into Chinese
27d84b8
@wilsonwu wilsonwu mentioned this pull request Dec 13, 2024
Merged
11 tasks
istio-testing pushed a commit that referenced this pull request Dec 16, 2024
@wilsonwu
Sync #16057 faq install and ambient install docs into Chinese (#16089)
992546f
istio-testing pushed a commit that referenced this pull request Dec 16, 2024
@wilsonwu
Sync #16057 helm and istioctl setup docs into Chinese (#16090)
ceccef9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@craigbox craigbox craigbox approved these changes

Assignees
No one assigned
Labels
cherrypick/release-1.24 Set this label on a PR to auto-merge it to the release-1.24 branch 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.
4 participants
@bleggett @craigbox @istio-testing @istio-policy-bot