mirror of
https://github.com/kubernetes/kubernetes.git
synced 2026-06-08 16:30:57 -04:00
Improve performance characteristic of selinux metric emission
This commit is contained in:
Hemant Kumar
parent
4cfd7e74b1
commit
9047f71b70
4 changed files with 227 additions and 59 deletions
|
|
@ -48,8 +48,8 @@ type VolumeCache interface {
|
|||
// change their SELinux support dynamically.
|
||||
GetPodsForCSIDriver(driverName string) []cache.ObjectName
|
||||
|
||||
// SendConflicts sends all current conflicts to the given channel.
|
||||
SendConflicts(logger klog.Logger, ch chan<- Conflict)
|
||||
// GetConflicts returns the current set of active conflicts (both directions).
|
||||
GetConflicts(logger klog.Logger) []Conflict
|
||||
}
|
||||
|
||||
// VolumeCache stores all volumes used by Pods and their properties that the controller needs to track,
|
||||
|
|
@ -62,6 +62,8 @@ type volumeCache struct {
|
|||
// Reverse index: maps each pod to the list of volumes it uses.
|
||||
// The index is used during pod deletion.
|
||||
podToVolumes map[cache.ObjectName]sets.Set[v1.UniqueVolumeName]
|
||||
// Currently active conflicts per volume (both directions, symmetric pairs).
|
||||
conflicts map[v1.UniqueVolumeName][]Conflict
|
||||
}
|
||||
|
||||
var _ VolumeCache = &volumeCache{}
|
||||
|
|
@ -72,6 +74,7 @@ func NewVolumeLabelCache(seLinuxTranslator *translator.ControllerSELinuxTranslat
|
|||
seLinuxTranslator: seLinuxTranslator,
|
||||
volumes: make(map[v1.UniqueVolumeName]usedVolume),
|
||||
podToVolumes: make(map[cache.ObjectName]sets.Set[v1.UniqueVolumeName]),
|
||||
conflicts: make(map[v1.UniqueVolumeName][]Conflict),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -164,6 +167,7 @@ func (c *volumeCache) AddVolume(logger klog.Logger, volumeName v1.UniqueVolumeNa
|
|||
OtherPod: podKey,
|
||||
OtherPropertyValue: string(changePolicy),
|
||||
})
|
||||
|
||||
}
|
||||
if c.seLinuxTranslator.ConflictsParsed(otherPodInfo.seLinuxParts, podInfo.seLinuxParts) {
|
||||
// Send conflict to both pods
|
||||
|
|
@ -184,6 +188,21 @@ func (c *volumeCache) AddVolume(logger klog.Logger, volumeName v1.UniqueVolumeNa
|
|||
})
|
||||
}
|
||||
}
|
||||
// Update the conflict cache for this volume: remove stale conflicts for this pod, then add new ones
|
||||
volumeConflicts := c.conflicts[volumeName]
|
||||
updated := make([]Conflict, 0, len(volumeConflicts))
|
||||
for _, existing := range volumeConflicts {
|
||||
if existing.Pod != podKey && existing.OtherPod != podKey {
|
||||
updated = append(updated, existing)
|
||||
}
|
||||
}
|
||||
updated = append(updated, conflicts...)
|
||||
if len(updated) == 0 {
|
||||
delete(c.conflicts, volumeName)
|
||||
} else {
|
||||
c.conflicts[volumeName] = updated
|
||||
}
|
||||
|
||||
return conflicts
|
||||
}
|
||||
|
||||
|
|
@ -205,6 +224,21 @@ func (c *volumeCache) DeletePod(logger klog.Logger, podKey cache.ObjectName) {
|
|||
}
|
||||
}
|
||||
delete(c.podToVolumes, podKey)
|
||||
|
||||
// Remove cached conflicts involving the deleted pod
|
||||
for volName, volConflicts := range c.conflicts {
|
||||
updated := make([]Conflict, 0, len(volConflicts))
|
||||
for _, existing := range volConflicts {
|
||||
if existing.Pod != podKey && existing.OtherPod != podKey {
|
||||
updated = append(updated, existing)
|
||||
}
|
||||
}
|
||||
if len(updated) == 0 {
|
||||
delete(c.conflicts, volName)
|
||||
} else {
|
||||
c.conflicts[volName] = updated
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// registerPodVolume adds volumeName to the pod volume index.
|
||||
|
|
@ -281,42 +315,16 @@ func (c *volumeCache) GetPodsForCSIDriver(driverName string) []cache.ObjectName
|
|||
return pods
|
||||
}
|
||||
|
||||
// SendConflicts sends all current conflicts to the given channel.
|
||||
func (c *volumeCache) SendConflicts(logger klog.Logger, ch chan<- Conflict) {
|
||||
// GetConflicts returns the current set of active conflicts (both directions, symmetric pairs).
|
||||
func (c *volumeCache) GetConflicts(logger klog.Logger) []Conflict {
|
||||
c.mutex.RLock()
|
||||
defer c.mutex.RUnlock()
|
||||
logger.V(4).Info("Scraping conflicts")
|
||||
c.dump(logger)
|
||||
|
||||
for _, volume := range c.volumes {
|
||||
// compare pods that use the same volume with each other
|
||||
for podKey, podInfo := range volume.pods {
|
||||
for otherPodKey, otherPodInfo := range volume.pods {
|
||||
if podKey == otherPodKey {
|
||||
continue
|
||||
}
|
||||
// create conflict only for the first pod. The other pod will get the same conflict in its own iteration of `volume.pods` loop.
|
||||
if podInfo.changePolicy != otherPodInfo.changePolicy {
|
||||
ch <- Conflict{
|
||||
PropertyName: "SELinuxChangePolicy",
|
||||
EventReason: "SELinuxChangePolicyConflict",
|
||||
Pod: podKey,
|
||||
PropertyValue: string(podInfo.changePolicy),
|
||||
OtherPod: otherPodKey,
|
||||
OtherPropertyValue: string(otherPodInfo.changePolicy),
|
||||
}
|
||||
}
|
||||
if c.seLinuxTranslator.Conflicts(podInfo.seLinuxLabel, otherPodInfo.seLinuxLabel) {
|
||||
ch <- Conflict{
|
||||
PropertyName: "SELinuxLabel",
|
||||
EventReason: "SELinuxLabelConflict",
|
||||
Pod: podKey,
|
||||
PropertyValue: podInfo.seLinuxLabel,
|
||||
OtherPod: otherPodKey,
|
||||
OtherPropertyValue: otherPodInfo.seLinuxLabel,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
result := make([]Conflict, 0)
|
||||
for _, volConflicts := range c.conflicts {
|
||||
result = append(result, volConflicts...)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
|
|
|||
|
|
@ -147,8 +147,8 @@ func addReverseConflict(conflicts []Conflict) []Conflict {
|
|||
return newConflicts
|
||||
}
|
||||
|
||||
// Test AddVolume and SendConflicts together, they both provide []conflict with the same data
|
||||
func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
|
||||
// Test that AddVolume and GetConflicts return the same []conflict data
|
||||
func TestVolumeCache_AddVolumeGetConflicts(t *testing.T) {
|
||||
existingPods := []podWithVolume{
|
||||
{
|
||||
podNamespace: "ns1",
|
||||
|
|
@ -488,18 +488,8 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
|
|||
// Verify reverse index consistency
|
||||
verifyReverseIndexConsistency(t, c)
|
||||
|
||||
// Act again: get the conflicts via SendConflicts
|
||||
ch := make(chan Conflict)
|
||||
go func() {
|
||||
c.SendConflicts(logger, ch)
|
||||
close(ch)
|
||||
}()
|
||||
|
||||
// Assert
|
||||
receivedConflicts := []Conflict{}
|
||||
for c := range ch {
|
||||
receivedConflicts = append(receivedConflicts, c)
|
||||
}
|
||||
// Verify that GetConflicts returns the same conflicts
|
||||
receivedConflicts := c.GetConflicts(logger)
|
||||
sortConflicts(receivedConflicts)
|
||||
if !reflect.DeepEqual(receivedConflicts, expectedConflicts) {
|
||||
t.Errorf("SendConflicts returned unexpected conflicts: %+v", receivedConflicts)
|
||||
|
|
@ -509,6 +499,182 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// Test that conflicts are tracked per-volume: a pod with conflicts on
|
||||
// multiple volumes retains all of them after successive AddVolume calls.
|
||||
func TestVolumeCache_MultiVolumeConflicts(t *testing.T) {
|
||||
logger, _ := getTestLoggers(t)
|
||||
seLinuxTranslator := &translator.ControllerSELinuxTranslator{}
|
||||
c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache)
|
||||
|
||||
podA := cache.ObjectName{Namespace: "ns", Name: "podA"}
|
||||
podB := cache.ObjectName{Namespace: "ns", Name: "podB"}
|
||||
podC := cache.ObjectName{Namespace: "ns", Name: "podC"}
|
||||
|
||||
// podB uses vol1 with label1
|
||||
c.AddVolume(logger, "vol1", podB, "system_u:system_r:labelB", v1.SELinuxChangePolicyMountOption, "driver1")
|
||||
// podC uses vol2 with label2
|
||||
c.AddVolume(logger, "vol2", podC, "system_u:system_r:labelC", v1.SELinuxChangePolicyMountOption, "driver1")
|
||||
|
||||
// podA uses vol1 with a different label (conflict with podB)
|
||||
conflicts1 := c.AddVolume(logger, "vol1", podA, "system_u:system_r:labelA", v1.SELinuxChangePolicyMountOption, "driver1")
|
||||
if len(conflicts1) == 0 {
|
||||
t.Fatal("Expected conflicts on vol1 between podA and podB")
|
||||
}
|
||||
|
||||
// podA also uses vol2 with a different label (conflict with podC)
|
||||
conflicts2 := c.AddVolume(logger, "vol2", podA, "system_u:system_r:labelA", v1.SELinuxChangePolicyMountOption, "driver1")
|
||||
if len(conflicts2) == 0 {
|
||||
t.Fatal("Expected conflicts on vol2 between podA and podC")
|
||||
}
|
||||
|
||||
// GetConflicts must return conflicts from BOTH volumes
|
||||
allConflicts := c.GetConflicts(logger)
|
||||
expectedCount := len(conflicts1) + len(conflicts2)
|
||||
if len(allConflicts) != expectedCount {
|
||||
t.Errorf("GetConflicts returned %d conflicts, expected %d (vol1: %d + vol2: %d)",
|
||||
len(allConflicts), expectedCount, len(conflicts1), len(conflicts2))
|
||||
}
|
||||
|
||||
// After deleting podA, all conflicts should be gone
|
||||
c.DeletePod(logger, podA)
|
||||
remaining := c.GetConflicts(logger)
|
||||
if len(remaining) != 0 {
|
||||
t.Errorf("Expected no conflicts after deleting podA, got %d: %+v", len(remaining), remaining)
|
||||
}
|
||||
}
|
||||
|
||||
func TestVolumeCache_DeletePodConflicts(t *testing.T) {
|
||||
podA := cache.ObjectName{Namespace: "ns", Name: "podA"}
|
||||
podB := cache.ObjectName{Namespace: "ns", Name: "podB"}
|
||||
podC := cache.ObjectName{Namespace: "ns", Name: "podC"}
|
||||
podD := cache.ObjectName{Namespace: "ns", Name: "podD"}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
// Pods to add before deletion.
|
||||
initialPods []podWithVolume
|
||||
// Pod to delete.
|
||||
podToDelete cache.ObjectName
|
||||
// If true, delete the pod a second time to verify idempotency.
|
||||
deleteTwice bool
|
||||
// Pod pairs that must still have symmetric conflicts after deletion.
|
||||
// Each pair [2]cache.ObjectName expects both (A→B) and (B→A) to be present.
|
||||
expectedSurvivingPairs [][2]cache.ObjectName
|
||||
}{
|
||||
{
|
||||
name: "delete one of two conflicting pods clears all conflicts",
|
||||
initialPods: []podWithVolume{
|
||||
{podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
},
|
||||
podToDelete: podA,
|
||||
expectedSurvivingPairs: nil,
|
||||
},
|
||||
{
|
||||
name: "delete non-conflicting pod preserves existing conflicts",
|
||||
initialPods: []podWithVolume{
|
||||
{podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podC", volumeName: "vol2", label: "system_u:system_r:labelC", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
},
|
||||
podToDelete: podC,
|
||||
expectedSurvivingPairs: [][2]cache.ObjectName{{podA, podB}},
|
||||
},
|
||||
{
|
||||
name: "three pods on same volume delete one leaves remaining pair conflict",
|
||||
initialPods: []podWithVolume{
|
||||
{podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podC", volumeName: "vol1", label: "system_u:system_r:labelC", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
},
|
||||
podToDelete: podA,
|
||||
expectedSurvivingPairs: [][2]cache.ObjectName{{podB, podC}},
|
||||
},
|
||||
{
|
||||
name: "delete pod with conflicts on multiple volumes",
|
||||
initialPods: []podWithVolume{
|
||||
{podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podC", volumeName: "vol2", label: "system_u:system_r:labelC", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podA", volumeName: "vol2", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
},
|
||||
podToDelete: podA,
|
||||
expectedSurvivingPairs: nil,
|
||||
},
|
||||
{
|
||||
name: "delete pod preserves conflicts on unrelated volumes",
|
||||
initialPods: []podWithVolume{
|
||||
{podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podC", volumeName: "vol2", label: "system_u:system_r:labelC", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podD", volumeName: "vol2", label: "system_u:system_r:labelD", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
},
|
||||
podToDelete: podA,
|
||||
expectedSurvivingPairs: [][2]cache.ObjectName{{podC, podD}},
|
||||
},
|
||||
{
|
||||
name: "delete pod that was already deleted is a no-op",
|
||||
initialPods: []podWithVolume{
|
||||
{podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
{podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption},
|
||||
},
|
||||
podToDelete: podA,
|
||||
deleteTwice: true,
|
||||
expectedSurvivingPairs: nil,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
logger, _ := getTestLoggers(t)
|
||||
seLinuxTranslator := &translator.ControllerSELinuxTranslator{}
|
||||
c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache)
|
||||
|
||||
for _, pod := range tt.initialPods {
|
||||
c.AddVolume(logger, pod.volumeName, cache.ObjectName{Namespace: pod.podNamespace, Name: pod.podName}, pod.label, pod.changePolicy, "driver1")
|
||||
}
|
||||
|
||||
c.DeletePod(logger, tt.podToDelete)
|
||||
if tt.deleteTwice {
|
||||
c.DeletePod(logger, tt.podToDelete)
|
||||
}
|
||||
|
||||
remaining := c.GetConflicts(logger)
|
||||
|
||||
// Deleted pod must not appear in any conflict
|
||||
for _, conflict := range remaining {
|
||||
if conflict.Pod == tt.podToDelete || conflict.OtherPod == tt.podToDelete {
|
||||
t.Errorf("found conflict involving deleted pod %s: %+v", tt.podToDelete, conflict)
|
||||
}
|
||||
}
|
||||
|
||||
// Verify each expected surviving pair exists in both directions
|
||||
for _, pair := range tt.expectedSurvivingPairs {
|
||||
hasForward := false
|
||||
hasReverse := false
|
||||
for _, conflict := range remaining {
|
||||
if conflict.Pod == pair[0] && conflict.OtherPod == pair[1] {
|
||||
hasForward = true
|
||||
}
|
||||
if conflict.Pod == pair[1] && conflict.OtherPod == pair[0] {
|
||||
hasReverse = true
|
||||
}
|
||||
}
|
||||
if !hasForward || !hasReverse {
|
||||
t.Errorf("expected symmetric conflict between %s and %s, got %+v", pair[0], pair[1], remaining)
|
||||
}
|
||||
}
|
||||
|
||||
// If no pairs are expected, there should be no conflicts at all
|
||||
if len(tt.expectedSurvivingPairs) == 0 && len(remaining) != 0 {
|
||||
t.Errorf("expected no conflicts, got %+v", remaining)
|
||||
}
|
||||
|
||||
verifyReverseIndexConsistency(t, c)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestVolumeCache_GetPodsForCSIDriver(t *testing.T) {
|
||||
seLinuxTranslator := &translator.ControllerSELinuxTranslator{}
|
||||
c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache)
|
||||
|
|
|
|||
|
|
@ -59,13 +59,7 @@ func (c *collector) DescribeWithStability(ch chan<- *metrics.Desc) {
|
|||
}
|
||||
|
||||
func (c *collector) CollectWithStability(ch chan<- metrics.Metric) {
|
||||
conflictCh := make(chan cache.Conflict)
|
||||
go func() {
|
||||
c.cache.SendConflicts(c.logger, conflictCh)
|
||||
close(conflictCh)
|
||||
}()
|
||||
|
||||
for conflict := range conflictCh {
|
||||
for _, conflict := range c.cache.GetConflicts(c.logger) {
|
||||
ch <- metrics.NewLazyConstMetric(seLinuxConflictDesc,
|
||||
metrics.GaugeValue,
|
||||
1.0,
|
||||
|
|
|
|||
|
|
@ -778,12 +778,12 @@ func (f *fakeVolumeCache) GetPodsForCSIDriver(driverName string) []cache.ObjectN
|
|||
return pods
|
||||
}
|
||||
|
||||
func (f *fakeVolumeCache) SendConflicts(logger klog.Logger, ch chan<- volumecache.Conflict) {
|
||||
func (f *fakeVolumeCache) GetConflicts(logger klog.Logger) []volumecache.Conflict {
|
||||
result := make([]volumecache.Conflict, 0)
|
||||
for _, conflicts := range f.conflictsToSend {
|
||||
for _, conflict := range conflicts {
|
||||
ch <- conflict
|
||||
}
|
||||
result = append(result, conflicts...)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
func collectEvents(source <-chan string) []string {
|
||||
|
|
|
|||
Loading…
Reference in a new issue