Change the type of CRI runtime option by adisky · Pull Request #5300 · containerd/containerd

adisky · 2021-04-01T14:14:43Z

Trying to fix the issue in which runtime options does not get printed in crictl info.
~~Added separate field for runtime options in status response.~~

Update the type of Options *toml.Tree to Options *toml.Tree map[string]interface{}. This is correctly marshalling the options in CRI status response.

Ref Issue: kubernetes-sigs/cri-tools#728

k8s-ci-robot · 2021-04-01T14:14:51Z

Hi @adisky. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

adisky · 2021-04-01T14:20:12Z

cc @dims @mikebrow

theopenlab-ci · 2021-04-01T14:22:38Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 6m 13s (non-voting)

kzys · 2021-04-01T17:24:21Z

@adisky Please rebase your branch against master, instead of merging commits from there.

adisky · 2021-04-01T17:49:17Z

@kzys It is based on the PR #5278 once it gets merged, I'll rebase my PR

dmcgowan · 2021-04-02T04:25:33Z

Its merged now

adisky · 2021-04-02T05:27:45Z

@dmcgowan @kzys Updated!!!

theopenlab-ci · 2021-04-02T05:32:19Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 6m 00s (non-voting)

mikebrow

LGTM hit the wrong button..

mikebrow

let's move this loop down to sometime after the rest.Info make on line 72 below.. otherwise it will panic..

and possibly consider substituting the options in response.Info["config"]

      "runtimes": {
        "runc": {
          "runtimeType": "io.containerd.runc.v2",
          "runtimeEngine": "",
          "PodAnnotations": null,
          "ContainerAnnotations": null,
          "runtimeRoot": "",
          "options": {},
          "privileged_without_host_devices": false,
          "baseRuntimeSpec": ""
        }
      },

after moving the loop down here was my full output:

sudo crictl info
{
  "status": {
    "conditions": [
      {
        "type": "RuntimeReady",
        "status": true,
        "reason": "",
        "message": ""
      },
      {
        "type": "NetworkReady",
        "status": true,
        "reason": "",
        "message": ""
      }
    ]
  },
  "cniconfig": {
    "PluginDirs": [
      "/opt/cni/bin"
    ],
    "PluginConfDir": "/etc/cni/net.d",
    "PluginMaxConfNum": 1,
    "Prefix": "eth",
    "Networks": [
      {
        "Config": {
          "Name": "cni-loopback",
          "CNIVersion": "0.3.1",
          "Plugins": [
            {
              "Network": {
                "type": "loopback",
                "ipam": {},
                "dns": {}
              },
              "Source": "{\"type\":\"loopback\"}"
            }
          ],
          "Source": "{\n\"cniVersion\": \"0.3.1\",\n\"name\": \"cni-loopback\",\n\"plugins\": [{\n  \"type\": \"loopback\"\n}]\n}"
        },
        "IFName": "lo"
      },
      {
        "Config": {
          "Name": "containerd-net",
          "CNIVersion": "0.4.0",
          "Plugins": [
            {
              "Network": {
                "type": "bridge",
                "ipam": {
                  "type": "host-local"
                },
                "dns": {}
              },
              "Source": "{\"bridge\":\"cni0\",\"ipMasq\":true,\"ipam\":{\"ranges\":[[{\"subnet\":\"10.88.0.0/16\"}],[{\"subnet\":\"2001:4860:4860::/64\"}]],\"routes\":[{\"dst\":\"0.0.0.0/0\"},{\"dst\":\"::/0\"}],\"type\":\"host-local\"},\"isGateway\":true,\"promiscMode\":true,\"type\":\"bridge\"}"
            },
            {
              "Network": {
                "type": "portmap",
                "capabilities": {
                  "portMappings": true
                },
                "ipam": {},
                "dns": {}
              },
              "Source": "{\"capabilities\":{\"portMappings\":true},\"type\":\"portmap\"}"
            }
          ],
          "Source": "{\n  \"cniVersion\": \"0.4.0\",\n  \"name\": \"containerd-net\",\n  \"plugins\": [\n    {\n      \"type\": \"bridge\",\n      \"bridge\": \"cni0\",\n      \"isGateway\": true,\n      \"ipMasq\": true,\n      \"promiscMode\": true,\n      \"ipam\": {\n        \"type\": \"host-local\",\n        \"ranges\": [\n          [{\n            \"subnet\": \"10.88.0.0/16\"\n          }],\n          [{\n            \"subnet\": \"2001:4860:4860::/64\"\n          }]\n        ],\n        \"routes\": [\n          { \"dst\": \"0.0.0.0/0\" },\n          { \"dst\": \"::/0\" }\n        ]\n      }\n    },\n    {\n      \"type\": \"portmap\",\n      \"capabilities\": {\"portMappings\": true}\n    }\n  ]\n}\n"
        },
        "IFName": "eth0"
      }
    ]
  },
  "config": {
    "containerd": {
      "snapshotter": "overlayfs",
      "defaultRuntimeName": "runc",
      "defaultRuntime": {
        "runtimeType": "",
        "runtimeEngine": "",
        "PodAnnotations": null,
        "ContainerAnnotations": null,
        "runtimeRoot": "",
        "options": null,
        "privileged_without_host_devices": false,
        "baseRuntimeSpec": ""
      },
      "untrustedWorkloadRuntime": {
        "runtimeType": "",
        "runtimeEngine": "",
        "PodAnnotations": null,
        "ContainerAnnotations": null,
        "runtimeRoot": "",
        "options": null,
        "privileged_without_host_devices": false,
        "baseRuntimeSpec": ""
      },
      "runtimes": {
        "runc": {
          "runtimeType": "io.containerd.runc.v2",
          "runtimeEngine": "",
          "PodAnnotations": null,
          "ContainerAnnotations": null,
          "runtimeRoot": "",
          "options": {},
          "privileged_without_host_devices": false,
          "baseRuntimeSpec": ""
        }
      },
      "noPivot": false,
      "disableSnapshotAnnotations": false,
      "discardUnpackedLayers": false
    },
    "cni": {
      "binDir": "/opt/cni/bin",
      "confDir": "/etc/cni/net.d",
      "maxConfNum": 1,
      "confTemplate": ""
    },
    "registry": {
      "configPath": "",
      "mirrors": {
        "docker.io": {
          "endpoint": [
            "https://registry-1.docker.io"
          ]
        }
      },
      "configs": null,
      "auths": null,
      "headers": null
    },
    "imageDecryption": {
      "keyModel": ""
    },
    "disableTCPService": true,
    "streamServerAddress": "127.0.0.1",
    "streamServerPort": "0",
    "streamIdleTimeout": "4h0m0s",
    "enableSelinux": false,
    "selinuxCategoryRange": 1024,
    "sandboxImage": "k8s.gcr.io/pause:3.2",
    "statsCollectPeriod": 10,
    "systemdCgroup": false,
    "enableTLSStreaming": false,
    "x509KeyPairStreaming": {
      "tlsCertFile": "",
      "tlsKeyFile": ""
    },
    "maxContainerLogSize": 16384,
    "disableCgroup": false,
    "disableApparmor": false,
    "restrictOOMScoreAdj": false,
    "maxConcurrentDownloads": 3,
    "disableProcMount": false,
    "unsetSeccompProfile": "",
    "tolerateMissingHugetlbController": true,
    "disableHugetlbController": true,
    "ignoreImageDefinedVolumes": false,
    "netnsMountsUnderStateDir": false,
    "containerdRootDir": "/var/lib/containerd",
    "containerdEndpoint": "/run/containerd/containerd.sock",
    "rootDir": "/var/lib/containerd/io.containerd.grpc.v1.cri",
    "stateDir": "/run/containerd/io.containerd.grpc.v1.cri"
  },
  "golang": "go1.15.5",
  "lastCNILoadStatus": "OK",
  "runcOptions": {
    "SystemdCgroup": false
  }
}

mikebrow · 2021-04-02T18:53:22Z

the other reported ci issues look to be well known ..

theopenlab-ci · 2021-04-04T13:56:03Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 14m 08s (non-voting)

adisky · 2021-04-04T15:54:30Z

@mikebrow Thanks for the review, I tried changing the approach here. I updated the runtime options field to map[string]interface{} and where ever we require it as toml.Tree we can convert it using toml.TreeFromMap(). This way it is generating complete output for options while marshalling it from go to json and CRI status response is complete.

crictl info output with this PR (only runtimes output shown here)

  "runtimes": {
        "runc": {
          "runtimeType": "io.containerd.runc.v2",
          "runtimeEngine": "",
          "PodAnnotations": null,
          "ContainerAnnotations": null,
          "runtimeRoot": "",
          "options": {
            "SystemdCgroup": true
          },
          "privileged_without_host_devices": false,
          "baseRuntimeSpec": ""
        }

mikebrow

LGTM
Given:t

// Tree is the result of the parsing of a TOML file.
type Tree struct {
	values    map[string]interface{} // string -> *tomlValue, *Tree, []*Tree
...

This seems to be a workable approach :-)
@mxpv thoughts?

mxpv · 2021-04-05T16:36:31Z

I'd add a comment to explain the motivation of this intermediate step.
Other than that LGTM on green.

adisky · 2021-04-06T08:44:01Z

@mxpv @mikebrow Addressed review comments.

theopenlab-ci · 2021-04-06T08:50:33Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 52s (non-voting)

mikebrow

LGTM
please rebase... to pick up the fixes to cri test

Changing Runtime.Options type to map[string]interface{} to correctly marshal it from go to JSON. See issue: kubernetes-sigs/cri-tools#728 Signed-off-by: Aditi Sharma <[email protected]>

theopenlab-ci · 2021-04-08T09:49:16Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 58s (non-voting)

adisky · 2021-04-08T12:28:03Z

LGTM
please rebase... to pick up the fixes to cri test

Done!!!

oldthreefeng · 2021-04-09T05:46:14Z

nice work!!!

k8s-ci-robot added the needs-ok-to-test label Apr 1, 2021

adisky changed the title ~~Add runtime options in CRI Status response~~ [WIP] Add runtime options in CRI Status response Apr 1, 2021

adisky force-pushed the fix-toml branch from c8ba106 to cf5490f Compare April 2, 2021 05:24

mikebrow approved these changes Apr 2, 2021

View reviewed changes

mikebrow requested changes Apr 2, 2021

View reviewed changes

adisky changed the title ~~[WIP] Add runtime options in CRI Status response~~ Change the type of CRI runtime option Apr 4, 2021

mikebrow approved these changes Apr 4, 2021

View reviewed changes

adisky force-pushed the fix-toml branch 4 times, most recently from 3446911 to 2dea684 Compare April 6, 2021 08:42

mikebrow approved these changes Apr 7, 2021

View reviewed changes

Change CRI config runtime options type

4d41174

Changing Runtime.Options type to map[string]interface{} to correctly marshal it from go to JSON. See issue: kubernetes-sigs/cri-tools#728 Signed-off-by: Aditi Sharma <[email protected]>

adisky force-pushed the fix-toml branch from 2dea684 to 4d41174 Compare April 8, 2021 09:41

mikebrow merged commit 7648ad2 into containerd:master Apr 8, 2021

adisky mentioned this pull request Apr 9, 2021

crictl info get wrong container runtime cgroupdrives when use containerd. kubernetes-sigs/cri-tools#728

Closed

adisky mentioned this pull request Jul 1, 2021

containerd.runtimes.runc（SystemdCgroup = true） #5656

Closed

katiewasnothere mentioned this pull request Apr 29, 2022

Replace toml package kevpar/cri#36

Merged

Conversation

adisky commented Apr 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Apr 1, 2021

Uh oh!

adisky commented Apr 1, 2021

Uh oh!

theopenlab-ci Bot commented Apr 1, 2021

Uh oh!

kzys commented Apr 1, 2021

Uh oh!

adisky commented Apr 1, 2021

Uh oh!

dmcgowan commented Apr 2, 2021

Uh oh!

adisky commented Apr 2, 2021

Uh oh!

theopenlab-ci Bot commented Apr 2, 2021

Uh oh!

mikebrow left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikebrow left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikebrow commented Apr 2, 2021

Uh oh!

theopenlab-ci Bot commented Apr 4, 2021

Uh oh!

adisky commented Apr 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

mxpv commented Apr 5, 2021

Uh oh!

adisky commented Apr 6, 2021

Uh oh!

theopenlab-ci Bot commented Apr 6, 2021

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

theopenlab-ci Bot commented Apr 8, 2021

Uh oh!

adisky commented Apr 8, 2021

Uh oh!

oldthreefeng commented Apr 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

adisky commented Apr 1, 2021 •

edited

Loading

mikebrow left a comment •

edited

Loading

mikebrow left a comment •

edited

Loading

adisky commented Apr 4, 2021 •

edited

Loading