Add crictl tools in image #21

neo-liang-sap · 2020-04-01T05:56:44Z

What this PR does / why we need it:

Add crictl tools in ops-toolbelt image
Which issue(s) this PR fixes:
Fixes #19
Special notes for your reviewer:

Release note:

add crictl tools in ops-toolbelt image, detailed info could be found via https://github.com/kubernetes-sigs/cri-tools

danielfoehrKn · 2020-04-06T11:09:32Z

dockerfile-configs/common-components.yaml

+  - name:  crictl
+    command: |
+      VERSION="v1.17.0";\
+      curl -fsSLO https://github.com/kubernetes-sigs/cri-tools/releases/download/$VERSION/crictl-$VERSION-linux-amd64.tar.gz;\


The problem with this installation approach is that crictl is only needed in Gardener to communicate with containerd CRI. However the crictl default configuration points to docker. Crictl in Gardener should point per default to containerd. If containerd is not installed crictl will not work, but that is fine as the user can just use the regular docker commands then.
Please create the config file for crictl having the configuration for containerd.
Thanks!

Hi @danielfoehrKn , thanks for the comments, i've added one config pointing to unix:///run/containerd/containerd.sock and copy the config file to crictl default config location in /etc/crictl.yaml

danielfoehrKn · 2020-04-06T11:58:16Z

dotfiles/crictl.yaml

+runtime-endpoint: unix:///run/containerd/containerd.sock
+image-endpoint: unix:///run/containerd/containerd.sock
+timeout: 10
+debug: true


Do you know what this configuration parameter is used for?

i got this sample from https://github.com/kubernetes-sigs/cri-tools/blob/master/docs/crictl.md - i haven't used crictl before so do let me know if i need to remove some of the configs...

Apparently the flag --debug enables debug output for crictl itself. So lets not use include that per default.

thanks, i've removed debug line from the config

danielfoehrKn · 2020-04-06T11:59:56Z

dockerfile-configs/common-components.yaml

@@ -131,7 +131,9 @@
      VERSION="v1.17.0";\
      curl -fsSLO https://github.com/kubernetes-sigs/cri-tools/releases/download/$VERSION/crictl-$VERSION-linux-amd64.tar.gz;\
      tar zxvf crictl-$VERSION-linux-amd64.tar.gz -C /usr/local/bin;\
-      rm -f crictl-$VERSION-linux-amd64.tar.gz
+      rm -f crictl-$VERSION-linux-amd64.tar.gz;\
+      rm -rf /etc/crictl.yaml;\


Maybe we should not delete an existing configuration on the node. What do you think about only adding it when it is not available yet?

i checked this file status before cp the new one - the installation process of crictl doesn't create config if you don't run it, after you first time run crictl it will create an /etc/crictl.yaml which contains one line of null in it.
the default config will point to unix:///var/run/dockershim.sock which is not desired containerd

what do you think?

What I mean is that, whenever an operator launches another ops/toolbelt pod, this will override the current /etc/crictl.yaml file on the host.
But thinking about it, that might be even better.

Follow up on this: there are cases when the node image already got crictl installed- so overriding an already existing config should be avoided. PN me for more details.

danielfoehrKn

Looks good to me. Thanks

neo-liang-sap · 2020-04-06T13:33:13Z

thanks, i've squash into one commit, can someone from code owner have a review on this PR? thanks ;)

petersutter · 2020-04-09T11:42:42Z

@danielfoehrKn @neo-liang-sap have you tried it out if it works?

danielfoehrKn

Another thing that came to my mind: installing crictl when the pod is a non-root pod does not make sense. Can that somehow be turned off when running in non-root?

@petersutter dashboard deployed pods are always running as non-root correct? So crictl would not make sense for the dashboard?

neo-liang-sap · 2020-04-09T12:44:03Z

Hi @petersutter @danielfoehrKn ,
I added some change to installation logic - check if running as non-root and whether crictl already installed.

I tested locally using .ci/build to build docker file and docker build an local image to run (docker run -it xxxx) and verified.

I also tried run hacks/ops-pod -i local_image_name node_name but seems the ops-pod can't pull image from my local but needs to pull image from GCR which i don't have access

Thanks and do let me know if there's other scenario i didn't covered/tested

petersutter · 2020-04-09T13:22:06Z

The pipeline always pushes the image to gcr, here it is for your latest commit eu.gcr.io/gardener-project/gardener/ops-toolbelt:0.10.0-dev-8a2137cd0491a01992e554e814f2484709516d02

neo-liang-sap · 2020-04-09T13:37:44Z

Hi @petersutter, thanks, i've tested on one shoot node and it's working, also verified the config file in /etc/crictl.yaml

BTW i never got access to ci/publish and other jobs (e.g.)https://concourse.ci.gardener.cloud/builds/29092 , after i tried login with github username/password i got 404 not found - whom should i refer to apply for (maybe) i'm lacking permission to access the CI?

thanks

petersutter · 2020-04-10T11:15:05Z

@petersutter dashboard deployed pods are always running as non-root correct? So crictl would not make sense for the dashboard?

@danielfoehrKn the ops-toolbelt is run by default as non-root, however in the settings you can enable the Privileged mode. So criticl should work.

Do you have containerd somewhere running to test this image agains it?

danielfoehrKn · 2020-04-10T20:37:57Z

@petersutter thats actually a super cool feature that I did not know about (maybe it could be somehow more prominent with a checkbox or something - just an idea). Yes I got a containerd installation on my local setup. Will try it out soon and update here.

petersutter · 2020-04-27T19:16:41Z

@danielfoehrKn did you have the time to try it out?

petersutter · 2020-04-27T19:19:43Z

Another thing that came to my mind: installing crictl when the pod is a non-root pod does not make sense. Can that somehow be turned off when running in non-root?

@neo-liang-sap have you checked if this is possible?

neo-liang-sap · 2021-06-29T02:54:03Z

PR are modified based on review comments

neo-liang-sap · 2021-06-29T10:27:58Z

conflicts resolved

plkokanov · 2021-06-29T12:23:03Z

Meanwhile, I personally would not bother with crictl anymore but use nerdctl for container instead.

@danielfoehrKn what are the benefits? Probably we can close this PR and open a new one for nerdctl.

danielfoehrKn · 2021-06-30T07:50:41Z

nerdctl is a docker-compatible CLI for containerd. And containerd is the only CRI compatible container runtime Gardener supports at the moment.
It is much easier to deal with compared to crictl - which focuses on low-level CRI interactions. It is mostly a debug tool for CRI.

neo-liang-sap · 2021-06-30T10:03:02Z

so what's the decision? i'm totally fine with closing this PR and work on a new ticket for nerdctl
@plkokanov @danielfoehrKn

plkokanov · 2021-06-30T12:39:10Z

@jfortin-sap, let's close this PR and open a new one for nerdctl. Tbh the chang shouldn't be much different than the current one. Again, just add in the info field some help text about how one can connect to the container runtime by using .sock files from the host's root directory.

neo-liang-sap · 2021-06-30T12:50:08Z

@plkokanov i guess you were to mentioning me but not Jonathan ;) , sure i will follow up

plkokanov · 2021-06-30T12:51:17Z

Yeah, oops @neo-liang-sap, sorry.

neo-liang-sap requested a review from a team as a code owner April 1, 2020 05:56

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 1, 2020

gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 1, 2020

neo-liang-sap mentioned this pull request Apr 6, 2020

Add crictl #19

Closed

danielfoehrKn suggested changes Apr 6, 2020

View reviewed changes

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 6, 2020

danielfoehrKn reviewed Apr 6, 2020

View reviewed changes

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 6, 2020

danielfoehrKn approved these changes Apr 6, 2020

View reviewed changes

Add crictl tools in image

fd57209

neo-liang-sap force-pushed the issue-19-add-crictl branch from 94a935b to fd57209 Compare April 6, 2020 13:31

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 6, 2020

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 6, 2020

danielfoehrKn suggested changes Apr 9, 2020

View reviewed changes

Add logic when running as non-root and crictl already installed

8a2137c

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 9, 2020

Merge branch 'master' into issue-19-add-crictl

63d85fb

gardener-robot removed the needs/rebase Needs git rebase label Jun 29, 2021

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 29, 2021

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 29, 2021

fix conflict

81823ac

gardener-robot added size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) and removed size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Jun 29, 2021

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jun 29, 2021

fix build error

6cc7f6c

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 29, 2021

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 29, 2021

remove change unrelated with this crictl pr

9675c11

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 29, 2021

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 29, 2021

gardener-robot added the lifecycle/rotten Nobody worked on this for 12 months (final aging stage) label Jun 29, 2021

Merge branch 'master' into issue-19-add-crictl

8f76ab1

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 29, 2021

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 29, 2021

neo-liang-sap closed this Jun 30, 2021

neo-liang-sap mentioned this pull request Jun 30, 2021

Add nerdctl (instead of crictl) #56

Closed

petersutter mentioned this pull request Jul 19, 2022

Added crictl to work with containerd to the toolbelt #81

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add crictl tools in image #21

Add crictl tools in image #21

neo-liang-sap commented Apr 1, 2020 •

edited

Loading

danielfoehrKn Apr 6, 2020

neo-liang-sap Apr 6, 2020

danielfoehrKn Apr 6, 2020

neo-liang-sap Apr 6, 2020

danielfoehrKn Apr 6, 2020

neo-liang-sap Apr 6, 2020

danielfoehrKn Apr 6, 2020

neo-liang-sap Apr 6, 2020

danielfoehrKn Apr 6, 2020

danielfoehrKn Apr 9, 2020

danielfoehrKn left a comment

neo-liang-sap commented Apr 6, 2020

petersutter commented Apr 9, 2020

danielfoehrKn left a comment •

edited

Loading

neo-liang-sap commented Apr 9, 2020 •

edited

Loading

petersutter commented Apr 9, 2020

neo-liang-sap commented Apr 9, 2020 •

edited

Loading

petersutter commented Apr 10, 2020

danielfoehrKn commented Apr 10, 2020 •

edited

Loading

petersutter commented Apr 27, 2020

petersutter commented Apr 27, 2020 •

edited

Loading

neo-liang-sap commented Jun 29, 2021

neo-liang-sap commented Jun 29, 2021

plkokanov commented Jun 29, 2021

danielfoehrKn commented Jun 30, 2021

neo-liang-sap commented Jun 30, 2021

plkokanov commented Jun 30, 2021

neo-liang-sap commented Jun 30, 2021

plkokanov commented Jun 30, 2021

Add crictl tools in image #21

Add crictl tools in image #21

Conversation

neo-liang-sap commented Apr 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielfoehrKn left a comment

Choose a reason for hiding this comment

neo-liang-sap commented Apr 6, 2020

petersutter commented Apr 9, 2020

danielfoehrKn left a comment • edited Loading

Choose a reason for hiding this comment

neo-liang-sap commented Apr 9, 2020 • edited Loading

petersutter commented Apr 9, 2020

neo-liang-sap commented Apr 9, 2020 • edited Loading

petersutter commented Apr 10, 2020

danielfoehrKn commented Apr 10, 2020 • edited Loading

petersutter commented Apr 27, 2020

petersutter commented Apr 27, 2020 • edited Loading

neo-liang-sap commented Jun 29, 2021

neo-liang-sap commented Jun 29, 2021

plkokanov commented Jun 29, 2021

danielfoehrKn commented Jun 30, 2021

neo-liang-sap commented Jun 30, 2021

plkokanov commented Jun 30, 2021

neo-liang-sap commented Jun 30, 2021

plkokanov commented Jun 30, 2021

neo-liang-sap commented Apr 1, 2020 •

edited

Loading

danielfoehrKn left a comment •

edited

Loading

neo-liang-sap commented Apr 9, 2020 •

edited

Loading

neo-liang-sap commented Apr 9, 2020 •

edited

Loading

danielfoehrKn commented Apr 10, 2020 •

edited

Loading

petersutter commented Apr 27, 2020 •

edited

Loading