From 0be5fdb5ced02f0ad9e3d343c6e10a918f4582ef Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 4 Oct 2023 14:38:01 +0200 Subject: [PATCH] Add volume plugin label to SELinux metrics Record volume plugin name when a volume in a Pod needs a different "mount -o context" value than the actually mounted one. We expect that NFS, CIFS and CephFS volumes would be able to mount such volumes just fine with multiple "-o context" values. We know that the block-volume based ones (ext4, xfs, btrfs, ...) cannot do that. Therefore want to distinguish the volume plugin in metrics, anything block-volume based could break an existing application. --- .../desired_state_of_wold_selinux_metrics.go | 18 +++++++---- .../cache/desired_state_of_world.go | 31 +++++++++++++++++-- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/pkg/kubelet/volumemanager/cache/desired_state_of_wold_selinux_metrics.go b/pkg/kubelet/volumemanager/cache/desired_state_of_wold_selinux_metrics.go index 16727d02591..4dd5ecd2eff 100644 --- a/pkg/kubelet/volumemanager/cache/desired_state_of_wold_selinux_metrics.go +++ b/pkg/kubelet/volumemanager/cache/desired_state_of_wold_selinux_metrics.go @@ -49,24 +49,30 @@ var ( Help: "Number of errors when a Pod defines different SELinux contexts for its containers that use the same volume. They are not errors yet, but they will become real errors when SELinuxMountReadWriteOncePod feature is expanded to all volume access modes.", StabilityLevel: compbasemetrics.ALPHA, }) - seLinuxVolumeContextMismatchErrors = compbasemetrics.NewGauge( + seLinuxVolumeContextMismatchErrors = compbasemetrics.NewGaugeVec( &compbasemetrics.GaugeOpts{ Name: "volume_manager_selinux_volume_context_mismatch_errors_total", Help: "Number of errors when a Pod uses a volume that is already mounted with a different SELinux context than the Pod needs. Kubelet can't start such a Pod then and it will retry, therefore value of this metric may not represent the actual nr. of Pods.", StabilityLevel: compbasemetrics.ALPHA, - }) - seLinuxVolumeContextMismatchWarnings = compbasemetrics.NewGauge( + }, + []string{"volume_plugin"}, + ) + seLinuxVolumeContextMismatchWarnings = compbasemetrics.NewGaugeVec( &compbasemetrics.GaugeOpts{ Name: "volume_manager_selinux_volume_context_mismatch_warnings_total", Help: "Number of errors when a Pod uses a volume that is already mounted with a different SELinux context than the Pod needs. They are not errors yet, but they will become real errors when SELinuxMountReadWriteOncePod feature is expanded to all volume access modes.", StabilityLevel: compbasemetrics.ALPHA, - }) - seLinuxVolumesAdmitted = compbasemetrics.NewGauge( + }, + []string{"volume_plugin"}, + ) + seLinuxVolumesAdmitted = compbasemetrics.NewGaugeVec( &compbasemetrics.GaugeOpts{ Name: "volume_manager_selinux_volumes_admitted_total", Help: "Number of volumes whose SELinux context was fine and will be mounted with mount -o context option.", StabilityLevel: compbasemetrics.ALPHA, - }) + }, + []string{"volume_plugin"}, + ) registerMetrics sync.Once ) diff --git a/pkg/kubelet/volumemanager/cache/desired_state_of_world.go b/pkg/kubelet/volumemanager/cache/desired_state_of_world.go index 2a7abe23c94..4406b941a04 100644 --- a/pkg/kubelet/volumemanager/cache/desired_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/desired_state_of_world.go @@ -31,6 +31,7 @@ import ( "k8s.io/apiserver/pkg/util/feature" "k8s.io/component-base/metrics" "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/volume/csi" resourcehelper "k8s.io/kubernetes/pkg/api/v1/resource" "k8s.io/kubernetes/pkg/features" @@ -273,6 +274,7 @@ func (dsw *desiredStateOfWorld) AddPodToVolume( volumeSpec.Name(), err) } + volumePluginName := getVolumePluginNameWithDriver(volumePlugin, volumeSpec) var volumeName v1.UniqueVolumeName @@ -326,7 +328,7 @@ func (dsw *desiredStateOfWorld) AddPodToVolume( effectiveSELinuxMountLabel = "" } if seLinuxFileLabel != "" { - seLinuxVolumesAdmitted.Add(1.0) + seLinuxVolumesAdmitted.WithLabelValues(volumePluginName).Add(1.0) } vmt := volumeToMount{ volumeName: volumeName, @@ -355,7 +357,12 @@ func (dsw *desiredStateOfWorld) AddPodToVolume( // TODO: update the error message after tests, e.g. add at least the conflicting pod names. fullErr := fmt.Errorf("conflicting SELinux labels of volume %s: %q and %q", volumeSpec.Name(), vol.originalSELinuxLabel, seLinuxFileLabel) supported := util.VolumeSupportsSELinuxMount(volumeSpec) - if err := handleSELinuxMetricError(fullErr, supported, seLinuxVolumeContextMismatchWarnings, seLinuxVolumeContextMismatchErrors); err != nil { + err := handleSELinuxMetricError( + fullErr, + supported, + seLinuxVolumeContextMismatchWarnings.WithLabelValues(volumePluginName), + seLinuxVolumeContextMismatchErrors.WithLabelValues(volumePluginName)) + if err != nil { return "", err } } @@ -646,7 +653,7 @@ func (dsw *desiredStateOfWorld) getSELinuxMountSupport(volumeSpec *volume.Spec) } // Based on isRWOP, bump the right warning / error metric and either consume the error or return it. -func handleSELinuxMetricError(err error, seLinuxSupported bool, warningMetric, errorMetric *metrics.Gauge) error { +func handleSELinuxMetricError(err error, seLinuxSupported bool, warningMetric, errorMetric metrics.GaugeMetric) error { if seLinuxSupported { errorMetric.Add(1.0) return err @@ -657,3 +664,21 @@ func handleSELinuxMetricError(err error, seLinuxSupported bool, warningMetric, e klog.V(4).ErrorS(err, "Please report this error in https://github.com/kubernetes/enhancements/issues/1710, together with full Pod yaml file") return nil } + +// Return the volume plugin name, together with the CSI driver name if it's a CSI volume. +func getVolumePluginNameWithDriver(plugin volume.VolumePlugin, spec *volume.Spec) string { + pluginName := plugin.GetPluginName() + if pluginName != csi.CSIPluginName { + return pluginName + } + + // It's a CSI volume + driverName, err := csi.GetCSIDriverName(spec) + if err != nil { + // In theory this is unreachable - such volume would not pass validation. + klog.V(4).ErrorS(err, "failed to get CSI driver name from volume spec") + driverName = "unknown" + } + // `/` is used to separate plugin + CSI driver in util.GetUniqueVolumeName() too + return pluginName + "/" + driverName +}