Repository: kubernetes/kubernetes (language: go)

Issue:
E2eNode Suite.[It] [sig-node] Summary API [NodeConformance] when querying /stats/summary should report resource usage through the stats apiChanges
### What happened?

Test is failing on crio conformance (cgroup v1 / cgroup v2)

### What did you expect to happen?

Test doesn't fail.

### How can we reproduce it (as minimally and precisely as possible)?

https://testgrid.k8s.io/sig-node-release-blocking#ci-crio-cgroupv1-node-e2e-conformance

You can reproduce it on a PR with 
/test pull-kubernetes-node-crio-e2e.

### Anything else we need to know?

I reverted go bump and that was not the case.

### Kubernetes version

master

### Cloud provider

CI

### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here
```

</details>


### Install tools

<details>

</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>



Discussion / hints:
This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the `triage/accepted` label and provide further guidance.

The `triage/accepted` label can be added by org members by writing `/triage accepted` in a comment.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
/priority critical

@kannon92: The label(s) `priority/critical` cannot be applied, because the repository doesn't have them.

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/126314#issuecomment-2246364816):

>/priority critical
>


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
failed to get imageFS data

```
E0723 20:50:30.009868    3866 handler.go:180] "HTTP InternalServerError serving" err="failed to get imageFs stats: failed to get container fs info: non-existent label \"crio-containers\"" request="/stats/summary"
I0723 20:50:30.010083    3866 httplog.go:134] "HTTP" verb="GET" URI="/stats/summary" latency="1.016589ms" userAgent="Go-http-client/1.1" audit-ID="" srcIP="127.0.0.1:39784" resp=500 statusStack=<
	
	goroutine 7917 [running]:
	k8s.io/apiserver/pkg/server/httplog.(*respLogger).recordStatus(0xc000be80b0, 0xc000061268?)
		k8s.io/apiserver/pkg/server/httplog/httplog.go:336 +0xf2
	k8s.io/apiserver/pkg/server/httplog.(*respLogger).WriteHeader(0xc000be80b0, 0x1f4)
		k8s.io/apiserver/pkg/server/httplog/httplog.go:316 +0x1d
	github.com/emicklei/go-restful/v3.(*Response).WriteHeader(...)
		github.com/emicklei/go-restful/v3@v3.11.0/response.go:224
	github.com/emicklei/go-restful/v3.(*Response).WriteErrorString(0xc0003be850, 0x1f4?, {0xc0001d6680, 0x72})
		github.com/emicklei/go-restful/v3@v3.11.0/response.go:204 +0xa4
	k8s.io/kubernetes/pkg/kubelet/server/stats.handleError(0xc0003be850, {0x2e81dd0, 0xe}, {0x32fa1a0, 0xc000eaabb0})
		k8s.io/kubernetes/pkg/kubelet/server/stats/handler.go:181 +0x1c5
	k8s.io/kubernetes/pkg/kubelet/server/stats.(*handler).handleSummary(0xc0008c8b40, 0xc000ea24c0, 0xc0003be850)
		k8s.io/kubernetes/pkg/kubelet/server/stats/handler.go:160 +0x15a
	github.com/emicklei/go-restful/v3.(*FilterChain).ProcessFilter(0x0?, 0x332ed28?, 0xc0011e2f60?)
		github.com/emicklei/go-restful/v3@v3.11.0/filter.go:23 +0x53
	go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful.OTelFilter.func1(0xc000ea24c0, 0xc0003be850, 0xc000493310)
		go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful@v0.42.0/restful.go:78 +0x9f6
	github.com/emicklei/go-restful/v3.(*FilterChain).ProcessFilter(0xc00157a388?, 0x332a1c0?, 0xc0011e2ff0?)
		github.com/emicklei/go-restful/v3@v3.11.0/filter.go:21 +0x42
	github.com/emicklei/go-restful/v3.(*Container).dispatch(0xc0008bee10, {0x332a1c0, 0xc0011e2ff0}, 0xc00024a500)
		github.com/emicklei/go-restful/v3@v3.11.0/container.go:296 +0x9cc
	net/http.HandlerFunc.ServeHTTP(0xc0001eb340?, {0x332a1c0?, 0xc0011e2ff0?}, 0x0?)
		net/http/server.go:2220 +0x29
	net/http.(*ServeMux).ServeHTTP(0xc00131a8b8?, {0x332a1c0, 0xc0011e2ff0}, 0xc00024a500)
		net/http/server.go:2747 +0x1ca
	github.com/emicklei/go-restful/v3.(*Container).ServeHTTP(0x1d6c6e0?, {0x332a1c0?, 0xc0011e2ff0?}, 0x32f15a8?)
		github.com/emicklei/go-restful/v3@v3.11.0/container.go:316 +0x1cb
	k8s.io/kubernetes/pkg/kubelet/server.(*Server).ServeHTTP.WithLogging.withLogging.func2({0x3317460, 0xc00118c000}, 0xc00024a280)
		k8s.io/apiserver/pkg/server/httplog/httplog.go:139 +0x224
	net/http.HandlerFunc.ServeHTTP(...)
		net/http/server.go:2220
	k8s.io/kubernetes/pkg/kubelet/server.(*Server).ServeHTTP(0xc00041d4a0, {0x3317460, 0xc00118c000}, 0xc00024a280)
		k8s.io/kubernetes/pkg/kubelet/server/server.go:1089 +0x548
	net/http.serverHandler.ServeHTTP({0xc0011e2d20?}, {0x3317460?, 0xc00118c000?}, 0x6?)
		net/http/server.go:3210 +0x8e
	net/http.(*conn).serve(0xc001544750, {0x332ed28, 0xc000a469f0})
		net/http/server.go:2092 +0x5d0
	created by net/http.(*Server).Serve in goroutine 212
		net/http/server.go:3360 +0x485
 > addedInfo=<
	logging error output: "Internal Error: failed to get imageFs stats: failed to get container fs info: non-existent label \"crio-containers\""
 >```
Ah. Yea this confirms it. 

https://github.com/kubernetes/kubernetes/pull/126313

@kwilczynski looks like the KEP-4191 bump to beta is causing the failure.
/priority critical-urgent 
/sig node
/assign kwilczynski 
```console
I0724 02:45:59.257147    3871 eviction_manager.go:251] "Eviction manager: synchronize housekeeping"
E0724 02:45:59.257247    3871 eviction_manager.go:257] "Eviction manager: failed to get HasDedicatedImageFs" err="failed to get container fs info: non-existent label \"crio-containers\""
E0724 02:45:59.257264    3871 eviction_manager.go:212] "Eviction manager: failed to synchronize" err="eviction manager: failed to get HasDedicatedImageFs: failed to get container fs info: non-existent label \"crio-containers\""
```
CRI-O version and runtime configuration from the test run:

- Version string (from the log)

```console
Jul 24 02:43:41 tmp-node-e2e-64ce3588-fedora-coreos-40-20240701-3-0-gcp-x86-64 crio[2647]: time="2024-07-24 02:43:41.270147552Z" level=info msg="Starting CRI-O, version: 1.31.0, git: fd87228975e28a2d53aa2960cb14520cf51f0245(clean)"
```

- Configuration

```console
Current CRI-O configuration:
[crio]
  root = "/var/lib/containers/storage"
  runroot = "/run/containers/storage"
  imagestore = ""
  storage_driver = "overlay"
  storage_option = ["overlay.imagestore=/usr/lib/containers/storage", "overlay.mountopt=nodev,metacopy=on"]
  log_dir = "/var/log/crio/pods"
  version_file = "/var/run/crio/version"
  version_file_persist = ""
  clean_shutdown_file = "/var/lib/crio/clean.shutdown"
  internal_wipe = true
  internal_repair = false
  [crio.api]
    grpc_max_send_msg_size = 83886080
    grpc_max_recv_msg_size = 83886080
    listen = "/var/run/crio/crio.sock"
    stream_address = "127.0.0.1"
    stream_port = "0"
    stream_enable_tls = false
    stream_tls_cert = ""
    stream_tls_key = ""
    stream_tls_ca = ""
    stream_idle_timeout = ""
  [crio.runtime]
    no_pivot = false
    selinux = true
    log_to_journald = false
    drop_infra_ctr = false
    read_only = false
    hooks_dir = ["/usr/share/containers/oci/hooks.d"]
    default_capabilities = ["CHOWN", "DAC_OVERRIDE", "FSETID", "FOWNER", "SETGID", "SETUID", "SETPCAP", "NET_BIND_SERVICE", "KILL"]
    add_inheritable_capabilities = false
    allowed_devices = ["/dev/fuse"]
    cdi_spec_dirs = ["/etc/cdi", "/var/run/cdi"]
    device_ownership_from_security_context = false
    default_runtime = "runc"
    decryption_keys_path = "/etc/crio/keys/"
    conmon = ""
    conmon_cgroup = ""
    seccomp_profile = ""
    apparmor_profile = "crio-default"
    blockio_config_file = ""
    blockio_reload = false
    irqbalance_config_file = "/etc/sysconfig/irqbalance"
    rdt_config_file = ""
    cgroup_manager = "systemd"
    default_mounts_file = ""
    container_exits_dir = "/var/run/crio/exits"
    container_attach_socket_dir = "/var/run/crio"
    bind_mount_prefix = ""
    uid_mappings = ""
    minimum_mappable_uid = -1
    gid_mappings = ""
    minimum_mappable_gid = -1
    log_level = "debug"
    log_filter = ""
    namespaces_dir = "/var/run"
    pinns_path = "/usr/local/bin/pinns"
    enable_criu_support = true
    pids_limit = -1
    log_size_max = -1
    ctr_stop_timeout = 30
    separate_pull_cgroup = ""
    infra_ctr_cpuset = ""
    shared_cpuset = ""
    enable_pod_events = false
    irqbalance_config_restore_file = "/etc/sysconfig/orig_irq_banned_cpus"
    hostnetwork_disable_selinux = true
    disable_hostport_mapping = false
    timezone = ""
    [crio.runtime.runtimes]
      [crio.runtime.runtimes.crun]
        runtime_config_path = ""
        runtime_path = "/usr/local/bin/crio-crun"
        runtime_type = ""
        runtime_root = ""
        monitor_path = "/usr/local/bin/crio-conmon"
        monitor_cgroup = "system.slice"
        container_min_memory = "12MiB"
      [crio.runtime.runtimes.runc]
        runtime_config_path = ""
        runtime_path = "/usr/local/bin/crio-runc"
        runtime_type = ""
        runtime_root = ""
        monitor_path = "/usr/local/bin/crio-conmon"
        monitor_cgroup = "system.slice"
        container_min_memory = "12MiB"
      [crio.runtime.runtimes.test-handler]
        runtime_config_path = ""
        runtime_path = "/usr/local/bin/crio-runc"
        runtime_type = ""
        runtime_root = ""
        monitor_path = "/usr/local/bin/crio-conmon"
        monitor_cgroup = "system.slice"
        container_min_memory = "12MiB"
  [crio.image]
    default_transport = "docker://"
    global_auth_file = ""
    pause_image = "registry.k8s.io/pause:3.9"
    pause_image_auth_file = ""
    pause_command = "/pause"
    signature_policy = "/etc/crio/policy.json"
    signature_policy_dir = "/etc/crio/policies"
    image_volumes = "mkdir"
    big_files_temporary_dir = ""
    auto_reload_registries = false
  [crio.network]
    cni_default_network = ""
    network_dir = "/etc/cni/net.d/"
    plugin_dirs = ["/opt/cni/bin/"]
  [crio.metrics]
    enable_metrics = false
    metrics_collectors = ["image_pulls_layer_size", "containers_events_dropped_total", "containers_oom_total", "processes_defunct", "operations_total", "operations_latency_seconds", "operations_latency_seconds_total", "operations_errors_total", "image_pulls_bytes_total", "image_pulls_skipped_bytes_total", "image_pulls_failure_total", "image_pulls_success_total", "image_layer_reuse_total", "containers_oom_count_total", "containers_seccomp_notifier_count_total", "resources_stalled_at_stage"]
    metrics_host = "127.0.0.1"
    metrics_port = 9090
    metrics_socket = ""
    metrics_cert = ""
    metrics_key = ""
  [crio.tracing]
    enable_tracing = false
    tracing_endpoint = "0.0.0.0:4317"
    tracing_sampling_rate_per_million = 0
  [crio.stats]
    stats_collection_period = 0
    collection_period = 0
  [crio.nri]
    enable_nri = true
    nri_listen = "/var/run/nri/nri.sock"
    nri_plugin_dir = "/opt/nri/plugins"
    nri_plugin_config_dir = "/etc/nri/conf.d"
    nri_plugin_registration_timeout = "5s"
    nri_plugin_request_timeout = "2s"
    nri_disable_connections = false
```
CRI-O is not configured to enable the split image file system per:

_(an excerpt from the runtime configuration)_

```console
root = "/var/lib/containers/storage"
runroot = "/run/containers/storage"
imagestore = ""
storage_driver = "overlay"
storage_option = ["overlay.imagestore=/usr/lib/containers/storage", "overlay.mountopt=nodev,metacopy=on"]
```
The code that is returning the error:

https://github.com/kubernetes/kubernetes/blob/c75e30d0495dc42a017bc1f4565831ce4d1c928a/pkg/kubelet/eviction/eviction_manager.go#L251-L278

https://github.com/kubernetes/kubernetes/blob/57d197fb890a50a508c4e9e485da5076a6d1b055/pkg/kubelet/server/stats/summary.go#L85-L88
Other logs I see:

```
I0724 02:54:47.717008    3966 eviction_manager.go:251] "Eviction manager: synchronize housekeeping"
E0724 02:54:47.717138    3966 eviction_manager.go:257] "Eviction manager: failed to get HasDedicatedImageFs" err="failed to get container fs info: non-existent label \"crio-containers\""
E0724 02:54:47.717191    3966 eviction_manager.go:212] "Eviction manager: failed to synchronize" err="eviction manager: failed to get HasDedicatedImageFs: failed to get container fs info: non-existent label \"crio-containers\""
```

@kannon92 @kwilczynski Are we planning to get this fixed for v1.31 or revert the promotion of KEP 4191 to beta with https://github.com/kubernetes/kubernetes/pull/126313. Since code freeze is in effect now, we'd need approval from the release team either way. If we choose to revert the graduation to beta, the KEP won't be part of v1.31.
> crio-containers

I think we have a bug where we have the feature enabled but the container runtime is not configured.
@sreeram-venkitesh, I am looking at fixing this as soon as possible...
> @kannon92 @kwilczynski Are we planning to get this fixed for v1.31 or revert the promotion of KEP 4191 to beta with #126313. Since code freeze is in effect now, we'd need approval from the release team either way. If we choose to revert the graduation to beta, the KEP won't be part of v1.31.

That PR was mostly to find out the issue. I'd like to fix it and we will file a release exception as we are past the freeze.
> I'd like to fix it and we will file a release exception as we are past the freeze.

@kannon92 @kwilczynski Since you're planning to file an exception request, I'm removing the KEP from the milestone so that the release team can better track KEPs. If the issue is not fixed and the exception request is not raised, we would still need to revert the KEP back into alpha (https://github.com/kubernetes/kubernetes/pull/126313). 

We should either fix the issue and merge any additional code changes with an exception request or revert the feature gate to alpha as done in the above PR.
/cc @kubernetes/release-team-release-signal 
> I think we have a bug where we have the feature enabled but the container runtime is not configured.

@kannon92, this is actually going to be quite a common use case, as users would most likely not have their runtimes configured initially or even at all (if someone does not care about things like a split or separate filesystem).

I wonder how we can reliably detect whether a runtime has support enabled or not. At the moment, the error originates from cAdvisor, where we ask for stats under a specific label, which does not exist. Since the feature is enabled in Kubernetes, we assume that the runtime also has it enabled and send the label that pertains to ContainreFs.

There is a features discovery bit to the CRI API, which we already use to divulge details about support for Recursive Read-only Mounts, Supplemental Groups, etc. I wonder if we should also convey support about split filesystem support that way?
/milestone v1.31

> E0723 16:50:45.279873    3992 eviction_manager.go:257] "Eviction manager: failed to get HasDedicatedImageFs" err="failed to get container fs info: non-existent label \"crio-containers\""
> E0723 16:50:45.279891    3992 eviction_manager.go:212] "Eviction manager: failed to synchronize" err="eviction manager: failed to get HasDedicatedImageFs: failed to get container fs info: non-existent label \"crio-containers\""
 

https://storage.googleapis.com/kubernetes-jenkins/logs/ci-crio-cgroupv1-node-e2e-conformance/1815785399683911680/artifacts/tmp-node-e2e-40ce48f0-fedora-coreos-40-20240701-3-0-gcp-x86-64/kubelet.log

Not only stat summary, we can see other similar failures.
So this only fails in the case of crio with feature on but system is not configured. I used the ImgaeFsInFo to detect if the system is split so I am curious if something went wrong there.

Containerd is not failing at all with this feature on so we are detecting containers lack of support without issue.
@kannon92, this is easily reproducible if you have CRI-O running without the `imagestore` option set.

So, is this what you mean by "system not configured"?
Yes.
@kannon92 @kwilczynski I see https://github.com/kubernetes/kubernetes/pull/126313#issuecomment-2247686588, is this going to be reverted or are you working on a fix? 

Fix patch:
diff --git a/pkg/kubelet/stats/cadvisor_stats_provider.go b/pkg/kubelet/stats/cadvisor_stats_provider.go
index e46a073a71859..b2330814de2e5 100644
--- a/pkg/kubelet/stats/cadvisor_stats_provider.go
+++ b/pkg/kubelet/stats/cadvisor_stats_provider.go
@@ -267,10 +267,6 @@ func (p *cadvisorStatsProvider) ImageFsStats(ctx context.Context) (imageFsRet *s
 		}
 		return imageFs, imageFs, nil
 	}
-	containerFsInfo, err := p.cadvisor.ContainerFsInfo()
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed to get container fs info: %v", err)
-	}
 	imageStats, err := p.imageService.ImageFsInfo(ctx)
 	if err != nil || imageStats == nil {
 		return nil, nil, fmt.Errorf("failed to get image stats: %v", err)
@@ -296,10 +292,17 @@ func (p *cadvisorStatsProvider) ImageFsStats(ctx context.Context) (imageFsRet *s
 		Inodes:         imageFsInfo.Inodes,
 		InodesUsed:     imageFsInodesUsed,
 	}
+	// We rely on cadvisor to have the crio-containers label for split filesystem case.
+	// We return to avoid checking ContainerFsInfo.
 	if !splitFileSystem {
 		return fsStats, fsStats, nil
 	}
 
+	containerFsInfo, err := p.cadvisor.ContainerFsInfo()
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed to get container fs info: %w", err)
+	}
+
 	containerFs := imageStats.ContainerFilesystems[0]
 	var containerFsInodesUsed *uint64
 	if containerFsInfo.Inodes != nil && containerFsInfo.InodesFree != nil {


Test patch:
diff --git a/pkg/kubelet/stats/cadvisor_stats_provider_test.go b/pkg/kubelet/stats/cadvisor_stats_provider_test.go
index fb4351f19613f..bff66f7acca07 100644
--- a/pkg/kubelet/stats/cadvisor_stats_provider_test.go
+++ b/pkg/kubelet/stats/cadvisor_stats_provider_test.go
@@ -569,7 +569,6 @@ func TestCadvisorImagesFsStats(t *testing.T) {
 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletSeparateDiskGC, true)
 
 	mockCadvisor.EXPECT().ImagesFsInfo().Return(imageFsInfo, nil)
-	mockCadvisor.EXPECT().ContainerFsInfo().Return(imageFsInfo, nil)
 	mockRuntime.EXPECT().ImageFsInfo(ctx).Return(imageFsInfoResponse, nil)
 
 	provider := newCadvisorStatsProvider(mockCadvisor, &fakeResourceAnalyzer{}, mockRuntime, nil, NewFakeHostStatsProvider())