Skip to content

Graduate istioctl analyze out of experimental#19488

Merged
istio-testing merged 5 commits intoistio:masterfrom
sushicw:graduate-analyze
Dec 10, 2019
Merged

Graduate istioctl analyze out of experimental#19488
istio-testing merged 5 commits intoistio:masterfrom
sushicw:graduate-analyze

Conversation

@sushicw
Copy link
Copy Markdown
Contributor

@sushicw sushicw commented Dec 9, 2019

Graduate istioctl analyze out of experimental for 1.5.

This PR proposes doing this as a "soft" graduation, where istioctl analyze becomes canonical but istioctl experimental analyze continues to work, but includes a run-time notice encouraging users to switch. The same message is also present in usage messages. Doing it this way, you'd naturally expect to make it a hard graduation in 1.6, then remove the experimental command in 1.7. I'm interested in feedback on whether this is a desirable approach, or if it just makes the migration ramp unnecessarily long.

Also updated the usage for istioctl validate to more strongly indicate that users should move to istioctl analyze.

This also includes a fix to getDefaultNamespace for istioctl, so that it correctly respects the --context option.

See #16777

Some examples below to demonstrate how the soft graduation looks:

$ istioctl analyze /tmp/default.yaml 
✔ No validation issues found.
$ istioctl x analyze /tmp/default.yaml 
(analyze has graduated. Use `istioctl analyze`)
✔ No validation issues found.
$ istioctl analyze --help | head -n 10
Analyze Istio configuration and print validation messages

Usage:
  istioctl analyze <file>... [flags]

Examples:

# Analyze yaml files
istioctl analyze a.yaml b.yaml

$ istioctl x analyze --help | head -n 10
Analyze Istio configuration and print validation messages (analyze has graduated. Use `istioctl analyze`)

Usage:
  istioctl experimental analyze <file>... [flags]

Examples:

# Analyze yaml files
istioctl analyze a.yaml b.yaml

@sushicw sushicw requested review from a team and esnible December 9, 2019 22:50
@sushicw sushicw requested a review from a team as a code owner December 9, 2019 22:50
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Dec 9, 2019
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 9, 2019
@howardjohn
Copy link
Copy Markdown
Member

This PR proposes doing this as a "soft" graduation, where istioctl analyze becomes canonical but istioctl experimental analyze continues to work, but includes a run-time notice encouraging users to switch. The same message is also present in usage messages. Doing it this way, you'd naturally expect to make it a hard graduation in 1.6, then remove the experimental command in 1.7. I'm interested in feedback on whether this is a desirable approach, or if it just makes the migration ramp unnecessarily long.

I was going to recommend this. Especially for something intended for CI use, arbitrarily changing the command is annoying.

@sushicw
Copy link
Copy Markdown
Contributor Author

sushicw commented Dec 10, 2019

/test pilot-multicluster-e2e_istio

@sushicw sushicw requested a review from ayj December 10, 2019 19:13
@sushicw
Copy link
Copy Markdown
Contributor Author

sushicw commented Dec 10, 2019

@esnible @ayj PTAL

@ayj
Copy link
Copy Markdown
Contributor

ayj commented Dec 10, 2019

I like this approach. One comment on wording, otherwise lgtm.

@sushicw
Copy link
Copy Markdown
Contributor Author

sushicw commented Dec 10, 2019

/test unit-tests_istio

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/user experience cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. 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.

6 participants