Docker reports error on parsing crun version about crun HOT 15 CLOSED

What is the version of docker you are testing with?
BTW Works well with Podman. :^)

from crun.

Built from tree in august, 12.

❯ docker version                                                                                                                                                                           ─╯
Client:
 Version:           unknown-version
 API version:       1.40
 Go version:        devel +0c72bd386b Fri Aug 9 17:52:39 2019 +1000
 Git commit:        f807b5ef
 Built:             Mon Aug 12 15:53:00 2019
 OS/Arch:           linux/riscv64
 Experimental:      true

Server:
 Engine:
  Version:          library-import
  API version:      1.41 (minimum version 1.12)
  Go version:       devel +0c72bd386b Fri Aug 9 17:52:39 2019 +1000
  Git commit:       library-import
  Built:            library-import
  OS/Arch:          linux/riscv64
  Experimental:     false
 containerd:
  Version:          1.2.0+unknown
  GitCommit:
 docker-init:
  Version:          0.18.0
  GitCommit:        3c53686

I know... have been using podman for a while too :)

from crun.

Must be something about the output that crun is generating that Docker does not like.

from crun.

It would be nice to get it working though, since F31 is going to cgroups v2 and runc is not be ready for it. So we are moving default to crun for the time being.

from crun.

That's great! This is the function on Docker that parses the runtime version:

// parseRuncVersion parses the output of `runc --version` and extracts the
// "version" and "git commit" from the output.
//
// Output example from `runc --version`:
//
//   runc version 1.0.0-rc5+dev
//   commit: 69663f0bd4b60df09991c08812a60108003fa340
//   spec: 1.0.0
func parseRuncVersion(v string) (version string, commit string, err error) {
	lines := strings.Split(strings.TrimSpace(v), "\n")
	for _, line := range lines {
		if strings.HasPrefix(line, "runc version") {
			version = strings.TrimSpace(strings.TrimPrefix(line, "runc version"))
			continue
		}
		if strings.HasPrefix(line, "commit:") {
			commit = strings.TrimSpace(strings.TrimPrefix(line, "commit:"))
			continue
		}
	}
	if version == "" && commit == "" {
		err = errors.Errorf("unknown output format: %s", v)
	}
	return version, commit, err
}

I think Docker shouldn't hardcore "runc version" on parsing.

from crun.

Could you fill an issue with Moby? We could easily work around it but I don't think there should be any assumption on the runtime version output if multiple runtime are supported, and should not be crun to lie about it.

from crun.

I will. Agree that it's not a thing to be fixed on crun side.
I also found other weirdnesses on Moby like if I don't have a runc binary or link, it does not start, even defining that crun is my default-runtime.
Looking for more hard-coded stuff on their side.

from crun.

Can you please tag me on the issue once you open it?

from crun.

Sure, one thing tho... I was looking at the code and is it possible to have something like "crun version 0.7" printed instead of "crun 0.7". This would ease parsing since it's the way runc does.
Do you think it's worth? I haven't looked into other runtimes to see how they print the version.

from crun.

That smells sloppy. I think we should not blindly copy what runc or other runtimes does.

from crun.

What do you think @giuseppe and @rhatdan ? Should we have something similar to how Docker parses the runtime to avoid swamping the log?

from crun.

I think having version on the line would make it easier to patch. I would just split the line via spaces and take the last value.

Why does Docker wants these values in the first place? Just for docker info?

from crun.

I think it’s the easier and faster option. I’ll submit a PR to address this. I believe it uses only for information.

from crun.

Opened moby/moby#39940. I think that with this, changing Crun won't be necessary.

from crun.

I also opened the PR #93 to address this on crun.

from crun.