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

Update consul-k8s CLI docs with Envoy Debugging #13860

Merged
merged 24 commits into from
Aug 9, 2022

Conversation

t-eckert
Copy link
Contributor

@t-eckert t-eckert commented Jul 22, 2022

Description

With the upcoming release of Consul on Kubernetes 0.47.0, the consul-k8s CLI will gain two new commands:

  • proxy list for listing the names of all Pods running Envoy proxies managed by Consul.
  • proxy read <PODNAME> for inspecting the Envoy configuration of a given Pod.

This PR updates the commands page for the consul-k8s CLI with documentation for the new commands.

In addition, this PR makes some changes to existing documentation with the intent of improving clarity:

  • Change of wording of the intro section.
  • Addition of descriptive text to the table of contents.
  • Removal of the "Required" column from flag tables. All flags are optional.

Links

PR Checklist

* [ ] updated test coverage (No code added)

  • external facing docs updated
  • not a security concern

@github-actions github-actions bot added the type/docs Documentation needs to be created/updated/clarified label Jul 22, 2022
@t-eckert t-eckert added the pr/no-changelog PR does not need a corresponding .changelog entry label Jul 22, 2022
@t-eckert t-eckert marked this pull request as ready for review August 8, 2022 19:34
@t-eckert t-eckert requested a review from a team as a code owner August 8, 2022 19:35
@t-eckert t-eckert requested a review from david-yu August 8, 2022 19:35
@t-eckert t-eckert requested review from im2nguyen and removed request for david-yu August 8, 2022 20:17
Copy link
Contributor

@im2nguyen im2nguyen left a comment

Choose a reason for hiding this comment

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

overall, this looks great! i really like the example commands you've added in these docs

a couple of nits, but pre-approving so we can merge along with the k8s release!

website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
Thomas Eckert and others added 6 commits August 8, 2022 16:55
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
Thomas Eckert and others added 2 commits August 8, 2022 17:08
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
Thomas Eckert and others added 3 commits August 8, 2022 17:25
Co-authored-by: Tu Nguyen <im2nguyen@users.noreply.github.com>
@t-eckert
Copy link
Contributor Author

t-eckert commented Aug 8, 2022

@im2nguyen, thanks! I figure I should wait to merge this until Thursday?

@im2nguyen
Copy link
Contributor

Yep! @boruszak is coordinating the other PRs too, do you want him to merge it too?

Copy link
Contributor

@trujillo-adam trujillo-adam left a comment

Choose a reason for hiding this comment

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

Looks really good. I added a few grammar and style nits, but otherwise very solid.

website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
website/content/docs/k8s/k8s-cli.mdx Outdated Show resolved Hide resolved
Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>
Copy link
Contributor

@trujillo-adam trujillo-adam left a comment

Choose a reason for hiding this comment

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

👍

@t-eckert
Copy link
Contributor Author

t-eckert commented Aug 9, 2022

Yep! @boruszak is coordinating the other PRs too, do you want him to merge it too?

Yes please!

@david-yu david-yu self-assigned this Aug 9, 2022
Copy link
Contributor

@david-yu david-yu left a comment

Choose a reason for hiding this comment

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

Great work Thomas! Could you also remove the beta notice for the upgrade command as part of this PR?

@boruszak boruszak merged commit 833fa9b into main Aug 9, 2022
@boruszak boruszak deleted the consul-1.13/update-consul-k8s-cli-docs branch August 9, 2022 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-changelog PR does not need a corresponding .changelog entry type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants