-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Drop or adapt tutorials that rely on Katacoda #40306
Conversation
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of nits. But overall, LGTM! Thank you so much for you efforts, @sftim!
content/en/docs/tutorials/kubernetes-basics/create-cluster/cluster-intro.html
Outdated
Show resolved
Hide resolved
<h3>View the app</h3> | ||
<p>Pods that are running inside Kubernetes are running on a private, isolated network. | ||
By default they are visible from other pods and services within the same kubernetes cluster, but not outside that network. | ||
When we use <code>kubectl</code>, we're interacting through an API endpoint to communicate with our application.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we'd benefit from telling the audience here what we're interacting with, as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to adapt the existing text as little as possible. Let's do that kind of tweak in a follow up PR.
content/en/docs/tutorials/kubernetes-basics/explore/explore-intro.html
Outdated
Show resolved
Hide resolved
<div class="row"> | ||
<div class="col-md-12"> | ||
<h3>Show the app in the terminal</h3> | ||
<p>Recall that Pods are running in an isolated, private network - so we need to proxy access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit.
<p>Recall that Pods are running in an isolated, private network - so we need to proxy access | |
<p>Recall that Pods are running in an isolated, private network - so we need proxy access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“need to” feels idiomatic to me
I'm trying to adapt the existing text as little as possible. Let's do that kind of tweak in a follow up PR.
content/en/docs/tutorials/kubernetes-basics/explore/explore-intro.html
Outdated
Show resolved
Hide resolved
<p>Again, it's worth mentioning that the name of the container itself can be omitted since we only have a single container in the Pod.</p> | ||
<p>Next let’s start a bash session in the Pod’s container:</p> | ||
<p><code><b>kubectl exec -ti $POD_NAME -- bash</b></code></p> | ||
<p>We have now an open console on the container where we run our NodeJS application. The source code of the app is in the <tt>server.js</tt> file:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p>We have now an open console on the container where we run our NodeJS application. The source code of the app is in the <tt>server.js</tt> file:</p> | |
<p>We have now an open console on the container where we run our NodeJS application. The source code of the app is in the <tt>server.js</tt> file:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to adapt the existing text as little as possible. Let's do that kind of tweak in a follow up PR.
<p>Next, let’s list the current Services from our cluster:</p> | ||
<p><code><b>kubectl get services</b></code></p> | ||
<p>We have a Service called <tt>kubernetes</tt> that is created by default when minikube starts the cluster. | ||
To create a new service and expose it to external traffic we'll use the expose command with NodePort as parameter.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To create a new service and expose it to external traffic we'll use the expose command with NodePort as parameter.</p> | |
To create a new service and expose it to external traffic we'll use the expose command with the NodePort parameter.</p> |
OR
To create a new service and expose it to external traffic we'll use the expose command with NodePort as parameter.</p> | |
To create a new service and expose it to external traffic we'll use the expose command with NodePort as a parameter.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither is right.
In another PR, we could change it to:
To create a new service and expose it to external traffic we'll use the expose command, specifying
NodePort as the type of Service to create.
We could also reword to avoid using we.
content/en/docs/tutorials/kubernetes-basics/expose/expose-intro.html
Outdated
Show resolved
Hide resolved
@@ -25,9 +27,10 @@ <h3>Objectives</h3> | |||
<div class="col-md-8"> | |||
<h3>Scaling an application</h3> | |||
|
|||
<p>In the previous modules we created a <a href="/docs/concepts/workloads/controllers/deployment/"> Deployment</a>, and then exposed it publicly via a <a href="/docs/concepts/services-networking/service/">Service</a>. The Deployment created only one Pod for running our application. When traffic increases, we will need to scale the application to keep up with user demand.</p> | |||
<p>Previously we created a <a href="/docs/concepts/workloads/controllers/deployment/"> Deployment</a>, and then exposed it publicly via a <a href="/docs/concepts/services-networking/service/">Service</a>. The Deployment created only one Pod for running our application. When traffic increases, we will need to scale the application to keep up with user demand.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit.
<p>Previously we created a <a href="/docs/concepts/workloads/controllers/deployment/"> Deployment</a>, and then exposed it publicly via a <a href="/docs/concepts/services-networking/service/">Service</a>. The Deployment created only one Pod for running our application. When traffic increases, we will need to scale the application to keep up with user demand.</p> | |
<p>Previously we created a <a href="/docs/concepts/workloads/controllers/deployment/"> Deployment</a>, and then exposed it publicly via a <a href="/docs/concepts/services-networking/service/">Service</a>. The Deployment created only one Pod for running our application. When traffic increases, we will need to scale the application to keep up with user demand.</p> |
efdc0cd
to
f662efe
Compare
I'm keen to get this approved and LGTMed (and held), so we are ready for the end of the month. |
{{< /tab >}} | ||
{{< /tabs >}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the change from {{% /tab %}}
to {{ /tab }}
.
@@ -287,7 +287,7 @@ kubectl delete service hello-node | |||
kubectl delete deployment hello-node | |||
``` | |||
|
|||
Optionally, stop the Minikube virtual machine (VM): | |||
Stop the Minikube cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stop the Minikube cluster | |
Stop the Minikube cluster: |
LGTM |
No more Katacoda. Explain that the tutorial isn't available.
No more Katacoda.
Avoid relying on Katacoda. Co-authored-by: Divya Mohan <divya.mohan0209@gmail.com>
d1930f1
to
252bcf6
Compare
/lgtm |
LGTM label has been added. Git tree hash: 0af4fed05a2a4e2b53e987c8c7e5f8ad6c2d25ef
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: divya-mohan0209 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/label tide/merge-method-squash |
No need to squash these commits: each represents a distinct unit of work /remove-label tide/merge-method-squash |
I reckon it's about time to ship this though. /hold cancel |
<a class="btn btn-lg btn-success" href="/docs/tutorials/kubernetes-basics/deploy-app/deploy-interactive/" role="button">Start Interactive Tutorial <span class="btn__next">›</span></a> | ||
<h3>Deploy an app</h3> | ||
<p>Let’s deploy our first app on Kubernetes with the <code>kubectl create deployment</code> command. We need to provide the deployment name and app image location (include the full repository url for images hosted outside Docker hub).</p> | ||
<p><b><code>kubectl create deployment kubernetes-bootcamp --image=gcr.io/google-samples/kubernetes-bootcamp:v1</code></b></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could these code blocks be worth formatting with a <pre>
block, so they render similarly to the fenced code blocks that currently exist in Markdown pages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use a separate issue for that discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end, we adapted most of these tutorials for people to run them locally. |
Prompted by #33936
Follows #40292
Remove pages where we don't have a replacement online, interactive tutorial mechanism, changing them to an apology message
(aside: we don't have an easy way to serve 410 Gone responses)
Update pages where we have an way to let people run the tutorial locally.
/hold
We should not make this change until the end of March 2023.