Mirantis / cri-dockerd

dockerd as a compliant Container Runtime Interface for Kubernetes
https://mirantis.github.io/cri-dockerd/
Apache License 2.0
1.12k stars 291 forks source link

Switch to cri container log format #213

Open nwneisen opened 1 year ago

nwneisen commented 1 year ago

cri-dockerd currently writes container logs using the docker json format

{"log":"I0719 22:04:51.982483       1 log.go:195] Started HTTP server on port 8080\n","stream":"stderr","time":"2023-07-19T22:04:51.993532538Z"}
{"log":"I0719 22:04:51.982923       1 log.go:195] Started UDP server on port  8081\n","stream":"stderr","time":"2023-07-19T22:04:51.993575341Z"}
{"log":"I0719 22:05:48.595826       1 log.go:195] GET /\n","stream":"stderr","time":"2023-07-19T22:05:48.595996805Z"}
{"log":"I0719 22:05:48.618534       1 log.go:195] GET /\n","stream":"stderr","time":"2023-07-19T22:05:48.618656559Z"}

This format is being deprecated in other projects https://github.com/kubernetes-sigs/cri-tools/pull/1216

As cri-dockerd is intended to be a cri interface, it should default to container logs being in the cri format.

afbjorklund commented 1 year ago

As far as I can tell, it was deleted and not deprecated. Not sure what it being removed from critest means, though?

Parser still exists in pkg/kubelet/kuberuntime/logs/logs.go for 1.28, so it is not completely removed from CRI just yet.

nwneisen commented 1 year ago

critest is used for the cri-dockerd integration tests which always pull the latest version. It is at least a part of the reason they have been failing on master.

afbjorklund commented 1 year ago

Unfortunately upstream removed all tests for docker in 1.26, blaming dockershim

nwneisen commented 1 year ago

My thoughts on this are that cri-tools is supposed to be a test suite and debug tools to be consumed by CRI projects. They shouldn't be testing any CRIs but rather be a standard interface for all of them.

cri-dockerd is failing its tests because it is no longer compliant with the standard interface. What makes it special from other CRIs that it should be tested as part of the cri-tools project?

afbjorklund commented 1 year ago

Currently they are testing with "both" container runtimes, but only look at containerd...

I don't think there is a standard interface, most of CRI is just ad-hoc and implementation?

But I don't know, still trying to remove dockershim.sock - which will still be there in 1.28*

* It hasn't worked since 1.24, but it will still look for it! At least it will continue looking now.

afbjorklund commented 12 months ago

It seems to be failing in the critest still (tried 1.28.0), guess the new CRI log format was never merged ?

Summarizing 24 Failures:
  [FAIL] [k8s.io] Container Mount Propagation runtime should support mount propagation [It] mount with 'rprivate' should not support propagation
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container.go:413
  [FAIL] [k8s.io] Container Mount Propagation runtime should support mount propagation [It] mount with 'rshared' should support propagation from host to container and vice versa
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container_linux.go:269
  [FAIL] [k8s.io] Container Mount Propagation runtime should support mount propagation [It] mount with 'rslave' should support propagation from host to container
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container_linux.go:269
  [FAIL] [k8s.io] Security Context NamespaceOption [It] runtime should support PodPID
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container.go:476
  [FAIL] [k8s.io] Security Context NamespaceOption [It] runtime should support HostNetwork is true
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container.go:584
  [FAIL] [k8s.io] Security Context NamespaceOption [It] runtime should support HostNetwork is false
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container.go:584
  [FAIL] [k8s.io] Security Context bucket [It] if the container's primary UID belongs to some groups in the image, runtime should add SupplementalGroups to them
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Security Context bucket [It] runtime should support RunAsGroup
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Security Context bucket [It] runtime should support that ReadOnlyRootfs is false
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Security Context bucket [It] runtime should support that ReadOnlyRootfs is true
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Security Context SeccompProfilePath [It] should support seccomp localhost profile on the container
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/security_context_linux.go:1196
  [FAIL] [k8s.io] Security Context SeccompProfilePath [It] should support seccomp default on the container
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/security_context_linux.go:1200
  [FAIL] [k8s.io] Security Context SeccompProfilePath [It] runtime should support an seccomp profile that blocks setting hostname with SYS_ADMIN
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/security_context_linux.go:1278
  [FAIL] [k8s.io] Security Context NoNewPrivs [BeforeEach] should not allow privilege escalation when true
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Security Context NoNewPrivs [BeforeEach] should allow privilege escalation when false
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Container runtime should support basic operations on container [It] runtime should support execSync with timeout [Conformance]
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container.go:176
  [FAIL] [k8s.io] Container runtime should support log [BeforeEach] runtime should support starting container with log [Conformance]
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Container runtime should support log [BeforeEach] runtime should support reopening container log [Conformance]
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Streaming runtime should support streaming interfaces [It] runtime should support exec with tty=false and stdin=false [Conformance]
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container.go:413
  [FAIL] [k8s.io] Streaming runtime should support streaming interfaces [It] runtime should support portforward in host network
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container.go:413
  [FAIL] [k8s.io] Multiple Containers [Conformance] when running multiple containers in a pod [BeforeEach] should support network
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Multiple Containers [Conformance] when running multiple containers in a pod [BeforeEach] should support container log
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Multiple Containers [Conformance] when running multiple containers in a pod [BeforeEach] should support container exec
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
  [FAIL] [k8s.io] Networking runtime should support networking [It] runtime should support set hostname [Conformance]
  /home/sascha/go/src/github.com/kubernetes-sigs/cri-tools/pkg/framework/util.go:217
Ran 78 of 83 Specs in 94.031 seconds
FAIL! -- 54 Passed | 24 Failed | 0 Pending | 5 Skipped
--- FAIL: TestCRISuite (94.03s)
FAIL

"Container runtime should support log" (which does not include JSON, since https://github.com/kubernetes-sigs/cri-tools/commit/cf42ba9f344df3a3ec18892d3b22cee276cd322a)