Go rewrite

[phyrog]

Will close #39. For now used for tracking purposes.

[phyrog]

We need to find a different way to update the containerd config, imports does not merge key by key, but instead overwrites entire sections. See containerd/containerd#5837 (comment) for more details on this.

Here's what NVIDIA is doing: https://github.com/NVIDIA/nvidia-container-toolkit/blob/807c87e057e13fbd559369b8fd722cc7a6f4e5bb/pkg/config/engine/containerd/config_v2.go#L27

[crabarca]

@phyrog regarding the update of the containerd config.toml. The following idea comes to my mind:

Create a new TOML file that contains all the data found on a set of shim configuration files and then import only this merged file inside the containerd config.toml. This gives flexibility about which runtimes classes we want to import onto the config.toml and the advantage of selecting runtime configuration on demand

Deletions or updates could be done in the same way by generating the config file with updates keys (for update) or just by not selecting certain configs (deletion).

[phyrog]

@crabarca Importing overwrites the whole [plugins."io.containerd.grpc.v1.cri"] section, so we'd need to get these parts (e.g. the default runc config) into that new config file as well before we import it. But yeah, I agree. This is probably the way to go.

1. Parse config.toml, get the imports and plugins fields
2. Merge plugins config with shim configs and write to new file
3. Update the imports field, if necessary
   3.1. Write updated config.toml file, if necessary
4. Restart containerd

One caveat here is that if a user manually updates something in the plugins section of config.toml, this will be overridden by our config, which can result in unexpected behavior.

[phyrog]

Unfortunately neither https://github.com/BurntSushi/toml nor https://github.com/pelletier/go-toml support preserving comments when unmarshalling/marshalling, so if we modified config.toml like this to add imports, it would not preserve any comments, order, etc.

Edit: I think https://github.com/pelletier/go-toml in version 1 might be able to do it using the Tree api, like NVIDIA does.

[crabarca]

I went down the rabbit hole on both libraries to see if there were some detailed AST structure that we could leverage to persist comments when doing a merge. The problem is that usually the TOML parsers store the file as a map of key=values without an specific order and doesn't considers comments. Even if comments were parsed the following problem appears:

If we think more deeply about this problem, the key=values allow a merge scenario out-of-the-box where conflicts can be resolved. This doesn't apply to comments as comments are not identifiable by a key or specific metadata. So merging it's hard as you don't know where to put the comments in the file in order for them to keep being relevant

To conclude, I would not go after trying to merge comments and just creating an intermediary config file which contains the merge of the shim configurations and won't contain comments, which then is going to be imported into config.toml

[phyrog]

@crabarca I was mainly thinking about the libraries because we need to potentially modify config.toml to add or update the imports field to include our custom config file. I think we should find a way to update the file with the minimal possible modifications (i.e. preserving comments). In the worst case (we don't find a way to modify it with minimal changes), we could opt for creating a backup of config.toml and then just not care about minimal changes.

But I do think the Tree API of pelletier/go-toml v1 that I linked to above parses and stores comments as well Edit: comments are just skipped

For the custom config file, I agree, preserving comments is not relevant.