kata: introduce kata container support #3465

egernst · 2018-07-11T23:31:33Z

Signed-off-by: Eric Ernst [email protected]

What this PR does / why we need it:

PR adds support for Kata Containers.

Which issue this PR fixes :

fixes # 3463

Special notes for your reviewer:

Should look very familiar to initial Clear Container support PRs.

If applicable:

documentation
unit tests

Release note:

msftclas · 2018-07-11T23:31:46Z

All CLA requirements met.

egernst · 2018-07-11T23:31:50Z

testing still wip

codecov · 2018-07-11T23:43:46Z

Codecov Report

Merging #3465 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3465      +/-   ##
==========================================
- Coverage   55.83%   55.83%   -0.01%     
==========================================
  Files         105      105              
  Lines       15885    15884       -1     
==========================================
- Hits         8870     8869       -1     
  Misses       6270     6270              
  Partials      745      745

egernst · 2018-07-12T00:19:38Z

To test this PR, I built my branch of acs-engine locally and then tried the basic kata containers test I introduced. Unfortunately it failed with an error that isn't real clear -- perhaps just a timeout? If anyone has pointers, I'd appreciate it.

Details:

eernst-mac02:acs-engine eernst$ ./bin/acs-engine deploy --subscription-id xxxxx --dns-prefix kata-test --location westus2 --api-model kubernetes-kata-containers.json

WARN[0003] apimodel: missing masterProfile.dnsPrefix will use "kata-test"
WARN[0003] --resource-group was not specified. Using the DNS prefix from the apimodel as the resource group name: kata-test
WARN[0006] apimodel: ServicePrincipalProfile was missing or empty, creating application...
WARN[0008] created application with applicationID (ef542ac5-28c0-45c0-bbf4-f8eefec6a701) and servicePrincipalObjectID (2d64c93b-de51-4801-ba8b-9ab2bc3e5c55).
WARN[0008] apimodel: ServicePrincipalProfile was empty, assigning role to application...
INFO[0045] Starting ARM Deployment (kata-test-1196995142). This will take some time...
INFO[0259] Finished ARM Deployment (kata-test-1196995142). Error: resources.DeploymentsClient#CreateOrUpdate: Failure sending request: StatusCode=200 -- Original Error: Long running operation terminated with status 'Failed': Code="DeploymentFailed" Message="At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/arm-debug for usage details."
ERRO[0259] {"status":"Failed","error":{"code":"DeploymentFailed","message":"At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/arm-debug for usage details.","details":[{"code":"Conflict","message":"{
 \"status\": \"Failed\",
 \"error\": {
   \"code\": \"ResourceDeploymentFailure\",
   \"message\": \"The resource operation completed with terminal provisioning state 'Failed'.\",
   \"details\": [
     {
       \"code\": \"VMExtensionProvisioningError\",
       \"message\": \"VM has reported a failure when processing extension 'cse-agent-0'. Error message: \\\"Enable failed: failed to execute command: command terminated with exit status=2\\n[stdout]\\n\\n[stderr]\\n\\\".\"
     }
   ]
 }
}"},{"code":"Conflict","message":"{
 \"status\": \"Failed\",
 \"error\": {
   \"code\": \"ResourceDeploymentFailure\",
   \"message\": \"The resource operation completed with terminal provisioning state 'Failed'.\",
   \"details\": [
     {
       \"code\": \"VMExtensionProvisioningError\",
       \"message\": \"VM has reported a failure when processing extension 'cse-agent-2'. Error message: \\\"Enable failed: failed to execute command: command terminated with exit status=2\\n[stdout]\\n\\n[stderr]\\n\\\".\"
     }
   ]
 }
}"},{"code":"Conflict","message":"{
 \"status\": \"Failed\",
 \"error\": {
   \"code\": \"ResourceDeploymentFailure\",
   \"message\": \"The resource operation completed with terminal provisioning state 'Failed'.\",
   \"details\": [
     {
       \"code\": \"VMExtensionProvisioningError\",
       \"message\": \"VM has reported a failure when processing extension 'cse-agent-1'. Error message: \\\"Enable failed: failed to execute command: command terminated with exit status=2\\n[stdout]\\n\\n[stderr]\\n\\\".\"
     }
   ]
 }
}"},{"code":"Conflict","message":"{
 \"status\": \"Failed\",
 \"error\": {
   \"code\": \"ResourceDeploymentFailure\",
   \"message\": \"The resource operation completed with terminal provisioning state 'Failed'.\",
   \"details\": [
     {
       \"code\": \"VMExtensionProvisioningError\",
       \"message\": \"VM has reported a failure when processing extension 'cse-master-0'. Error message: \\\"Enable failed: failed to execute command: command terminated with exit status=2\\n[stdout]\\n\\n[stderr]\\n\\\".\"
     }
   ]
 }
}"}]}}
FATA[0259] resources.DeploymentsClient#CreateOrUpdate: Failure sending request: StatusCode=200 -- Original Error: Long running operation terminated with status 'Failed': Code="DeploymentFailed" Message="At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/arm-debug for usage details."

egernst · 2018-07-12T00:31:48Z

@scooley, @jessfraz - let me know if this failure is familiar, or if you have tips on where to look for more details. Thx!

adelina-t · 2018-07-12T15:58:55Z

@egernst I ran into the same issue while running windows k8s CI jobs on this PR. Basically the k8s script that provisions the master node failed. You can see logs on the master node in /var/log/azure/cluster-provision.log . You basically forgot a "||" operator at L367 and L373 ( letf a comment inline as well ) :) That's the reason for "VM has reported a failure when processing extension 'cse-master-0'"

adelina-t · 2018-07-12T15:59:47Z

parts/k8s/kubernetescustomscript.sh

@@ -344,13 +364,13 @@ function installContainerd() {
 	sed -i '/\[Service\]/a ExecStartPost=\/sbin\/iptables -P FORWARD ACCEPT' /etc/systemd/system/containerd.service

 	echo "Successfully installed cri-containerd..."
-	if [[ "$CONTAINER_RUNTIME" == "clear-containers" ]] || [[ "$CONTAINER_RUNTIME" == "containerd" ]]; then
+	if [[ "$CONTAINER_RUNTIME" == "clear-containers" ]] [[ "$CONTAINER_RUNTIME" == "kata-containers" ]] || [[ "$CONTAINER_RUNTIME" == "containerd" ]]; then


Forgot "||" operator here :)

adelina-t · 2018-07-12T16:00:16Z

parts/k8s/kubernetescustomscript.sh

 		setupContainerd
 	fi
 }

 function ensureContainerd() {
-	if [[ "$CONTAINER_RUNTIME" == "clear-containers" ]] || [[ "$CONTAINER_RUNTIME" == "containerd" ]]; then
+	if [[ "$CONTAINER_RUNTIME" == "clear-containers" ]] [[ "$CONTAINER_RUNTIME" == "kata-containers" ]] || [[ "$CONTAINER_RUNTIME" == "containerd" ]]; then


and here :) that's why you got cse-master-0 error.

Oh, that's embarrassing -- thanks for the help.

egernst · 2018-07-12T16:00:53Z

@adelina-t - sweet, thanks for teaching me to fish - the /var/log/azure/cluster-provision.log is very helpful; that's what I was missing.

egernst · 2018-07-12T16:36:32Z

@adelina-t -- repushed with these fixed.

I verified I could create a cluster using the added example, kubernetes-kata-containers.json

You mentioned a comment inline? Can you clarify.

adelina-t · 2018-07-12T17:11:23Z

@egernst By "comment inline" I was referring to the comments I left on the commit. Probably not the clearest choice of words on my part :)

CecileRobertMichon · 2018-07-12T17:10:14Z

examples/e2e-tests/kubernetes/kubernetes-config/kata-containers.json

@@ -0,0 +1,40 @@
+{
+  "apiVersion": "vlabs",


I don't think we need the example json in both places, we have a //todo item to move the feature examples in e2e-tests/ so you can probably keep just the other one

CecileRobertMichon · 2018-07-12T17:11:24Z

parts/k8s/kubernetescustomscript.sh

+    echo "Adding Kata Containers repository key..."
+    KATA_RELEASE_KEY_TMP=/tmp/kata-containers-release.key
+    KATA_URL=http://download.opensuse.org/repositories/home:/katacontainers:/release/xUbuntu_16.04/Release.key
+    retrycmd_if_failure_no_stats 20 1 5 curl -fsSL $KATA_URL > $KATA_RELEASE_KEY_TMP || exit $ERR_APT_INSTALL_TIMEOUT


let's add a new exit code specific to Kata install here

You can see installDocker() for a model

CecileRobertMichon · 2018-07-12T17:13:20Z

parts/k8s/kubernetescustomscript.sh

+
+    # Install Kata Containers runtime
+    echo "Installing Kata Containers runtime..."
+    apt_get_update


this should be apt_get_update || exit $ERR_APT_UPDATE_TIMEOUT

CecileRobertMichon · 2018-07-12T17:14:21Z

parts/k8s/kubernetescustomscript.sh

+    # Install Kata Containers runtime
+    echo "Installing Kata Containers runtime..."
+    apt_get_update
+    apt_get_install 20 30 120 kata-runtime


and apt_get_install 20 30 120 kata-runtime || exit $ERR_KATA_INSTALL_TIMEOUT here, ERR_KATA_INSTALL_TIMEOUT needs to be defined at the top of the file

CecileRobertMichon · 2018-07-12T17:16:19Z

parts/k8s/kubernetescustomscript.sh

@@ -281,6 +281,24 @@ function configNetworkPlugin() {
    fi
 }

+function installKataContainersRuntime() {


I realize that my comments in this function also apply to installClearContainersRuntime(), which is probably why you did it that way but I think installClearContainersRuntime needs to be changed as well to have proper exit codes.

No worries. Okay to update the example function, installClearContainersRuntime(), in a follow-on PR?

CecileRobertMichon · 2018-07-12T17:17:04Z

parts/k8s/kubernetescustomscript.sh

+
+if [[ "$CONTAINER_RUNTIME" == "kata-containers" ]]; then
+	# Ensure we can nest virtualization
+	if grep -q vmx /proc/cpuinfo; then


what happens if this is false?

in that case, the given node would not install kata container artifacts

Would the node be functional? Does that mean there would be no container runtime installed?

It'll be fully functional, though the user may hope to use Kata, but in reality be using runc.
When Kata is installed, the operator deploying workloads would have option of either using runc or kata-runtime. In the case VMX isn't supported on the node, any workloads targeting kata-runtime would be handled by the default runtime, runc.

Sounds good, thanks for clarifying

CecileRobertMichon · 2018-07-12T17:17:49Z

pkg/api/vlabs/validate_test.go

@@ -1083,6 +1083,18 @@ func Test_Properties_ValidateContainerRuntime(t *testing.T) {
 		)
 	}

+	p.OrchestratorProfile.KubernetesConfig.ContainerRuntime = "kata-containers"


Is kata-containers supported for all k8s versions?

Kata is more tightly couple with the CRI-shim version (in this case, containerd). I think if there's an error, it'll likely be a mismatch between containerd + k8s?

egernst

Thanks for the quick feedback, @CecileRobertMichon. The updates error codes will definitely make debug much easier - updated per your feedback. PTAL.

egernst · 2018-07-12T18:07:43Z

examples/e2e-tests/kubernetes/kubernetes-config/kata-containers.json

@@ -0,0 +1,40 @@
+{
+  "apiVersion": "vlabs",


egernst · 2018-07-12T18:11:01Z

pkg/api/vlabs/validate_test.go

@@ -1083,6 +1083,18 @@ func Test_Properties_ValidateContainerRuntime(t *testing.T) {
 		)
 	}

+	p.OrchestratorProfile.KubernetesConfig.ContainerRuntime = "kata-containers"


Kata is more tightly couple with the CRI-shim version (in this case, containerd). I think if there's an error, it'll likely be a mismatch between containerd + k8s?

egernst · 2018-07-12T18:13:58Z

parts/k8s/kubernetescustomscript.sh

@@ -281,6 +281,24 @@ function configNetworkPlugin() {
    fi
 }

+function installKataContainersRuntime() {


No worries. Okay to update the example function, installClearContainersRuntime(), in a follow-on PR?

egernst · 2018-07-12T18:17:13Z

parts/k8s/kubernetescustomscript.sh

+
+    # Install Kata Containers runtime
+    echo "Installing Kata Containers runtime..."
+    apt_get_update


egernst · 2018-07-12T18:17:20Z

parts/k8s/kubernetescustomscript.sh

+    # Install Kata Containers runtime
+    echo "Installing Kata Containers runtime..."
+    apt_get_update
+    apt_get_install 20 30 120 kata-runtime


egernst · 2018-07-12T18:17:29Z

parts/k8s/kubernetescustomscript.sh

+    echo "Adding Kata Containers repository key..."
+    KATA_RELEASE_KEY_TMP=/tmp/kata-containers-release.key
+    KATA_URL=http://download.opensuse.org/repositories/home:/katacontainers:/release/xUbuntu_16.04/Release.key
+    retrycmd_if_failure_no_stats 20 1 5 curl -fsSL $KATA_URL > $KATA_RELEASE_KEY_TMP || exit $ERR_APT_INSTALL_TIMEOUT


egernst · 2018-07-12T18:17:35Z

parts/k8s/kubernetescustomscript.sh

+
+if [[ "$CONTAINER_RUNTIME" == "kata-containers" ]]; then
+	# Ensure we can nest virtualization
+	if grep -q vmx /proc/cpuinfo; then


in that case, the given node would not install kata container artifacts

CecileRobertMichon · 2018-07-12T18:28:51Z

examples/kubernetes-kata-containers.json

+      "kubernetesConfig": {
+        "networkPlugin": "flannel",
+        "containerRuntime": "kata-containers",
+        "addons": [


any reason why tiller and dashboard are disabled explicitly? If not, we can probably let them go to default

No, this was just me being lazy and copying what was done for Clear Containers in the past.

Signed-off-by: Eric Ernst <[email protected]>

egernst · 2018-07-12T18:59:41Z

@CecileRobertMichon I verified example after suggested lines. PTAL

egernst · 2018-07-12T22:17:06Z

I'm not sure if there is any action required on my end for kicking this CI (waiting for status to be reported), but please let me know if I can do anything to help push this along.

CecileRobertMichon · 2018-07-12T22:26:44Z

@egernst I'll kick off ci once the PR is approved, will finish reviewing later today if I have time. Thank you for following this through!

CecileRobertMichon

lgtm

Signed-off-by: Eric Ernst <[email protected]>

adelina-t reviewed Jul 12, 2018

View reviewed changes

egernst force-pushed the add-kata branch from 1d1eeb8 to af331fb Compare July 12, 2018 16:34

CecileRobertMichon reviewed Jul 12, 2018

View reviewed changes

egernst force-pushed the add-kata branch from af331fb to ca4568e Compare July 12, 2018 18:23

egernst commented Jul 12, 2018

View reviewed changes

CecileRobertMichon reviewed Jul 12, 2018

View reviewed changes

kata: introduce kata container support

b10f7d6

Signed-off-by: Eric Ernst <[email protected]>

egernst force-pushed the add-kata branch from ca4568e to b10f7d6 Compare July 12, 2018 18:58

CecileRobertMichon approved these changes Jul 13, 2018

View reviewed changes

CecileRobertMichon merged commit 81cf27c into Azure:master Jul 13, 2018

CecileRobertMichon mentioned this pull request Jul 16, 2018

add Kata Containers as a valid runtime #3463

Closed

julienstroheker pushed a commit to julienstroheker/acs-engine that referenced this pull request Jul 16, 2018

Introduce kata container support (Azure#3465)

ce377c3

Signed-off-by: Eric Ernst <[email protected]>

kata: introduce kata container support #3465

kata: introduce kata container support #3465

Conversation

egernst commented Jul 11, 2018 • edited Loading

msftclas commented Jul 11, 2018 • edited Loading

egernst commented Jul 11, 2018

codecov bot commented Jul 11, 2018 • edited Loading

Codecov Report

egernst commented Jul 12, 2018 • edited Loading

egernst commented Jul 12, 2018

adelina-t commented Jul 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egernst commented Jul 12, 2018

egernst commented Jul 12, 2018

adelina-t commented Jul 12, 2018

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

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

egernst Jul 12, 2018 • 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

egernst left a comment • 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

egernst commented Jul 12, 2018

egernst commented Jul 12, 2018

CecileRobertMichon commented Jul 12, 2018

CecileRobertMichon left a comment

Choose a reason for hiding this comment

egernst commented Jul 11, 2018 •

edited

Loading

msftclas commented Jul 11, 2018 •

edited

Loading

codecov bot commented Jul 11, 2018 •

edited

Loading

egernst commented Jul 12, 2018 •

edited

Loading

adelina-t commented Jul 12, 2018 •

edited

Loading

egernst Jul 12, 2018 •

edited

Loading

egernst left a comment •

edited

Loading