Skip to content
This repository was archived by the owner on Nov 9, 2022. It is now read-only.
This repository is currently being migrated. It's locked while the migration is in progress.

Set priority class for helper pods #333

Merged
merged 1 commit into from
May 12, 2021
Merged

Set priority class for helper pods #333

Show file tree
Hide file tree
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
13 changes: 0 additions & 13 deletions pkg/storageos/csi_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,6 @@ func (s Deployment) createCSIHelperDeployment(replicas int32) error {
return s.k8sResourceManager.Deployment(csiHelperName, s.stos.Spec.GetResourceNS(), nil, spec).Create()
}

// addCommonPodProperties adds common pod properties to a given pod spec. The
// common pod properties are common for all the pods that are part of storageos
// deployment, including the CSI helpers pod.
func (s Deployment) addCommonPodProperties(podSpec *corev1.PodSpec) error {
s.addNodeAffinity(podSpec)

// Add helper tolerations.
if err := s.addHelperTolerations(podSpec, podTolerationSeconds); err != nil {
return err
}
return nil
}

// csiHelperContainers returns a list of containers that should be part of the
// CSI helper pods.
func (s Deployment) csiHelperContainers() ([]corev1.Container, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/storageos/daemonset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func (s *Deployment) createDaemonSet() error {
podSpec := &spec.Template.Spec
nodeContainer := &podSpec.Containers[0]

s.addPodPriorityClass(podSpec)
s.addPodPriorityClass(podSpec, nodeCriticalPriorityClass)

s.addTLSEtcdCerts(podSpec)

Expand Down
75 changes: 43 additions & 32 deletions pkg/storageos/deploy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1606,34 +1606,49 @@ func TestDeployTLSEtcdCerts(t *testing.T) {
// for the daemonset when deployed in kube-system namespace.
func TestDeployPodPriorityClass(t *testing.T) {
testCases := []struct {
name string
resourceNS string
csiDeploymentStrategy string
wantPriorityClass bool
name string
resourceNS string
csiDeploymentStrategy string
wantDaemonSetPriorityClass string
wantCSIHelperPriorityClass string
wantAPIManagerPriorityClass string
wantSchedulerPriorityClass string
}{
{
name: "have priority class set | CSI Deployment",
resourceNS: "kube-system",
csiDeploymentStrategy: "deployment",
wantPriorityClass: true,
name: "have priority class set in kube-system ns | CSI Deployment",
resourceNS: "kube-system",
csiDeploymentStrategy: "deployment",
wantDaemonSetPriorityClass: nodeCriticalPriorityClass,
wantCSIHelperPriorityClass: clusterCriticalPriorityClass,
wantAPIManagerPriorityClass: clusterCriticalPriorityClass,
wantSchedulerPriorityClass: clusterCriticalPriorityClass,
},
{
name: "have priority class set | CSI StatefulSet",
resourceNS: "kube-system",
csiDeploymentStrategy: "statefulset",
wantPriorityClass: true,
name: "have priority class set in kube-system ns | CSI StatefulSet",
resourceNS: "kube-system",
csiDeploymentStrategy: "statefulset",
wantDaemonSetPriorityClass: nodeCriticalPriorityClass,
wantCSIHelperPriorityClass: clusterCriticalPriorityClass,
wantAPIManagerPriorityClass: clusterCriticalPriorityClass,
wantSchedulerPriorityClass: clusterCriticalPriorityClass,
},
{
name: "no priority class set | CSI Deployment",
resourceNS: "storageos",
csiDeploymentStrategy: "deployment",
wantPriorityClass: false,
name: "no priority class set in storageos ns | CSI Deployment",
resourceNS: "storageos",
csiDeploymentStrategy: "deployment",
wantDaemonSetPriorityClass: nodeCriticalPriorityClass,
wantCSIHelperPriorityClass: clusterCriticalPriorityClass,
wantAPIManagerPriorityClass: clusterCriticalPriorityClass,
wantSchedulerPriorityClass: clusterCriticalPriorityClass,
},
{
name: "no priority class set | CSI StatefulSet",
resourceNS: "storageos",
csiDeploymentStrategy: "statefulset",
wantPriorityClass: false,
name: "no priority class set in storageos ns | CSI StatefulSet",
resourceNS: "storageos",
csiDeploymentStrategy: "statefulset",
wantDaemonSetPriorityClass: nodeCriticalPriorityClass,
wantCSIHelperPriorityClass: clusterCriticalPriorityClass,
wantAPIManagerPriorityClass: clusterCriticalPriorityClass,
wantSchedulerPriorityClass: clusterCriticalPriorityClass,
},
}

Expand Down Expand Up @@ -1684,12 +1699,8 @@ func TestDeployPodPriorityClass(t *testing.T) {
}

daemonsetPC := createdDaemonset.Spec.Template.Spec.PriorityClassName
if tc.wantPriorityClass && daemonsetPC != criticalPriorityClass {
t.Errorf("unexpected daemonset pod priority class:\n\t(GOT) %v \n\t(WNT) %v", daemonsetPC, criticalPriorityClass)
}

if !tc.wantPriorityClass && daemonsetPC != "" {
t.Errorf("expected daemonset priority class to be not set")
if daemonsetPC != tc.wantDaemonSetPriorityClass {
t.Errorf("unexpected daemonset pod priority class:\n\t(GOT) %v \n\t(WNT) %v", daemonsetPC, tc.wantDaemonSetPriorityClass)
}

// Check pod priority class for both the kinds of CSI helpers.
Expand Down Expand Up @@ -1721,8 +1732,8 @@ func TestDeployPodPriorityClass(t *testing.T) {
csiHelperPC = createdStatefulset.Spec.Template.Spec.PriorityClassName
}

if csiHelperPC != "" {
t.Errorf("unexpected CSI helper pod priority class:\n\t(GOT) %v \n\t(WNT) %v", csiHelperPC, "")
if csiHelperPC != tc.wantCSIHelperPriorityClass {
t.Errorf("unexpected CSI helper pod priority class:\n\t(GOT) %v \n\t(WNT) %v", csiHelperPC, tc.wantCSIHelperPriorityClass)
}

// Check pod priority class for the api-manager.
Expand All @@ -1735,8 +1746,8 @@ func TestDeployPodPriorityClass(t *testing.T) {
t.Fatal("failed to get the created api-manager deployment", err)
}
apiManagerPC := createdAPIManagerDeployment.Spec.Template.Spec.PriorityClassName
if apiManagerPC != "" {
t.Errorf("unexpected api-manager pod priority class:\n\t(GOT) %v \n\t(WNT) %v", apiManagerPC, "")
if apiManagerPC != tc.wantAPIManagerPriorityClass {
t.Errorf("unexpected api-manager pod priority class:\n\t(GOT) %v \n\t(WNT) %v", apiManagerPC, tc.wantAPIManagerPriorityClass)
}

// Check pod priority class for the scheduler.
Expand All @@ -1749,8 +1760,8 @@ func TestDeployPodPriorityClass(t *testing.T) {
t.Fatal("failed to get the created scheduler deployment", err)
}
schedulerPC := createdSchedulerDeployment.Spec.Template.Spec.PriorityClassName
if schedulerPC != "" {
t.Errorf("unexpected scheduler pod priority class:\n\t(GOT) %v \n\t(WNT) %v", schedulerPC, "")
if schedulerPC != tc.wantSchedulerPriorityClass {
t.Errorf("unexpected scheduler pod priority class:\n\t(GOT) %v \n\t(WNT) %v", schedulerPC, tc.wantSchedulerPriorityClass)
}
})
}
Expand Down
31 changes: 23 additions & 8 deletions pkg/storageos/podspec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import (
)

const (
// Name of kube-system namespace.
kubeSystemNamespace = "kube-system"
// Name of the node critical priority class.
nodeCriticalPriorityClass = "system-node-critical"

// Name of the critical priority class.
criticalPriorityClass = "system-node-critical"
// Name of the cluster critical priority class.
clusterCriticalPriorityClass = "system-cluster-critical"
)

// addSharedDir adds env var and volumes for shared dir when running kubelet in
Expand Down Expand Up @@ -273,9 +273,24 @@ func (s *Deployment) addTLSEtcdCerts(podSpec *corev1.PodSpec) {
}
}

func (s *Deployment) addPodPriorityClass(podSpec *corev1.PodSpec) {
// Set pod priority to critical only when deployed in kube-system namespace.
if s.stos.Spec.GetResourceNS() == kubeSystemNamespace {
podSpec.PriorityClassName = criticalPriorityClass
// addCommonPodProperties adds common pod properties to a given pod spec. The
// common pod properties are common for all the pods that are part of storageos
// deployment, including the CSI helpers, api-manager and the scheduler.
func (s Deployment) addCommonPodProperties(podSpec *corev1.PodSpec) error {
s.addNodeAffinity(podSpec)
s.addPodPriorityClass(podSpec, clusterCriticalPriorityClass)

// Add helper tolerations.
if err := s.addHelperTolerations(podSpec, podTolerationSeconds); err != nil {
return err
}
return nil
}

// addPodPriorityClass sets the priority class for a Pod.
//
// Prior to StorageOS v2.4.0, the priority class was only set for Pods running
// in the kube-system namespace, as k8s 1.16 and earlier would only allow this.
func (s *Deployment) addPodPriorityClass(podSpec *corev1.PodSpec, priorityClass string) {
podSpec.PriorityClassName = priorityClass
Copy link
Contributor

@darkowlzz darkowlzz May 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Getting rid of the kube-system namespace conditional, finally 😃
Maybe the comment about kube-system above is not necessary.

}