libpod.conf runtimes section

[llchan]

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description
The new events_logger config param was added to libpod.conf at the end of the file, which causes it to be parsed as part of the [runtimes] block above it, leading to a type error on parsing.

This is at least the second time something has been appended to the end of libpod.conf, breaking the [runtimes] section. Could we maybe add a comment in there so we can prevent this in the future? Or is there some other toml syntax we can use to express the runtimes section, maybe something like runtimes.runc = [...], that ends the table? Thankfully in this case it led to a type error so didn't go unnoticed, but it would be bad if someone appended a string slice param at the end and it silently just got sucked into the runtimes map.

Steps to reproduce the issue:

1. podman info

Describe the results you received:

Error: could not get runtime: error decoding configuration file /jump/software/rhel7/podman-1.3.0-em/share/containers/libpod.conf: Type mismatch for 'libpod.RuntimeConfig.runtimes': Expected slice but found 'string'.

Describe the results you expected:
The usual podman info output

Output of podman version:

Version:            1.3.0
RemoteAPI Version:  1
Go Version:         go1.12.3
Built:              Tue May  7 15:22:07 2019
OS/Arch:            linux/amd64

Output of podman info --debug:

Error: could not get runtime: error decoding configuration file /jump/software/rhel7/podman-1.3.0-em/share/containers/libpod.conf: Type mismatch for 'libpod.RuntimeConfig.runtimes': Expected slice but found 'string'.

[mheon]

We need to do something about the [runtimes] section so it doesn't try parsing everything below - I don't think forcing people to have no options underneath it is a viable long-term solution

[llchan]

Sure, yeah, that's definitely better. I don't know enough about the TOML format to comment on how we might do that in a backwards-compatible manner though. Hopefully it's possible with the dotted syntax?

[llchan]

Scanning the toml key docs, I think it should be equivalent if we convert

[runtimes]
runc = [...]

to

runtimes.runc = [...]

with the main difference being that it doesnt continue consuming lines for the runtimes table.

[llchan]

As a side note, I think it would be helpful to add a step to the CI that tests podman info against the default libpod.conf on a fully wiped setup (i.e. delete the database file, since that can influence the outcome).

[llchan]

Naively trying that change, seems like it doesnt work: Bare keys cannot contain '.'. Searching for this error, seems like people are saying we need to use DecodeFile rather than Decode, so that may be all that's required to get it up and running. I don't have the bandwidth today to hack at this and test a new build but just figured I'd relay the information.

[mcritchlow]

Experiencing this on Arch Linux as well, latest version 1.3.0, clean install with no configuration changes.

~ % podman info
Error: could not get runtime: error decoding configuration file /usr/share/containers/libpod.conf: Type mismatch for 'libpod.RuntimeConfig.runtimes': Expected slice but found 'string'.

[mheon]

We're planning on pushing a 1.3.1 soon anyways (had some serious pod-related bugs), so we'll see about getting this patched and included as well.

[mheon]

Fixing this in a more future-proof way is not promising. TOML does not have syntax for end-of-table, and dotted syntax does not seem to be supported for this case.

Best way forward seems to be to remove anything before the [runtimes] section and add a comment explaining why, and why it must be the absolute last thing in the file, and then to be more vigilant in the future when reviewing incoming PRs

[mheon]

#3116 to fix

[llchan]

Is it possible to switch to DecodeFile rather than Decode for the TOML parsing? I haven't tried it myself but from what I gather that may enable dotted keys in away that would work here.

[mheon]

Tried it, and it didn't work - same "no periods in bare keys" errors

[llchan]

Ah, did not realize this was newly added to the TOML spec.

The relevant parser issue is BurntSushi/toml#242. We can probably just leave the comment for now, and when that lands we can upgrade.

Alternatively some other parsers have added support for dotted keys (e.g. pelletier/go-toml#260), but I think it's sufficient to put a band-aid on this and wait for our parser to add support.

[mheon]

Closing for now as #3116 landed