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

Replace mermaid diagram, add guided instructions, remove includes wit… #33554

Merged
merged 6 commits into from
Oct 26, 2023

Conversation

lsgunn-teleport
Copy link
Contributor

@lsgunn-teleport lsgunn-teleport commented Oct 16, 2023

…h the get context command

I'm not a Kubernetes expert (or even a novice), so this might have a lot of errors. The main goals here are:

  • Replace the Mermaid diagram with one that uses the new LucidChart library.
  • Add guided enrollment instructions.
  • Remove misleading instruction: Run the following command to get the Kubernetes user for your current context:
    kubectl config view \
    -o jsonpath="{.contexts[?(@.name==\"$(kubectl config current-context)\")].context.user}"
    

See #18495

  • Simplify steps based on a minikube and using the UI example.

Maybe manual enrollment should be a separate topic?

@lsgunn-teleport lsgunn-teleport temporarily deployed to vercel October 16, 2023 23:19 — with GitHub Actions Inactive
@github-actions
Copy link

🤖 Vercel preview here: https://docs-r9bxjt9s0-goteleport.vercel.app/docs/ver/preview

@lsgunn-teleport lsgunn-teleport temporarily deployed to vercel October 17, 2023 17:10 — with GitHub Actions Inactive
@lsgunn-teleport lsgunn-teleport temporarily deployed to vercel October 17, 2023 17:16 — with GitHub Actions Inactive
@github-actions
Copy link

🤖 Vercel preview here: https://docs-2f7dbpmt5-goteleport.vercel.app/docs/ver/preview

@github-actions
Copy link

🤖 Vercel preview here: https://docs-o0996fat5-goteleport.vercel.app/docs/ver/preview

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the purple "Kubernetes" box meant to indicate here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After you click Next, Teleport generates a script to configure and enroll the
Kubernetes cluster as a resource in the Teleport cluster.

1. Copy the command displayed in the Teleport Web UI and run it on the server running
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kubernetes clusters typically consist of several servers. Does this command have to be run on a Kubernetes node or can it run anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, but I'm not really sure what a real-world scenario would be like. I ran through the steps using a single computer minikube installation.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the minikube case, Helm would be making requests to the Kubernetes API server running on minikube, which would be running in a container or VM on your workstation. In a real-life scenario, the Kubernetes API server would be running remotely, e.g., as part of an EKS control plane.

@lsgunn-teleport lsgunn-teleport temporarily deployed to vercel October 17, 2023 18:37 — with GitHub Actions Inactive
@github-actions
Copy link

🤖 Vercel preview here: https://docs-lzvdovih0-goteleport.vercel.app/docs/ver/preview

Copy link
Contributor

Choose a reason for hiding this comment

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

Should the namespace we set here be one the user would understand?

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 was happy to have it work, so I didn't consider editing it. I'm not sure it's hard to understand, but the default is "default" and that would be a "recognizable" namespace identifier but it didn't work for me.

Kubernetes cluster—and information you specify, such as the Kubernetes namespace,
users, and groups to grant access to.

If you can't use the Teleport Web UI or want to run commands and edit file manually,
Copy link
Contributor

Choose a reason for hiding this comment

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

When would a user be unable to use the Web UI? If there aren't any situations where this would be impossible, would it make sense to use only the guided enrollment step in this guide?

Since this is a getting started guide, I think it would make sense to remove Step 1 and "Enroll Kubernetes manually", and have all readers follow Step 2. Then we can use the "Next steps" section to link the reader to the Kubernetes Discovery guides or Registering Kubernetes Clusters with Teleport for their more advanced use cases.

How does that sound?

1. Type all or part of **Kubernetes** in the Search field to filter the resource types
displayed, then click **Kubernetes**.

1. Copy the command to add the `teleport-agent` chart to your charts repository and paste
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@lsgunn-teleport lsgunn-teleport Oct 18, 2023

Choose a reason for hiding this comment

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

From the UI, it says:

Add teleport-agent chart to your charts repository
and

Generate a command to automatically configure and install the teleport-agent namespace

I'm not sure, but I think teleport-kube-agent is an example chart, but I'm not sure it's exactly what gets added here (I'm guessing that this step adds a boilerplate to the charts repo and that your settings in the UI fill in the details, making it "different" from the teleport-kube-agent chart as defined in the reference section.)

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the command shown in the source, helm install teleport-agent teleport/teleport-kube-agent, installs the teleport-kube-agent chart but names the release teleport-agent.

After you click Next, Teleport generates a script to configure and enroll the
Kubernetes cluster as a resource in the Teleport cluster.

1. Copy the command displayed in the Teleport Web UI and run it on a server in
Copy link
Contributor

Choose a reason for hiding this comment

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

How does the script enroll the Kubernetes cluster? If it deploys the Teleport Kubernetes Service via the teleport-kube-agent chart, would you be running the script on your workstation instead of on a Kubernetes node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell based on the original diagram, text, and my test with minikube, the assumption is that you are running this on a server that's part of the cluster. Otherwise, wouldn't it be the same as the standalone cluster with a separate Linux host?
@tigrato Can you clarify the scenario?

@ptgott
Copy link
Contributor

ptgott commented Oct 17, 2023

@lsgunn-teleport Are we backporting this, or is it v15 behavior?

@lsgunn-teleport lsgunn-teleport temporarily deployed to vercel October 18, 2023 21:53 — with GitHub Actions Inactive
@lsgunn-teleport lsgunn-teleport temporarily deployed to vercel October 18, 2023 22:01 — with GitHub Actions Inactive
@github-actions
Copy link

🤖 Vercel preview here: https://docs-3ydjo06w9-goteleport.vercel.app/docs/ver/preview

@github-actions
Copy link

🤖 Vercel preview here: https://docs-k5jaw5hxa-goteleport.vercel.app/docs/ver/preview

@lsgunn-teleport lsgunn-teleport temporarily deployed to vercel October 18, 2023 22:47 — with GitHub Actions Inactive
@github-actions
Copy link

🤖 Vercel preview here: https://docs-poxsrb2c9-goteleport.vercel.app/docs/ver/preview

@lsgunn-teleport
Copy link
Contributor Author

I find the Kubernetes setup options rather confusing:

  • Getting started manual instructions (are they covered by one of the other topics? it doesn't seem like it)
  • Kubernetes Discovery
    • AWS
    • Azure
    • Google Cloud
  • Manual Cluster Registration
    • Register a Cluster with Static kubeconfigs
    • Register a Cluster with IAM Joining
    • Register Clusters Dynamically

Might be worth revisiting the section holistically to clarify what they use cases are and how they differ.

@lsgunn-teleport lsgunn-teleport added the no-changelog Indicates that a PR does not require a changelog entry label Oct 23, 2023
1. Type all or part of **Kubernetes** in the Search field to filter the resource types
displayed, then click **Kubernetes**.

1. Copy the command to add the `teleport-agent` chart repository and paste
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. Copy the command to add the `teleport-agent` chart repository and paste
1. Copy the command to add the Teleport chart repository and paste

teleport-agent is the name of the release

@lsgunn-teleport lsgunn-teleport added this pull request to the merge queue Oct 26, 2023
Merged via the queue into master with commit b1eb339 Oct 26, 2023
@lsgunn-teleport lsgunn-teleport deleted the LG/k8s-getting-started branch October 26, 2023 18:35
@public-teleport-github-review-bot

@lsgunn-teleport See the table below for backport results.

Branch Result
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v14 documentation no-changelog Indicates that a PR does not require a changelog entry size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants