Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

kata-deploy: add support for deploying Kata on K8S #65

Merged
merged 2 commits into from Jul 12, 2018

Conversation

egernst
Copy link
Member

@egernst egernst commented Jun 13, 2018

A Dockerfile is created and reference daemonsets are also
provided for deploying Kata Containers onto a running Kubernetes
cluster. A few daemonsets are introduced:

  1. runtime-labeler: This daemonset will create a label on each node in
    the cluster identifying the CRI shim in use. For example,
    container-runtime=crio or container-runtime=containerd.

  2. crio and containerd kata installer: Assuming either CRIO or
    containerd is the CRI runtime on the node (determined based on label
    from (1),, either the crio or containerd variant will execute. These daemonsets
    will install the VM artifacts and host binaries required for using
    Kata Containers. Once installed, it will add a node label kata-runtime=true
    and reconfigure either crio or containerd to make use of Kata for untrusted workloads.
    As a final step it will restart the CRI shim and kubelet. Upon deletion,
    the daemonset will remove the kata binaries and VM artifacts and update
    the label to kata-runtime=cleanup.

  3. crio and containerd cleanup: Either of these two daemonsets will run,
    pending the container-runtime label value and if the node has label
    kata-runtime=cleanup. This daemonset simply restarts crio/containerd as
    well as kubelet. This was not feasible in a preStepHook, hence the
    seperate cleanup step.

An RBAC is created to allow the daemonsets to modify labels on the node.

To deploy kata:
kubectl apply -f kata-rbac.yaml
kubectl apply -f kata-deploy.yaml

To remove kata:
kubectl delete -f kata-deploy.yaml
kubectl apply -f kata-cleanup.yaml
kubectl delete -f kata-cleanup.yaml
kubectl delete -f kata-rbac.yaml

This initial commit is based on contributions by a few folks on
github.com/egernst/kata-deploy

Also-by: Saikrishna Edupuganti saikrishna.edupuganti@intel.com
Signed-off-by: Eric Ernst eric.ernst@intel.com
Signed-off-by: Jon Olson jonolson@google.com
Signed-off-by: Ricardo Aravena raravena@branch.io

@egernst egernst requested a review from jodh-intel June 13, 2018 05:07
@egernst
Copy link
Member Author

egernst commented Jun 13, 2018

/cc @klynnrif @jcvenegas

cp /etc/crio/crio.conf /etc/crio/crio.conf.bak

echo "Set Kata containers as default runtime in CRI-O for untrusted workloads"
sed -i 's/runtime_untrusted_workload = ""/runtime_untrusted_workload = "\/opt\/kata\/bin\/kata-runtime"/' /etc/crio/crio.conf

Choose a reason for hiding this comment

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

remove space in the beginning, for sure change the untrusted setting to kata and no need for escape characters -

sed -i '/runtime_untrusted_workload = /c\runtime_untrusted_workload = "/opt/kata/bin/kata-runtime"' /etc/crio/crio.conf

Copy link
Member Author

Choose a reason for hiding this comment

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

@jodh-intel, @chavafg - I did this based on our CI scripts -- can you comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @krsna1729 as the code is easier to read without requireing backslashes. However, I think the command would need to be something like the following with uses ! as the delimiter for sed:

sed -i 's!runtime_untrusted_workload = ""!runtime_untrusted_workload = "/opt/kata/bin/kata-runtime"!' /etc/crio/crio.conf

Choose a reason for hiding this comment

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

So if there is a runtime_untrusted_workload already registered, @jodh-intel's version nor the original one would change that. The node would still be labelled kata-runtime=true (this part is in the yaml)? Should we pull that in here or exit with error from here so that the said label will not be applied.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, that's a good question. Since they are explicitly trying to install Kata, I think the best thing may be to just overwrite what was there before. WDYT @krsna1729

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll take your input, @krsna1729, so I just do a line replace instead of matching in ""

cat << EOT | tee /etc/containerd/config.toml
[plugins]
[plugins.cri.containerd]
snapshotter = "overlayfs"

Choose a reason for hiding this comment

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

what does this do? this would change the behavior for runc too, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jcvenegas - I pulled this from your reference in our CI/guides. Was this an explicit addition, or can you clarify?

Choose a reason for hiding this comment

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

Can this be wrapped in a environment variable check? Then those deploying the DaemonSet can skip this behaviour if they customize the config themselves.

Copy link
Member

Choose a reason for hiding this comment

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

you can remove it and use the default

nodeSelector:
container-runtime: cri-o
kata-runtime: cleanup
containers:

Choose a reason for hiding this comment

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

need volume mount for systemd and dbus just like kata-deploy.yaml below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, agreed.

kind: DaemonSet
metadata:
name: kubelet-containerd-kata-cleanup
namespace: kube-system

Choose a reason for hiding this comment

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

can we combine this into the above daemonset?

      nodeSelector:
          kata-runtime: cleanup

and

systemctl daemon-reload; systemctl restart crio;  systemctl restart containerd; systemctl restart kubelet;

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed -done

Copy link

@klynnrif klynnrif left a comment

Choose a reason for hiding this comment

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

Scrubbed for grammar, flow, and structure. Apologies ahead if I've changed the meaning of anything. Thanks!

README.md Outdated
Kata Containers currently supports packages for many distributions. Tooling to aid in creating these
packages are contained within this repository.

In addition, Kata build artifacts are also available within a container image, created by a

Choose a reason for hiding this comment

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

(Removing "also"): In addition, Kata build artifacts are available within a container image, created by a

README.md Outdated
packages are contained within this repository.

In addition, Kata build artifacts are also available within a container image, created by a
[Dockerfile](kata-deploy/Dockerfile). Reference daemonsets are provided in [kata-deploy](kata-deploy)

Choose a reason for hiding this comment

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

A couple notes for lines 7-8: Add a comma after (kata-deploy). Also, If the subject here is kata-deploy, then you need to change "make" to "makes" - if the subject is "Reference daemonsets", leave "make" as is (I am not sure which is the subject) :)

@@ -0,0 +1,115 @@
# kata-deploy

[kata-deploy](kata-deploy) provides a Dockerfile which contains all of the binaries

Choose a reason for hiding this comment

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

Lines 3-4 suggested rewrite: kata-deploy provides a Dockerfile, which contains all of the binaries and artifacts required to run Kata Containers. The Dockerfile also references daemonsets, which can be utilized to install Kata Containers...

[kata-deploy](kata-deploy) provides a Dockerfile which contains all of the binaries
and artifacts required to run Kata Containers, as well as reference daemonsets which can be utilized to install Kata Containers on a running Kubernetes cluster.

Note, installation via daemonsets will only succesfully install `kata-runtime` on

Choose a reason for hiding this comment

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

Lines 6-7 suggested rewrite: Note, installation through daemonsets only successfully installs kata-runtime on a node if it uses either containerd or CRI-O CRI-shims.


## Quick start:

### Installing Kata on a running Kubernetes cluster

Choose a reason for hiding this comment

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

Consider removing the "ing" gerund formatting on any title (helps SEO and keeps things consistent): Install Kata on a running Kubernetes cluster


Virtual Machine artifacts:
* kata-containers.img: pulled from Kata github releases page
* vmliuz.container: pulled from Kata github releases page

Choose a reason for hiding this comment

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

github => GitHub

### Daemonsets and RBAC:

A few daemonsets are introduced for kata-deploy, as well as an RBAC to facilitate
being able to apply labels to the nodes.

Choose a reason for hiding this comment

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

applying labels to the nodes


#### runtime-labeler:

This daemonset will create a label on each node in

Choose a reason for hiding this comment

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

This daemonset creates a label on each node in


#### CRI-O and containerd kata installer:

Depending the value of `container-runtime` label on the node, either the CRI-O or

Choose a reason for hiding this comment

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

Lines 105-106 suggested rewrite (to stay active):

Depending on the value of container-runtime label on the node, either the CRI-O or containerd kata installation daemonset executes. These daemonsets install the necessary kata binaries, configuration files, and virtual machine artifacts on the node. Once installed, the daemonset adds a node label kata-runtime=true and reconfigures either CRI-O or containerd to make use of Kata for untrusted workloads. As a final step the daemonset restarts either CRI-O or containerd and kubelet. Upon deletion, the daemonset removes the kata binaries and VM artifacts and updates the node label to kata-runtime=cleanup.

to `kata-runtime=cleanup.`

### CRI-O and containerd cleanup:
Depending on the value of `container-runtime`, either the CRI-O or conatinerd Kata cleanup daemonset will run if the node has label `kata-runtime=cleanup.` This daemonsets will remove the `container-runtime` and `kata-runtime` labels as well

Choose a reason for hiding this comment

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

Lines 113-115 suggested rewrite (to stay active):

Depending on the value of container-runtime, either the CRI-O or conatinerd Kata cleanup daemonset runs if the node has the label kata-runtime=cleanup. This daemonset removes the container-runtime and kata-runtime labels as well as restarts either CRI-O or containerd systemctl daemon and kubelet. You cannot execute these resets during the preStopHook of the Kata installer daemonset, which necessitated this final cleanup daemonset.

imagePullPolicy: Always
command: [ "sh", "-c" ]
args:
- kubectl label node $NODE_NAME container-runtime- kata-runtime-;

Choose a reason for hiding this comment

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

should we use katacontainers.io/<label-name> ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah... probably a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

### Installing Kata on a running Kubernetes cluster

```
kubectl apply -f kata-rbac.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Please can you add the standard $ prefix to all the shell lines.
  • Does this need to run as root? If so, please prefix with sudo -E.

Choose a reason for hiding this comment

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

@jodh-intel this can be run without root. root or not depends on how/where the user has setup their kubeconfig. The user should run it whichever way they run any other kubectl command against their cluster.

* kata-runtime: pulled from Kata github releases page
* kata-proxy: pulled from Kata github releases page
* kata-shim: pulled from Kata github releases page
* qemu-system-x86_64: statically built and included in this repo, based on Kata's QEMU repo
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this makes this PR blocked on #67.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh - I've just seen that you've added the static binary (and the bios files, etc) to this PR. I don't think we (need to do / should be doing) that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Once we have static binaries available, yes, I'd pull them from the release. I can plan on removing this in a follow on PR once #67 lands.

Kata Containers.

Host artifacts:
* kata-runtime: pulled from Kata github releases page
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to links to the repos here.

* qemu/* : supporting binaries required for qemu-system-x86_64

Virtual Machine artifacts:
* kata-containers.img: pulled from Kata github releases page
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the initrd image?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just a sample setup. My configuration.toml for this does't make use of the initrd, so didn't include it. I similarly don't include other versions of QEMU or other rootfs variants. Not sure if I should/need to include here.

@@ -0,0 +1,115 @@
# kata-deploy
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a TOC please?

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

kata-runtime: cleanup
containers:
- name: kube-kata-cleanup
image: egernst/kata-deploy
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this to the kata-containers project to avoid hard-coding your username?

Copy link
Member Author

Choose a reason for hiding this comment

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

once it is released and available, yes. For now, we don't have a Dockerfile in place. Once this merges, I'll do a follow on PR immediately to point to the created one after configuring it on dockerhub.

Copy link
Member Author

Choose a reason for hiding this comment

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

will do once we create that first container image (follow on pr)

- kubectl label node $NODE_NAME container-runtime- kata-runtime-;
systemctl daemon-reload && systemctl restart crio && systemctl restart kubelet;
while true;
do sleep 36000; done;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Que?
    • Why do we need this?
    • Why 10 hours?

If we really need it please add a comment. Also, could you format it in a better way? Maybe:

while true; do sleep 36000; done;

Same comment for the other instances.

Choose a reason for hiding this comment

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

@jodh-intel DaemonSet pods should not exit. The controller will try to restart them in case they exit. That line was to workaround this behavior . Maybe sleep infinity ?

Copy link
Contributor

Choose a reason for hiding this comment

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

tail -f /dev/null sleeps forever ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

tailing /dev/null...

Copy link
Member Author

Choose a reason for hiding this comment

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

ACK - will use tail -f!

@@ -0,0 +1,184 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

This file appears to have a lot of boilerplate code. I'd much prefer we built the embedded cri-o and cri-containerd documents from a common source file (kata-deploy.yaml.in or similar) otherwise it's going to be very hard to maintain this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree the deploymant/daemonset yamls in general have a lof of boiler plate content, but don't want to over-engineer this for maintenance until I feel the pain.

I want to make sure it is easy for developers to be able to pull the yaml from github (raw) directly. I don't think this'll change much over time - the deployment should stay the same, while the contents of the Dockerfile will move per release. If we see that there's thrash here over time, though, I'm more than happy to go down this road.

cp /etc/crio/crio.conf /etc/crio/crio.conf.bak

echo "Set Kata containers as default runtime in CRI-O for untrusted workloads"
sed -i 's/runtime_untrusted_workload = ""/runtime_untrusted_workload = "\/opt\/kata\/bin\/kata-runtime"/' /etc/crio/crio.conf
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @krsna1729 as the code is easier to read without requireing backslashes. However, I think the command would need to be something like the following with uses ! as the delimiter for sed:

sed -i 's!runtime_untrusted_workload = ""!runtime_untrusted_workload = "/opt/kata/bin/kata-runtime"!' /etc/crio/crio.conf


### Dockerfile

A Dockerfile is created which contains all of the necessary artifacts for running
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I'm lost - you run the k8s yaml and it creates a Dockerfile?

Can you explain what you do with the Dockerfile.

@egernst egernst force-pushed the introduce-kata-deploy branch 2 times, most recently from fd0d418 to 0d51255 Compare June 17, 2018 18:02
@egernst
Copy link
Member Author

egernst commented Jun 17, 2018

@jodh-intel - for a follow on PR, I'll address a couple of things:

  1. use a kata-containers based dockerhub image in the .yamls (once the dockerfile is posted)
  2. pull an official static qemu once it is available and configured for sane deployment paths (I like /opt/kata for this)
  3. pull a released reference configuration.toml, and then sed it to the paths that I expect for kata-deploy.

WDYT?

@nitkon
Copy link
Contributor

nitkon commented Jun 17, 2018

@egernst : Can we have options to do packaging for multiple architectures like arm64 and ppc64le? A bit of documentation/overview on how currently packaging works for kata for x86_64 would have been helpful.

the necessary kata binaries, configuration files and virtual machine artifacts on
the node. Once installed, the daemonset adds a node label `kata-containers.io/kata-runtime=true` and reconfigures
either CRI-O or containerd to make use of Kata for untrusted workloads. As a final step the daemonset
restarts either CRI-O or containerd and kubelet. Upon deletion, the daemonset removes the kata binaries

Choose a reason for hiding this comment

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

Remove this line as its part of kata cleanup?
As a final step the daemonset restarts either CRI-O or containerd and kubelet.

Copy link
Member Author

@egernst egernst Jun 18, 2018

Choose a reason for hiding this comment

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

Well, we still restart kubelet and the cri shim after installing the binaries.

@egernst
Copy link
Member Author

egernst commented Jun 18, 2018

@nitkon I agree in general, we need to add packaging support for the additional architectures (not sure if its specific problem for this PR, though). I sent an email out re: starting a package/release "team" in kata -- it probably makes sense to add someone for ppc on this. WDYT?

@jodh-intel
Copy link
Contributor

@egernst - if we can't get the qemu script to build a static qemu) landed quickly, I'd still much prefer this PR does not store huge binary blobs - they'll stay in the history for ever. Could you maybe make them downloads on one of the repos releases page instead and just get this PR to curl them?

Copy link

@kfox1111 kfox1111 left a comment

Choose a reason for hiding this comment

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

Looking pretty good.

ARG KATA_VER=1.0.0
ARG KATA_URL=https://github.com/kata-containers/runtime/releases/download/${KATA_VER}

RUN yum install -y wget

Choose a reason for hiding this comment

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

curl should be in the image already.

Copy link
Member

Choose a reason for hiding this comment

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

+1 for curl

@@ -0,0 +1,21 @@
FROM centos/systemd

Choose a reason for hiding this comment

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

centos:centos7 is more commonly used. would be smaller I think.

Copy link

@krsna1729 krsna1729 Jun 18, 2018

Choose a reason for hiding this comment

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

we need systemd to restart services

centos                        centos7             49f7960eb7e4        13 days ago         200MB
centos/systemd                latest              5477808193ce        2 months ago        199MB

Choose a reason for hiding this comment

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

oh. right. thanks.

RUN wget -q ${KATA_URL}/{vmlinuz.container,kata-containers.img}

WORKDIR /tmp/kata/bin/
RUN wget -q ${KATA_URL}/{kata-runtime,kata-proxy,kata-shim}

Choose a reason for hiding this comment

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

Can combine the layers into one like:
ARG KUBECTL_VER=v1.10.2

RUN
curl -s -o kata-runtime ${KATA_URL}/kata-runtime &&
curl -s -o kata-proxy ${KATA_URL}/kata-proxy &&
curl -s -o kata-shim ${KATA_URL}/kata-shim &&
curl -s -o /bin/kubectl https://storage.googleapis.com/kubernetes-release/release/${KUBECTL_VER}/bin/linux/amd64/kubectl &&
chmod +x /bin/kubectl

RUN wget -qO /bin/kubectl https://storage.googleapis.com/kubernetes-release/release/${KUBECTL_VER}/bin/linux/amd64/kubectl && \
chmod +x /bin/kubectl

COPY bin /tmp/kata/bin

Choose a reason for hiding this comment

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

maybe somewhere in /opt instead? I think some things automount things over /tmp?

### Daemonsets and RBAC:

A few daemonsets are introduced for kata-deploy, as well as an RBAC to facilitate
appyling labels to the nodes.

Choose a reason for hiding this comment

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

Is this optional? I'm a little worried about extra daemonsets with the power to label nodes. The kubelet permissions are restricted on a per node basis so a node can only edit its own record, but I don't think that applies to other service accounts?

Choose a reason for hiding this comment

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

alternately, could you just use kubelets local credential? Then it would already exist and be restricted in exactly the needed way.

Choose a reason for hiding this comment

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

@kfox1111 can you please provide suggested reading material and any examples that use it this way? RBAC noob :)

Choose a reason for hiding this comment

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

maybe just pass /etc/kubernetes/kubelet.conf into the container and use that as the kubeconf? I don't think you would need rbac at all then.

Choose a reason for hiding this comment

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

seems like that file refers to system:node and that's a whole lot more permissions?

kubectl describe clusterrole system:node
Name:         system:node
Labels:       kubernetes.io/bootstrapping=rbac-defaults
Annotations:  rbac.authorization.kubernetes.io/autoupdate=true
PolicyRule:
  Resources                                       Non-Resource URLs  Resource Names  Verbs
  ---------                                       -----------------  --------------  -----
  configmaps                                      []                 []              [get]
  endpoints                                       []                 []              [get]
  events                                          []                 []              [create patch update]
  nodes                                           []                 []              [create get list watch delete patch update]
  nodes/status                                    []                 []              [patch update]
  persistentvolumeclaims                          []                 []              [get]
  persistentvolumes                               []                 []              [get]
  pods                                            []                 []              [get list watch create delete]
  pods/eviction                                   []                 []              [create]
  pods/status                                     []                 []              [update]
  secrets                                         []                 []              [get]
  services                                        []                 []              [get list watch]
  tokenreviews.authentication.k8s.io              []                 []              [create]
  localsubjectaccessreviews.authorization.k8s.io  []                 []              [create]
  subjectaccessreviews.authorization.k8s.io       []                 []              [create]
  certificatesigningrequests.certificates.k8s.io  []                 []              [create get list watch]
  volumeattachments.storage.k8s.io                []                 []              [get]

vs

kubectl describe clusterrole "node-labeler"
Name:         node-labeler
Labels:       <none>
Annotations:  kubectl.kubernetes.io/last-applied-configuration={"apiVersion":"rbac.authorization.k8s.io/v1","kind":"ClusterRole","metadata":{"annotations":{},"name":"node-labeler","namespace":""},"rules":[{"apiGrou...
PolicyRule:
  Resources  Non-Resource URLs  Resource Names  Verbs
  ---------  -----------------  --------------  -----
  nodes      []                 []              [get patch]

Choose a reason for hiding this comment

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

yes, but no. Think of it in terms of what happens when a node gets compromised.

Adversary breaks out of container. Adversary has access to all secrets available on the single node they are on. From there, what can they do?

The system:node role has a special admin controller on the api side that restricts its behaviour to only the node it belongs to. so, for example, you can't evict pods on node B from node A.

But, with the role you created, an advisory can label any node. This could, for example, be used to label some more privileged nodes with labels that would attract the vulnerable workload the adversary used to get onto the machine in the first place. This would allow them to spread to all other machines, rather then just compromising some edge machines.

Ideally, the service account spec would allow node like restrictions on service accounts like it can do for node accounts. I don't think there is any plans currently to tackle that though.

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

BTW, this is something we want to solve generically in Kubernetes (see kubernetes/kubernetes#62747, DSP -> apiserver authz). I have a draft proposal for node-scoped roles, but it needs to be cleaned up before publishing.

Choose a reason for hiding this comment

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

cool. thanks for the info.

kubectl get node $NODE_NAME --show-labels;
kubectl label node $NODE_NAME kata-containers.io/container-runtime=$(kubectl describe node $NODE_NAME | awk -F'[:]' '/Container Runtime Version/ {print $2}' | tr -d ' ');
kubectl get node $NODE_NAME --show-labels;
tail -f /dev/null;

Choose a reason for hiding this comment

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

sleep infinity

command: ["sh", "-c", "/tmp/kata/scripts/remove-kata-crio.sh && kubectl label node $NODE_NAME --overwrite kata-containers.io/kata-runtime=cleanup"]
command: [ "sh", "-c" ]
args:
- /tmp/kata/scripts/install-kata-crio.sh && kubectl label node $NODE_NAME kata-containers.io/kata-runtime=true;

Choose a reason for hiding this comment

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

very small nit. it can be made more readable, and handle more errors like:
command: [ "sh", "-ce"]
args: |
/tmp/kata/scripts/install-kata-crio.sh
kubectl label node $NODE_NAME kata-containers.io/kata-runtime=true
kubectl get node $NODE_NAME --show-labels
sleep infinity

Copy link
Member Author

Choose a reason for hiding this comment

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

finally coming back around to this -- @kfox1111 in this case I don't want to label the node unless the installation succeeded.

Ack on the -ce instead of -c

@@ -0,0 +1,23 @@
#!/bin/sh

Choose a reason for hiding this comment

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

Should the binary files above be excluded? Usually you don'