Skip to content
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions test/extended/node/image_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ package node

import (
"context"
"fmt"
"regexp"
"strconv"
"strings"
"time"

g "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -87,6 +91,52 @@ var _ = g.Describe("[sig-node] [FeatureGate:ImageVolume] ImageVolume", func() {
verifyVolumeMounted(f, pod2, "ls", "/mnt/image/bin/oc")
})

g.It("should report kubelet image volume metrics correctly [OCP-84149]", func(ctx context.Context) {
const (
podName = "image-volume-metrics-test"
imageRef = "quay.io/crio/artifact:v1"
mountPath = "/mnt/image"
)

// Step 1: Create a pod with OCI image as volume source
g.By("Creating a pod with OCI image as volume source")
pod := buildPodWithImageVolume(f.Namespace.Name, "", podName, imageRef)
pod = createPodAndWaitForRunning(ctx, oc, pod)

// Step 2: Verify the image is mounted successfully and read-only
g.By("Verifying image volume is mounted into the container")
verifyImageVolumeMounted(f, pod, mountPath)

g.By("Verifying the mounted volume is read-only")
verifyVolumeReadOnly(f, pod, mountPath)

// Step 3: Check kubelet metrics about image volume
g.By("Checking kubelet metrics for image volume")
metrics, err := getKubeletMetrics(ctx, oc, pod.Spec.NodeName)
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get kubelet metrics")

g.By("Verifying kubelet_image_volume_requested_total metric")
requestedTotal := parseMetricValue(metrics, "kubelet_image_volume_requested_total")
o.Expect(requestedTotal).To(o.BeNumerically(">=", 1),
"kubelet_image_volume_requested_total should be at least 1")
Comment on lines +121 to +122
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we test for an exact count here? Same for the 2 other cases below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it intends to check if there's requested_total_count exists and is calculated correctly, not to check the exact count.

framework.Logf("kubelet_image_volume_requested_total: %d", requestedTotal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how framework.Logf works, but it looks a bit verbose.

If the Expect fails, it should output the number, and otherwise we don't have to know the exact number.
Same for other framework.Logf usages. I don't think we need logs for successful scenario in most cases. Passing test implies "it works correctly", and for failing scenario, Expect will do.

If it's not printed when the test passes, it's non blocking comment, but otherwise should be removed.

Copy link
Author

@lyman9966 lyman9966 Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, framework.Logf is not mandatory. Yet in certain situations, it's necessary, like this line framework.Logf("Metric %s not found in output", metricName)
But for some redundant info print, I will remove it.


g.By("Verifying kubelet_image_volume_mounted_succeed_total metric")
succeededTotal := parseMetricValue(metrics, "kubelet_image_volume_mounted_succeed_total")
o.Expect(succeededTotal).To(o.BeNumerically(">=", 1),
"kubelet_image_volume_mounted_succeed_total should be at least 1")
framework.Logf("kubelet_image_volume_mounted_succeed_total: %d", succeededTotal)

g.By("Verifying kubelet_image_volume_mounted_errors_total metric")
errorsTotal := parseMetricValue(metrics, "kubelet_image_volume_mounted_errors_total")
o.Expect(errorsTotal).To(o.Equal(0),
"kubelet_image_volume_mounted_errors_total should be 0")
framework.Logf("kubelet_image_volume_mounted_errors_total: %d", errorsTotal)

framework.Logf("All image volume metrics are reporting correctly")

})

g.Context("when subPath is used", func() {
g.It("should handle image volume with subPath", func(ctx context.Context) {
pod := buildPodWithImageVolumeSubPath(f.Namespace.Name, "", podName, image, "bin")
Expand Down Expand Up @@ -186,3 +236,93 @@ func buildPodWithMultipleImageVolumes(namespace, nodeName, podName, image1, imag
})
return pod
}

// verifyImageVolumeMounted verifies that the image volume is mounted and accessible
func verifyImageVolumeMounted(f *framework.Framework, pod *v1.Pod, mountPath string) {
g.By(fmt.Sprintf("Checking if volume is mounted at %s", mountPath))

// Check if directory exists
stdout := e2epod.ExecCommandInContainer(f, pod.Name, pod.Spec.Containers[0].Name,
"ls", "-la", mountPath)
o.Expect(stdout).NotTo(o.BeEmpty(), "Mount path should contain files")
framework.Logf("Files in %s:\n%s", mountPath, stdout)

// Verify the expected file exists
stdout = e2epod.ExecCommandInContainer(f, pod.Name, pod.Spec.Containers[0].Name,
"ls", mountPath+"/file")
o.Expect(stdout).To(o.ContainSubstring("file"), "Expected file should exist")

// Verify file content
stdout = e2epod.ExecCommandInContainer(f, pod.Name, pod.Spec.Containers[0].Name,
"cat", mountPath+"/file")
o.Expect(stdout).To(o.Equal("2"), "File content should be '1'")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
o.Expect(stdout).To(o.Equal("2"), "File content should be '1'")
o.Expect(stdout).To(o.Equal("2"), "File content should be '2'")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit(non blocking): I think only the last check is enough.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, it's ok to only keep the last check.

framework.Logf("File content verified: %s", strings.TrimSpace(stdout))
}

// verifyVolumeReadOnly verifies that the mounted volume is read-only
func verifyVolumeReadOnly(f *framework.Framework, pod *v1.Pod, mountPath string) {
g.By("Verifying the volume is mounted as read-only")

// Check mount options
stdout := e2epod.ExecCommandInContainer(f, pod.Name, pod.Spec.Containers[0].Name,
"mount")
o.Expect(stdout).To(o.ContainSubstring(mountPath), "Mount point should be listed")

// Verify read-only in mount output
mountLines := strings.Split(stdout, "\n")
for _, line := range mountLines {
if strings.Contains(line, mountPath) {
o.Expect(line).To(o.MatchRegexp(`\bro\b`),
"Volume should be mounted with 'ro' (read-only) option")
framework.Logf("Mount info: %s", line)
break
}
}

// Try to write to the volume (should fail)
g.By("Attempting to write to the read-only volume (should fail)")
_, _, err := e2epod.ExecCommandInContainerWithFullOutput(f, pod.Name, pod.Spec.Containers[0].Name,
"touch", mountPath+"/testfile")
o.Expect(err).To(o.HaveOccurred(), "Writing to read-only volume should fail")
framework.Logf("Write attempt correctly failed (volume is read-only)")
}

// getKubeletMetrics fetches kubelet metrics from a specific node
func getKubeletMetrics(ctx context.Context, oc *exutil.CLI, nodeName string) (string, error) {
metricsPath := fmt.Sprintf("/api/v1/nodes/%s/proxy/metrics", nodeName)

data, err := oc.AdminKubeClient().CoreV1().RESTClient().Get().
AbsPath(metricsPath).
DoRaw(ctx)
if err != nil {
return "", fmt.Errorf("failed to get metrics from node %s: %w", nodeName, err)
}

return string(data), nil
}

// parseMetricValue parses a Prometheus metric value from metrics output
func parseMetricValue(metrics, metricName string) int {
// Look for lines like: kubelet_image_volume_requested_total 1
// Skip HELP and TYPE lines
re := regexp.MustCompile(fmt.Sprintf(`^%s\s+(\d+)`, metricName))

lines := strings.Split(metrics, "\n")
for _, line := range lines {
line = strings.TrimSpace(line)
if strings.HasPrefix(line, "#") {
continue // Skip comment lines
}

matches := re.FindStringSubmatch(line)
if len(matches) == 2 {
value, err := strconv.Atoi(matches[1])
if err == nil {
return value
}
}
}

framework.Logf("Metric %s not found in output", metricName)
return 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning 0 when a metric isn't found could lead to misleading test results. Consider returning an error or using a tuple return (int, bool) to distinguish between "metric = 0" and "metric not found".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, I will use (int, bool) as return type.

}