E590 Graduate image volume sources to beta · kubernetes/kubernetes@a3c336d · GitHub
[go: up one dir, main page]
Skip to content

Commit a3c336d

Browse files
saschagrunertsaschagrunert
committed
Graduate image volume sources to beta
Graduate the feature to beta, by: - Allowing `subPath`/`subPathExpr` for image volumes - Modifying the CRI to pass down the (resolved) sub path - Adding metrics which are outlined in the KEP Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
1 parent 2642d82 commit a3c336d

File tree

12 files changed

+690
-464
lines changed

12 files changed

+690
-464
lines changed

‎pkg/apis/core/validation/validation.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2937,16 +2937,6 @@ func ValidateVolumeMounts(mounts []core.VolumeMount, voldevices map[string]strin
29372937
allErrs = append(allErrs, field.Invalid(idxPath.Child("mountPath"), mnt.MountPath, "must not already exist as a path in volumeDevices"))
29382938
}
29392939

2940-
// Disallow subPath/subPathExpr for image volumes
2941-
if v, ok := volumes[mnt.Name]; ok && v.Image != nil {
2942-
if len(mnt.SubPath) != 0 {
2943-
allErrs = append(allErrs, field.Invalid(idxPath.Child("subPath"), mnt.SubPath, "not allowed in image volume sources"))
2944-
}
2945-
if len(mnt.SubPathExpr) != 0 {
2946-
allErrs = append(allErrs, field.Invalid(idxPath.Child("subPathExpr"), mnt.SubPathExpr, "not allowed in image volume sources"))
2947-
}
2948-
}
2949-
29502940
if len(mnt.SubPath) > 0 {
29512941
allErrs = append(allErrs, validateLocalDescendingPath(mnt.SubPath, fldPath.Child("subPath"))...)
29522942
}

‎pkg/apis/core/validation/validation_test.go‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7146,23 +7146,21 @@ func TestValidateVolumeMounts(t *testing.T) {
71467146
}
71477147

71487148
errorCases := map[string][]core.VolumeMount{
7149-
"empty name": {{Name: "", MountPath: "/foo"}},
7150-
"name not found": {{Name: "", MountPath: "/foo"}},
7151-
"empty mountpath": {{Name: "abc", MountPath: ""}},
7152-
"mountpath collision": {{Name: "foo", MountPath: "/path/a"}, {Name: "bar", MountPath: "/path/a"}},
7153-
"absolute subpath": {{Name: "abc", MountPath: "/bar", SubPath: "/baz"}},
7154-
"subpath in ..": {{Name: "abc", MountPath: "/bar", SubPath: "../baz"}},
7155-
"subpath contains ..": {{Name: "abc", MountPath: "/bar", SubPath: "baz/../bat"}},
7156-
"subpath ends in ..": {{Name: "abc", MountPath: "/bar", SubPath: "./.."}},
7157-
"disabled MountPropagation feature gate": {{Name: "abc", MountPath: "/bar", MountPropagation: &propagation}},
7158-
"name exists in volumeDevice": {{Name: "xyz", MountPath: "/bar"}},
7159-
"mountpath exists in volumeDevice": {{Name: "uvw", MountPath: "/mnt/exists"}},
7160-
"both exist in volumeDevice": {{Name: "xyz", MountPath: "/mnt/exists"}},
7161-
"rro but not ro": {{Name: "123", MountPath: "/rro-bad1", ReadOnly: false, RecursiveReadOnly: ptr.To(core.RecursiveReadOnlyEnabled)}},
7162-
"rro with incompatible propagation": {{Name: "123", MountPath: "/rro-bad2", ReadOnly: true, RecursiveReadOnly: ptr.To(core.RecursiveReadOnlyEnabled), MountPropagation: ptr.To(core.MountPropagationHostToContainer)}},
7163-
"rro-if-possible but not ro": {{Name: "123", MountPath: "/rro-bad1", ReadOnly: false, RecursiveReadOnly: ptr.To(core.RecursiveReadOnlyIfPossible)}},
7164-
"subPath not allowed for image volume sources": {{Name: "image-volume", MountPath: "/image-volume-err-1", SubPath: "/foo"}},
7165-
"subPathExpr not allowed for image volume sources": {{Name: "image-volume", MountPath: "/image-volume-err-2", SubPathExpr: "$(POD_NAME)"}},
7149+
"empty name": {{Name: "", MountPath: "/foo"}},
7150+
"name not found": {{Name: "", MountPath: "/foo"}},
7151+
"empty mountpath": {{Name: "abc", MountPath: ""}},
7152+
"mountpath collision": {{Name: "foo", MountPath: "/path/a"}, {Name: "bar", MountPath: "/path/a"}},
7153+
"absolute subpath": {{Name: "abc", MountPath: "/bar", SubPath: "/baz"}},
7154+
"subpath in ..": {{Name: "abc", MountPath: "/bar", SubPath: "../baz"}},
7155+
"subpath contains ..": {{Name: "abc", MountPath: "/bar", SubPath: "baz/../bat"}},
7156+
"subpath ends in ..": {{Name: "abc", MountPath: "/bar", SubPath: "./.."}},
7157+
"disabled MountPropagation feature gate": {{Name: "abc", MountPath: "/bar", MountPropagation: &propagation}},
7158+
"name exists in volumeDevice": {{Name: "xyz", MountPath: "/bar"}},
7159+
"mountpath exists in volumeDevice": {{Name: "uvw", MountPath: "/mnt/exists"}},
7160+
"both exist in volumeDevice": {{Name: "xyz", MountPath: "/mnt/exists"}},
7161+
"rro but not ro": {{Name: "123", MountPath: "/rro-bad1", ReadOnly: false, RecursiveReadOnly: ptr.To(core.RecursiveReadOnlyEnabled)}},
7162+
"rro with incompatible propagation": {{Name: "123", MountPath: "/rro-bad2", ReadOnly: true, RecursiveReadOnly: ptr.To(core.RecursiveReadOnlyEnabled), MountPropagation: ptr.To(core.MountPropagationHostToContainer)}},
7163+
"rro-if-possible but not ro": {{Name: "123", MountPath: "/rro-bad1", ReadOnly: false, RecursiveReadOnly: ptr.To(core.RecursiveReadOnlyIfPossible)}},
71667164
}
71677165
badVolumeDevice := []core.VolumeDevice{
71687166
{Name: "xyz", DevicePath: "/mnt/exists"},

‎pkg/features/versioned_kube_features.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
404404

405405
ImageVolume: {
406406
{Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Alpha},
407+
{Version: version.MustParse("1.33"), Default: false, PreRelease: featuregate.Beta},
407408
},
408409

409410
InPlacePodVerticalScaling: {

‎pkg/kubelet/container/runtime.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,9 @@ type Mount struct {
472472
Propagation runtimeapi.MountPropagation
473473
// Image is set if an OCI volume as image ID or digest should get mounted (special case).
474474
Image *runtimeapi.ImageSpec
475+
// ImageSubPath is set if an image volume sub path should get mounted. This
476+
// field is only required if the above Image is set.
477+
ImageSubPath string
475478
}
476479

477480
// ImageVolumes is a map of image specs by volume name.

‎pkg/kubelet/kubelet_pods.go‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -296,13 +296,24 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
296296
}
297297

298298
var (
299-
hostPath string
300-
image *runtimeapi.ImageSpec
301-
err error
299+
hostPath string
300+
image *runtimeapi.ImageSpec
301+
imageSubPath string
302+
err error
302303
)
303304

305+
subPath := mount.SubPath
306+
if mount.SubPathExpr != "" {
307+
subPath, err = kubecontainer.ExpandContainerVolumeMounts(mount, expandEnvs)
308+
309+
if err != nil {
310+
return nil, cleanupAction, err
311+
}
312+
}
313+
304314
if imageVolumes != nil && utilfeature.DefaultFeatureGate.Enabled(features.ImageVolume) {
305315
image = imageVolumes[mount.Name]
316+
imageSubPath = subPath
306317
}
307318

308319
if image == nil {
@@ -311,15 +322,6 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
311322
return nil, cleanupAction, err
312323
}
313324

314-
subPath := mount.SubPath
315-
if mount.SubPathExpr != "" {
316-
subPath, err = kubecontainer.ExpandContainerVolumeMounts(mount, expandEnvs)
317-
318-
if err != nil {
319-
return nil, cleanupAction, err
320-
}
321-
}
322-
323325
if subPath != "" {
324326
if utilfs.IsAbs(subPath) {
325327
return nil, cleanupAction, fmt.Errorf("error SubPath `%s` must not be an absolute path", subPath)
@@ -398,6 +400,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
398400
ContainerPath: containerPath,
399401
HostPath: hostPath,
400402
Image: image,
403+
ImageSubPath: imageSubPath,
401404
ReadOnly: mount.ReadOnly || mustMountRO,
402405
RecursiveReadOnly: rro,
403406
SELinuxRelabel: relabelVolume,

‎pkg/kubelet/kuberuntime/kuberuntime_container.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ func (m *kubeGenericRuntimeManager) makeMounts(opts *kubecontainer.RunContainerO
449449
Propagation: v.Propagation,
450450
RecursiveReadOnly: v.RecursiveReadOnly,
451451
Image: v.Image,
452+
ImageSubPath: v.ImageSubPath,
452453
}
453454

454455
volumeMounts = append(volumeMounts, mount)
@@ -674,6 +675,7 @@ func toKubeContainerStatus(status *runtimeapi.ContainerStatus, runtimeName strin
674675
SELinuxRelabel: mount.SelinuxRelabel,
675676
Propagation: mount.Propagation,
676677
Image: mount.Image,
678+
ImageSubPath: mount.ImageSubPath,
677679
})
678680
}
679681
return cStatus

‎pkg/kubelet/kuberuntime/kuberuntime_manager.go‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1350,7 +1350,9 @@ func (m *kubeGenericRuntimeManager) SyncPod(ctx context.Context, pod *v1.Pod, po
13501350
}
13511351

13521352
// NOTE (aramase) podIPs are populated for single stack and dual stack clusters. Send only podIPs.
1353-
if msg, err := m.startContainer(ctx, podSandboxID, podSandboxConfig, spec, pod, podStatus, pullSecrets, podIP, podIPs, imageVolumes); err != nil {
1353+
msg, err = m.startContainer(ctx, podSandboxID, podSandboxConfig, spec, pod, podStatus, pullSecrets, podIP, podIPs, imageVolumes)
1354+
incrementImageVolumeMetrics(err, spec.container, imageVolumes)
1355+
if err != nil {
13541356
// startContainer() returns well-defined error codes that have reasonable cardinality for metrics and are
13551357
// useful to cluster administrators to distinguish "server errors" from "user errors".
13561358
metrics.StartedContainersErrorsTotal.WithLabelValues(metricLabel, err.Error()).Inc()
@@ -1427,6 +1429,27 @@ func (m *kubeGenericRuntimeManager) SyncPod(ctx context.Context, pod *v1.Pod, po
14271429
return
14281430
}
14291431

1432+
// incrementImageVolumeMetrics increments the image volume mount metrics
1433+
// depending on the provided error and the usage of the image volume mount
1434+
// within the container.
1435+
func incrementImageVolumeMetrics(err error, container *v1.Container, imageVolumes kubecontainer.ImageVolumes) {
1436+
if !utilfeature.DefaultFeatureGate.Enabled(features.ImageVolume) {
1437+
return
1438+
}
1439+
1440+
metrics.ImageVolumeRequestedTotal.Add(float64(len(imageVolumes)))
1441+
1442+
for _, m := range container.VolumeMounts {
1443+
if _, exists := imageVolumes[m.Name]; exists {
1444+
if err != nil {
1445+
metrics.ImageVolumeMountedErrorsTotal.Inc()
1446+
} else {
1447+
metrics.ImageVolumeMountedSucceedTotal.Inc()
1448+
}
1449+
}
1450+
}
1451+
}
1452+
14301453
// imageVolumePulls are the pull results for each image volume name.
14311454
type imageVolumePulls = map[string]imageVolumePullResult
14321455

‎pkg/kubelet/kuberuntime/kuberuntime_manager_test.go‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"reflect"
2525
goruntime "runtime"
2626
"sort"
27+
"strings"
2728
"testing"
2829
"time"
2930

@@ -44,6 +45,7 @@ import (
4445
utilfeature "k8s.io/apiserver/pkg/util/feature"
4546
"k8s.io/client-go/util/flowcontrol"
4647
featuregatetesting "k8s.io/component-base/featuregate/testing"
48+
"k8s.io/component-base/metrics/testutil"
4749
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
4850
apitest "k8s.io/cri-api/pkg/apis/testing"
4951
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
@@ -54,6 +56,7 @@ import (
5456
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
5557
containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
5658
imagetypes "k8s.io/kubernetes/pkg/kubelet/images"
59+
"k8s.io/kubernetes/pkg/kubelet/metrics"
5760
proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
5861
kubelettypes "k8s.io/kubernetes/pkg/kubelet/types"
5962
"k8s.io/utils/ptr"
@@ -3397,3 +3400,98 @@ func TestDoPodResizeAction(t *testing.T) {
33973400
})
33983401
}
33993402
}
3403+
3404+
func TestIncrementImageVolumeMetrics(t *testing.T) {
3405+
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ImageVolume, true)
3406+
metrics.Register()
3407+
3408+
testCases := map[string]struct {
3409+
err error
3410+
volumeMounts []v1.VolumeMount
3411+
imageVolumes kubecontainer.ImageVolumes
3412+
wants string
3413+
}{
3414+
"without error": {
3415+
volumeMounts: []v1.VolumeMount{
3416+
{Name: "mount1"},
3417+
{Name: "mount2"},
3418+
},
3419+
imageVolumes: kubecontainer.ImageVolumes{
3420+
"mount1": {Image: "image1"},
3421+
"mount2": {Image: "image2"},
3422+
"mount3": {Image: "image3"},
3423+
},
3424+
wants: `
3425+
# HELP kubelet_image_volume_mounted_errors_total [ALPHA] Number of failed image volume mounts.
3426+
# TYPE kubelet_image_volume_mounted_errors_total counter
3427+
kubelet_image_volume_mounted_errors_total 0
3428+
# HELP kubelet_image_volume_mounted_succeed_total [ALPHA] Number of successful image volume mounts.
3429+
# TYPE kubelet_image_volume_mounted_succeed_total counter
3430+
kubelet_image_volume_mounted_succeed_total 2
3431+
# HELP kubelet_image_volume_requested_total [ALPHA] Number of requested image volumes.
3432+
# TYPE kubelet_image_volume_requested_total counter
3433+
kubelet_image_volume_requested_total 3
3434+
`,
3435+
},
3436+
"with error": {
3437+
err: errors.New(""),
3438+
volumeMounts: []v1.VolumeMount{
3439+
{Name: "mount1"},
3440+
{Name: "mount2"},
3441+
},
3442+
imageVolumes: kubecontainer.ImageVolumes{
3443+
"mount1": {Image: "image1"},
3444+
"mount2": {Image: "image2"},
3445+
"mount3": {Image: "image3"},
3446+
},
3447+
wants: `
3448+
# HELP kubelet_image_volume_mounted_errors_total [ALPHA] Number of failed image volume mounts.
3449+
# TYPE kubelet_image_volume_mounted_errors_total counter
3450+
kubelet_image_volume_mounted_errors_total 2
3451+
# HELP kubelet_image_volume_mounted_succeed_total [ALPHA] Number of successful image volume mounts.
3452+
# TYPE kubelet_image_volume_mounted_succeed_total counter
3453+
kubelet_image_volume_mounted_succeed_total 0
3454+
# HELP kubelet_image_volume_requested_total [ALPHA] Number of requested image volumes.
3455+
# TYPE kubelet_image_volume_requested_total counter
3456+
kubelet_image_volume_requested_total 3
3457+
`,
3458+
},
3459+
"without used volume": {
3460+
volumeMounts: []v1.VolumeMount{
3461+
{Name: "mount1"},
3462+
},
3463+
imageVolumes: kubecontainer.ImageVolumes{},
3464+
wants: `
3465+
# HELP kubelet_image_volume_mounted_errors_total [ALPHA] Number of failed image volume mounts.
3466+
# TYPE kubelet_image_volume_mounted_errors_total counter
3467+
kubelet_image_volume_mounted_errors_total 0
3468+
# HELP kubelet_image_volume_mounted_succeed_total [ALPHA] Number of successful image volume mounts.
3469+
# TYPE kubelet_image_volume_mounted_succeed_total counter
3470+
kubelet_image_volume_mounted_succeed_total 0
3471+
# HELP kubelet_image_volume_requested_total [ALPHA] Number of requested image volumes.
3472+
# TYPE kubelet_image_volume_requested_total counter
3473+
kubelet_image_volume_requested_total 0
3474+
`,
3475+
},
3476+
}
3477+
3478+
// Run tests.
3479+
for name, tc := range testCases {
3480+
t.Run(name, func(t *testing.T) {
3481+
metrics.ImageVolumeRequestedTotal.Reset()
3482+
metrics.ImageVolumeMountedErrorsTotal.Reset()
3483+
metrics.ImageVolumeMountedSucceedTotal.Reset()
3484+
3485+
// Call the function.
3486+
incrementImageVolumeMetrics(tc.err, &v1.Container{VolumeMounts: tc.volumeMounts}, tc.imageVolumes)
3487+
3488+
if err := testutil.GatherAndCompare(metrics.GetGather(), strings.NewReader(tc.wants),
3489+
"kubelet_"+metrics.ImageVolumeRequestedTotalKey,
3490+
"kubelet_"+metrics.ImageVolumeMountedSucceedTotalKey,
3491+
"kubelet_"+metrics.ImageVolumeMountedErrorsTotalKey,
3492+
); err != nil {
3493+
t.Error(err)
3494+
}
3495+
})
3496+
}
3497+
}

‎pkg/kubelet/metrics/metrics.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,11 @@ const (
152152

153153
// Metrics to track kubelet admission rejections.
154154
AdmissionRejectionsTotalKey = "admission_rejections_total"
155+
156+
// Image Volume metrics
157+
ImageVolumeRequestedTotalKey = "image_volume_requested_total"
158+
ImageVolumeMountedSucceedTotalKey = "image_volume_mounted_succeed_total"
159+
ImageVolumeMountedErrorsTotalKey = "image_volume_mounted_errors_total"
155160
)
156161

157162
type imageSizeBucket struct {
@@ -1008,6 +1013,36 @@ var (
10081013
},
10091014
[]string{"reason"},
10101015
)
1016+
1017+
// ImageVolumeRequestedTotal trakcs the number of requested image volumes.
1018+
ImageVolumeRequestedTotal = metrics.NewCounter(
1019+
&metrics.CounterOpts{
1020+
Subsystem: KubeletSubsystem,
1021+
Name: ImageVolumeRequestedTotalKey,
1022+
Help: "Number of requested image volumes.",
1023+
StabilityLevel: metrics.ALPHA,
1024+
},
1025+
)
1026+
1027+
// ImageVolumeMountedSucceedTotal tracks the number of successful image volume mounts.
1028+
ImageVolumeMountedSucceedTotal = metrics.NewCounter(
1029+
&metrics.CounterOpts{
1030+
Subsystem: KubeletSubsystem,
1031+
Name: ImageVolumeMountedSucceedTotalKey,
1032+
Help: "Number of successful image volume mounts.",
1033+
StabilityLevel: metrics.ALPHA,
1034+
},
1035+
)
1036+
1037+
// ImageVolumeMountedErrorsTotal tracks the number of failed image volume mounts.
1038+
ImageVolumeMountedErrorsTotal = metrics.NewCounter(
1039+
&metrics.CounterOpts{
1040+
Subsystem: KubeletSubsystem,
1041+
Name: ImageVolumeMountedErrorsTotalKey,
1042+
Help: "Number of failed image volume mounts.",
1043+
StabilityLevel: metrics.ALPHA,
1044+
},
1045+
)
10111046
)
10121047

10131048
var registerMetrics sync.Once
@@ -1107,6 +1142,12 @@ func Register(collectors ...metrics.StableCollector) {
11071142
}
11081143

11091144
legacyregistry.MustRegister(AdmissionRejectionsTotal)
1145+
1146+
if utilfeature.DefaultFeatureGate.Enabled(features.ImageVolume) {
1147+
legacyregistry.MustRegister(ImageVolumeRequestedTotal)
1148+
legacyregistry.MustRegister(ImageVolumeMountedSucceedTotal)
1149+
legacyregistry.MustRegister(ImageVolumeMountedErrorsTotal)
1150+
}
11101151
})
11111152
}
11121153

0 commit comments

Comments
 (0)
0