Skip to content

OCPBUGS-65617: fix(conformance): set priorityClass on global-pull-secret-syncer DaemonSet #7236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Nov 15, 2025
Merged

OCPBUGS-65617: fix(conformance): set priorityClass on global-pull-secret-syncer DaemonSet #7236

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ import (
)

const (
ControllerName = "globalps"
configSeedLabelKey = "hypershift.openshift.io/globalps-config-hash"
globalPSLabelKey = "hypershift.openshift.io/nodepool-globalps-enabled"
ControllerName = "globalps"
configSeedLabelKey = "hypershift.openshift.io/globalps-config-hash"
globalPSLabelKey = "hypershift.openshift.io/nodepool-globalps-enabled"
openshiftUserCriticalPriorityClass = "openshift-user-critical"
)

// GlobalPullSecretPodConfig encapsulates the configuration for GlobalPullSecret DaemonSet pods
Expand Down Expand Up @@ -286,6 +287,7 @@ func reconcileDaemonSet(ctx context.Context, daemonSet *appsv1.DaemonSet, global
AutomountServiceAccountToken: ptr.To(false),
SecurityContext: &corev1.PodSecurityContext{},
DNSPolicy: corev1.DNSDefault,
PriorityClassName: openshiftUserCriticalPriorityClass,

@coderabbitai coderabbitai Bot Nov 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Verify the priority class choice is appropriate for this system-level DaemonSet.

The assignment correctly sets a priority class, which addresses the conformance test failure. However, since this DaemonSet performs privileged system operations (writes to kubelet config, restarts kubelet service), consider verifying that openshift-user-critical is the intended priority class rather than system-node-critical, which is typically used for node-level critical infrastructure.

🌐 Web query:

What is the difference between openshift-user-critical and system-node-critical priority classes in OpenShift, and which should be used for DaemonSets that manage kubelet configuration?

💡 Result:

Short answer: use system-node-critical for DaemonSets that manage kubelet configuration.

Why:

  • system-node-critical is reserved for pods that should never be evicted and protect node-level services. OpenShift lists node-essential components under system-node-critical. [1][2]
  • openshift-user-critical is intended for important user/monitoring workloads (Prometheus, user-workload monitoring) that intentionally can drop priority when they over-consume resources so the scheduler has flexibility. It is not for node-level control-plane/agent DaemonSets. [2]
  • Kubernetes/OpenShift priority semantics: higher priority prevents preemption/eviction for critical node services; node-management/ kubelet-affecting DaemonSets belong to the node-critical class. [3]

Citations:

  • OpenShift docs — priority classes (system-node-critical, system-cluster-critical, openshift-user-critical). [1][2]
  • Kubernetes Pod Priority & Preemption overview. [3]

Use system-node-critical priority class for this DaemonSet.

The system-node-critical priority class is reserved for pods that should never be evicted and protect node-level services, while openshift-user-critical is intended for important user/monitoring workloads that can intentionally drop priority when over-consuming resources. Since this DaemonSet manages kubelet configuration (a node-critical operation), it should use system-node-critical rather than openshift-user-critical.

Change openshiftUserCriticalPriorityClass to systemNodeCriticalPriorityClass at line 290.

🤖 Prompt for AI Agents 
In
control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go
around line 290, the DaemonSet currently sets PriorityClassName to
openshiftUserCriticalPriorityClass but must use systemNodeCriticalPriorityClass
because this manages kubelet/node-critical functionality; update the
PriorityClassName value from openshiftUserCriticalPriorityClass to
systemNodeCriticalPriorityClass and ensure the systemNodeCriticalPriorityClass
constant (or equivalent string "system-node-critical") is defined/available in
scope and used consistently.

Tolerations: []corev1.Toleration{{Operator: corev1.TolerationOpExists}},
// Use nodeSelector to only include nodes that are explicitly enabled for GlobalPullSecret
NodeSelector: map[string]string{
Expand Down