Skip to content

helm: split proxy/auth documentation#19881

Merged
hugoShaka merged 1 commit intomasterfrom
hugo/chart-split-docs
Feb 2, 2023
Merged

helm: split proxy/auth documentation#19881
hugoShaka merged 1 commit intomasterfrom
hugo/chart-split-docs

Conversation

@hugoShaka
Copy link
Copy Markdown
Contributor

This PR contains documentation changes for the PR #18857.

@hugoShaka hugoShaka changed the title Hugo/chart split docs helm: split proxy/auth documentation Jan 4, 2023
@hugoShaka hugoShaka force-pushed the hugo/chart-split-docs branch from 63c2172 to aec38dc Compare January 5, 2023 19:57
@hugoShaka hugoShaka force-pushed the hugo/chart-split-proxy-auth branch from c83b7d1 to 3ec9f8f Compare January 5, 2023 21:44
@hugoShaka hugoShaka force-pushed the hugo/chart-split-docs branch from 17b4a89 to 8c4ddbf Compare January 9, 2023 22:28
@hugoShaka hugoShaka marked this pull request as ready for review January 10, 2023 16:39
@hugoShaka hugoShaka requested review from alexfornuto, marcoandredinis, ptgott, tigrato and webvictim and removed request for ptgott and webvictim January 10, 2023 16:39
@alexfornuto alexfornuto removed their request for review January 10, 2023 18:26
Copy link
Copy Markdown
Contributor

@webvictim webvictim left a comment

Choose a reason for hiding this comment

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

Took a first pass

Comment thread docs/config.json Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments.mdx Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/custom.mdx Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/custom.mdx Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/custom.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Base automatically changed from hugo/chart-split-proxy-auth to master January 13, 2023 14:58
@hugoShaka hugoShaka force-pushed the hugo/chart-split-docs branch from 6b68014 to 823f4b6 Compare January 13, 2023 15:00
Copy link
Copy Markdown
Contributor

@alexfornuto alexfornuto left a comment

Choose a reason for hiding this comment

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

I reviewed purely for copy edits, and did not test anything.

Comment thread docs/pages/deploy-a-cluster/helm-deployments/aws.mdx Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/aws.mdx Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/custom.mdx Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/custom.mdx Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/custom.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread examples/chart/teleport-cluster/values.yaml Outdated
Comment thread examples/chart/teleport-cluster/values.yaml Outdated
Comment thread docs/config.json Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/kubernetes-cluster.mdx Outdated
Comment thread docs/pages/deploy-a-cluster/helm-deployments/kubernetes-cluster.mdx Outdated
$ tsh kube login tele.example.com

# Once working, remove the KUBECONFIG= override to switch to teleport
$ KUBECONFIG=${HOME?}/teleport.yaml kubectl get -n teleport-cluster pods
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

teleport.yaml can be confused with the teleport config file. Isn't it better to rename it as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tsh kube login must also include the KUBECONFIG=${HOME?}/teleport.yaml prefix
tsh login no longer updates the kubeconfig - unless the --kube-cluster flag is defined - so you can drop it from the command above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This section always confused me as I was not aware tsh login previously edited kubeconfig. I rewrote this bit, can you confirm it makes sense?

@hugoShaka hugoShaka requested a review from tigrato January 23, 2023 23:08
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
Comment thread docs/pages/reference/helm-reference/teleport-cluster.mdx Outdated
@r0mant
Copy link
Copy Markdown
Collaborator

r0mant commented Jan 26, 2023

@ptgott @alexfornuto To follow up - is your team going to be able to give this a shot before we merge this?

@ptgott
Copy link
Copy Markdown
Contributor

ptgott commented Jan 26, 2023

@r0mant I tested out the "kubernetes-cluster" guide and proposed edits (while working through the test plan) before I saw this PR (#20779). When are you aiming to merge this PR? Since @alexfornuto has given his approval, I'm happy to defer to him on this.

@alexfornuto
Copy link
Copy Markdown
Contributor

I only ever reviewed this for copy, I did not test it. I don't think I have the prerequisite k8s knowledge to reasonably test this.

@hugoShaka hugoShaka force-pushed the hugo/chart-split-docs branch from 2e5a4a9 to 6577f97 Compare February 2, 2023 15:26
@hugoShaka
Copy link
Copy Markdown
Contributor Author

hugoShaka commented Feb 2, 2023

I'll merge the PR, as this is required for v12. We can do testing and finish the review a posteriori, maybe when rebasing #20779.

If this is relevant to your interests, I can also provide Kubernetes training covering the parts relevant here, so we can co-own this part of the documentation.

@hugoShaka hugoShaka enabled auto-merge February 2, 2023 15:32
@hugoShaka hugoShaka added this pull request to the merge queue Feb 2, 2023
@ptgott
Copy link
Copy Markdown
Contributor

ptgott commented Feb 2, 2023

@hugoShaka Sounds good to me! The Docs Team is going to rework our product area ownership this quarter, so I'll let you know if any of our organizational changes apply to Kubernetes-related docs.

Merged via the queue into master with commit 2b3b9f7 Feb 2, 2023
@public-teleport-github-review-bot
Copy link
Copy Markdown

@hugoShaka See the table below for backport results.

Branch Result
branch/v12 Create PR

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants