kata-deploy: error appending to containerd config

[wilsonianb]

Description of problem

curl -sfL https://get.k3s.io | sh -
kubectl apply -f https://raw.githubusercontent.com/kata-containers/packaging/master/kata-deploy/kata-rbac.yaml
kubectl apply -f https://raw.githubusercontent.com/wilsonianb/packaging/k3s-test/kata-deploy/kata-deploy.yaml

or I suspect installing kata containers via kata-deploy on any Kubernetes node with a containerd config similar to the k3s default.

Expected result

kata-deploy should produce a valid containerd config.toml

Actual result

In this case, appending an array of tables to the existing containerd config.toml (#797) produces an invalid configuration.

[plugins.opt]
  path = "/var/lib/rancher/k3s/agent/containerd"

[plugins.cri]
  stream_server_address = "127.0.0.1"
  stream_server_port = "10010"
[plugins.cri.cni]
  bin_dir = "/var/lib/rancher/k3s/data/89e07bdac6af146f1afb8d8b89f4c99d0a79ea4024f997bf37fb5a13aa44ebc5/bin"
  conf_dir = "/var/lib/rancher/k3s/agent/etc/cni/net.d"


[plugins.cri.containerd.runtimes.runc]
  runtime_type = "io.containerd.runc.v2"


[[plugins]]
  [[plugins.cri]]
   [[plugins.cri.containerd]]
     [plugins.cri.containerd.runtimes.kata]
        runtime_type = "io.containerd.kata.v2"
        [plugins.cri.containerd.runtimes.kata.options]
              ConfigPath = "/opt/kata/share/defaults/kata-containers/configuration.toml"
     [plugins.cri.containerd.runtimes.kata-fc]
        runtime_type = "io.containerd.kata-fc.v2"
        [plugins.cri.containerd.runtimes.kata-fc.options]
              ConfigPath = "/opt/kata/share/defaults/kata-containers/configuration-fc.toml"
     [plugins.cri.containerd.runtimes.kata-qemu]
        runtime_type = "io.containerd.kata-qemu.v2"
        [plugins.cri.containerd.runtimes.kata-qemu.options]
              ConfigPath = "/opt/kata/share/defaults/kata-containers/configuration-qemu.toml"
     [plugins.cri.containerd.runtimes.kata-qemu-virtiofs]
        runtime_type = "io.containerd.kata-qemu-virtiofs.v2"
        [plugins.cri.containerd.runtimes.kata-qemu-virtiofs.options]
              ConfigPath = "/opt/kata/share/defaults/kata-containers/configuration-qemu-virtiofs.toml"
     [plugins.cri.containerd.runtimes.kata-nemu]
        runtime_type = "io.containerd.kata-nemu.v2"
        [plugins.cri.containerd.runtimes.kata-nemu.options]
              ConfigPath = "/opt/kata/share/defaults/kata-containers/configuration-nemu.toml"

containerd: Near line 17 (last key parsed ''): Key '' was already created and cannot be used as an array.

From the TOML docs:

If the parent of a table or array of tables is an array element, that element must already have been defined before the child can be defined. Attempts to reverse that ordering must produce an error at parse time.

# INVALID TOML DOC
[fruit.physical]  # subtable, but to which parent element should it belong?
  color = "red"
  shape = "round"

[[fruit]]  # parser must throw an error upon discovering that "fruit" is
           # an array rather than a table
  name = "apple"

[grahamwhaley]

/cc @egernst , who was looking at the toml issues recently.
/cc @jodh-intel who might be interested to see the problem space.
OOI, iirc, some time back @mcastelino may have tried kata with k3s ... but, that may have been before kata-deploy was a thing...

[jodh-intel]

I think this is #307.

@egernst - The TOML tooling landscape is somewhat sparse fwics but we should be able to use crudini I think? Having to convert to JSON sounds somewhat extreme but if forced, the remarshal tool claims to be able to do TOML -> JSON -> TOML.

[egernst]

I didn’t know crudini could work with toml. I can give it a try...

[wilsonianb]

@egernst I've got an outdated commit that uses crudini that you can reference.

As noted there, you lose indentation in the config file with crudini, plus I'm uncertain if INI's limited nesting support would ever cause issues.

@jodh-intel It sounds like remarshal is currently using a deprecated TOML package
remarshal-project/remarshal#19

[egernst]

@wilsonianb I'm happy with that commit. Will test locally now. Do you want to PR it, otherwise I can submit your original change with modifications.

[jodh-intel]

Thanks for letting us know about both those points @wilsonianb !

[wilsonianb]

I can open a PR later today. Thanks

[egernst]

From IRC discussion with @grahamwhaley:

 seeing as apparently crudini might throw away your toml formatting - maybe you also want to combine with http://manpages.ubuntu.com/manpages/bionic/man1/tomll.1.html to put all the formatting back again... ;-)

Something to consider, @wilsonianb. I worry that linter may run into preexisting issues, but let's see..

[wilsonianb]

Actually, given this from the TOML docs:

You don't need to specify all the super-tables if you don't want to. TOML knows how to do it for you.

# [x] you
# [x.y] don't
# [x.y.z] need these
[x.y.z.w] # for this to work

I think the current appending solution would work if the super-tables weren't included:
wilsonianb@f543c7a

You'd probably still get an invalid config if any plugins.cri.containerd.runtimes.kata* fields are already specified, but maybe erroring is preferable to someone (potentially inadvertently) overwriting their existing configuration (which is what would happen with crudini)?

[egernst]

I think this makes sense. Want to PR?