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

Removed old trick to create pods with kubectl run #167

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

StevenJDH
Copy link
Contributor

Before k8s 1.18, people would set the restart policy in their imperative commands to make kubectl run generate deployments, pods, or jobs depending on whether the restart policy was set to Always, OnFailure, or Never respectively. For example, kubectl run nginx --image=nginx --restart=Always created a deployment, and kubectl run nginx --image=nginx --restart=Never created a pod. The default behavior of kubectl run when a restart policy wasn't specified was also a deployment. You could even specify --schedule to create cronjobs too. With k8s 1.18 and above, kubectl run can now ONLY create pods, and people now have to use the kubectl create commands to create the other objects, including deployments. The bad thing about this is that in 1.18, kubectl create deployment doesn't support all the flags that kubectl run did, but with 1.19, some of these flags like --port and --replicas have been added to make the command more useful. With all of this in mind, this pull requests removes the --restart=Never trick where kubectl run is used, since we no longer need it for forcing the command to create pods. In other words, how this flag is being used is redundant and only serves to override the default restart policy of a pod, which is Always and not why it was included originally. I feel this change will make things more consistent now and in line with the exam, which is based on k8s 1.19+ now.

References:
https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.18.md#kubectl
https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.19.md#feature-3
https://alexellisuk.medium.com/kubernetes-1-18-broke-kubectl-run-heres-what-to-do-about-it-2a88e5fb389a

@@ -77,9 +77,9 @@ kubectl run nginx --image=nginx --restart=Never --dry-run=client -o yaml | kubec
<p>

```bash
kubectl run busybox --image=busybox --command --restart=Never -it -- env # -it will help in seeing the output
Copy link

@merlixo merlixo Feb 10, 2021

Choose a reason for hiding this comment

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

Default is restart=Always, which implies that the busybox container will be restarted indefinitely...
Pod lifecyle in this case:

start container > execute command > stop container > restart container > ... (loop infinitely...)

==> --restart=Never arg should not be omitted here.

Same comment is valid for several similar changes in this PR.

Copy link
Owner

Choose a reason for hiding this comment

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

@StevenJDH any thoughts about that?

Copy link
Contributor Author

@StevenJDH StevenJDH Feb 18, 2021

Choose a reason for hiding this comment

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

@dgkanatsios, my mistake for those pod with busybox, merlixo makes a good point. I was thinking about something else when I made these changes since I normally use busybox with something like sleep 3600 to keep it running for an hour in labs, or to use it as a temp container using --rm to run some tests.

For the other changes where nginx is used, my original commit message still applies because nginx doesn't run and complete like busybox, creating a loop. In other words, using kubectl run nginx --image=nginx --restart=Never -n mynamespace is an example of the old trick being applied. Using kubectl run nginx --image=nginx -n mynamespace without --restart=Never is fine because the logic here is correct.

I've updated my PR so that the changes are only applied where nginx is used. The busybox entries are now the original entries before the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants