From 03a94e512ff77aaaba96efecb4b118ef1ae8efc5 Mon Sep 17 00:00:00 2001 From: Mihai IDU Date: Wed, 22 Jul 2026 18:50:23 +0200 Subject: [PATCH] RFE-9311: Add workload partitioning support for must-gather pods When CPU partitioning is enabled (cpuPartitioningMode: AllNodes), must-gather pods now carry the workload partitioning annotation so CRI-O pins them to Reserved CPU cores, preventing interference with Isolated application workloads. Changes: - Query Infrastructure resource to detect CPU partitioning mode - Annotate temp namespace with workload.openshift.io/allowed - Annotate pods with target.workload.openshift.io/management - Fail open if Infrastructure resource is unavailable --- pkg/cli/admin/mustgather/mustgather.go | 34 ++++++- pkg/cli/admin/mustgather/mustgather_test.go | 104 ++++++++++++++++++++ 2 files changed, 135 insertions(+), 3 deletions(-) diff --git a/pkg/cli/admin/mustgather/mustgather.go b/pkg/cli/admin/mustgather/mustgather.go index 64af976948..144c6af7d4 100644 --- a/pkg/cli/admin/mustgather/mustgather.go +++ b/pkg/cli/admin/mustgather/mustgather.go @@ -45,6 +45,7 @@ import ( "k8s.io/utils/exec" utilptr "k8s.io/utils/ptr" + configv1 "github.com/openshift/api/config/v1" configclient "github.com/openshift/client-go/config/clientset/versioned" imagev1client "github.com/openshift/client-go/image/clientset/versioned/typed/image/v1" "github.com/openshift/library-go/pkg/image/imageutil" @@ -63,6 +64,10 @@ const ( defaultMustGatherCommand = "/usr/bin/gather" defaultVolumePercentage = 70 defaultSourceDir = "/must-gather/" + + workloadAnnotationKey = "target.workload.openshift.io/management" + workloadAnnotationValue = `{"effect": "PreferredDuringScheduling"}` + namespaceWorkloadAllowedAnnotation = "workload.openshift.io/allowed" ) var ( @@ -314,6 +319,15 @@ func (o *MustGatherOptions) getClusterIDSuffix() string { return id } +func (o *MustGatherOptions) isCPUPartitioningEnabled(ctx context.Context) bool { + infra, err := o.ConfigClient.ConfigV1().Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + klog.V(4).Infof("unable to determine CPU partitioning mode, proceeding without workload annotations: %v", err) + return false + } + return infra.Status.CPUPartitioning == configv1.CPUPartitioningAllNodes +} + func (o *MustGatherOptions) completeImages(ctx context.Context) error { for _, imageStream := range o.ImageStreams { if image, err := o.resolveImageStreamTagString(ctx, imageStream); err == nil { @@ -453,6 +467,8 @@ type MustGatherOptions struct { LogWriter *os.File LogWriterMux sync.Mutex + + cpuPartitioningEnabled bool } func (o *MustGatherOptions) Validate() error { @@ -674,6 +690,8 @@ func (o *MustGatherOptions) Run() error { o.BackupGathering(ctx, errs) }() + o.cpuPartitioningEnabled = o.isCPUPartitioningEnabled(ctx) + // Get or create "working" namespace ... var ns *corev1.Namespace ns, cleanupNamespace, err = o.getNamespace(ctx) @@ -1083,7 +1101,7 @@ func (o *MustGatherOptions) getNamespace(ctx context.Context) (*corev1.Namespace } func (o *MustGatherOptions) createTempNamespace(ctx context.Context) (*corev1.Namespace, func(), error) { - ns, err := o.Client.CoreV1().Namespaces().Create(ctx, newNamespace(), metav1.CreateOptions{}) + ns, err := o.Client.CoreV1().Namespaces().Create(ctx, newNamespace(o.cpuPartitioningEnabled), metav1.CreateOptions{}) if err != nil { return nil, nil, fmt.Errorf("creating temp namespace: %w", err) } @@ -1106,8 +1124,8 @@ func (o *MustGatherOptions) createTempNamespace(ctx context.Context) (*corev1.Na return ns, cleanup, nil } -func newNamespace() *corev1.Namespace { - return &corev1.Namespace{ +func newNamespace(cpuPartitioning bool) *corev1.Namespace { + ns := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "openshift-must-gather-", Labels: map[string]string{ @@ -1123,6 +1141,10 @@ func newNamespace() *corev1.Namespace { }, }, } + if cpuPartitioning { + ns.Annotations[namespaceWorkloadAllowedAnnotation] = "management" + } + return ns } func newClusterRoleBinding(ns *corev1.Namespace) *rbacv1.ClusterRoleBinding { @@ -1262,6 +1284,12 @@ func (o *MustGatherOptions) newPod(node, image string, hasMaster bool, affinity podCmd := buildPodCommand(volumeUsageChecker, executedCommand) ret := defaultMustGatherPod(image) + if o.cpuPartitioningEnabled { + if ret.Annotations == nil { + ret.Annotations = make(map[string]string) + } + ret.Annotations[workloadAnnotationKey] = workloadAnnotationValue + } ret.Spec.Containers[0].Command = []string{"/bin/bash", "-c", podCmd} ret.Spec.Containers[0].VolumeMounts = []corev1.VolumeMount{{ Name: "must-gather-output", diff --git a/pkg/cli/admin/mustgather/mustgather_test.go b/pkg/cli/admin/mustgather/mustgather_test.go index 8fb6fdba37..c0c1fc303d 100644 --- a/pkg/cli/admin/mustgather/mustgather_test.go +++ b/pkg/cli/admin/mustgather/mustgather_test.go @@ -688,6 +688,28 @@ func TestNewPod(t *testing.T) { }) }), }, + { + name: "with CPU partitioning enabled", + options: &MustGatherOptions{ + VolumePercentage: defaultVolumePercentage, + SourceDir: defaultSourceDir, + cpuPartitioningEnabled: true, + }, + expectedPod: generateMustGatherPod("image", func(pod *corev1.Pod) { + pod.Annotations = map[string]string{ + workloadAnnotationKey: workloadAnnotationValue, + } + }), + }, + { + name: "without CPU partitioning", + options: &MustGatherOptions{ + VolumePercentage: defaultVolumePercentage, + SourceDir: defaultSourceDir, + cpuPartitioningEnabled: false, + }, + expectedPod: generateMustGatherPod("image", func(pod *corev1.Pod) {}), + }, } for _, test := range tests { @@ -699,3 +721,85 @@ func TestNewPod(t *testing.T) { }) } } + +func TestIsCPUPartitioningEnabled(t *testing.T) { + tests := []struct { + name string + infra *configv1.Infrastructure + expected bool + }{ + { + name: "AllNodes mode returns true", + infra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + CPUPartitioning: configv1.CPUPartitioningAllNodes, + }, + }, + expected: true, + }, + { + name: "None mode returns false", + infra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + CPUPartitioning: configv1.CPUPartitioningNone, + }, + }, + expected: false, + }, + { + name: "missing Infrastructure resource returns false", + expected: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var objs []runtime.Object + if tc.infra != nil { + objs = append(objs, tc.infra) + } + o := &MustGatherOptions{ + ConfigClient: configv1fake.NewSimpleClientset(objs...), + } + got := o.isCPUPartitioningEnabled(context.TODO()) + if got != tc.expected { + t.Errorf("expected %v, got %v", tc.expected, got) + } + }) + } +} + +func TestNewNamespaceWorkloadAnnotation(t *testing.T) { + tests := []struct { + name string + cpuPartitioning bool + expectAnnotation bool + }{ + { + name: "with CPU partitioning adds workload annotation", + cpuPartitioning: true, + expectAnnotation: true, + }, + { + name: "without CPU partitioning omits workload annotation", + cpuPartitioning: false, + expectAnnotation: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ns := newNamespace(tc.cpuPartitioning) + _, exists := ns.Annotations[namespaceWorkloadAllowedAnnotation] + if tc.expectAnnotation && !exists { + t.Errorf("expected %s annotation, got annotations: %v", namespaceWorkloadAllowedAnnotation, ns.Annotations) + } + if !tc.expectAnnotation && exists { + t.Errorf("did not expect %s annotation when partitioning is disabled", namespaceWorkloadAllowedAnnotation) + } + if tc.expectAnnotation && ns.Annotations[namespaceWorkloadAllowedAnnotation] != "management" { + t.Errorf("expected annotation value %q, got %q", "management", ns.Annotations[namespaceWorkloadAllowedAnnotation]) + } + }) + } +}