[FEATURE] Support PriorityClass for Longhorn deployment

[zaggash]

Is your feature request related to a problem? Please describe.
This is to avoid the situation where as soon as longhorn-pods are evicted, all other workloads depending on the evicted longhorn-pod can no longer work.

Describe the solution you'd like
In case of resource exhaustion (memory most probably), it would be nice if longhorn can be evicted later than any other "normal" workload

Describe alternatives you've considered
Is there a way to add something like the PriorityClass to leverage the functionality described above.

gz#10575

[ttpcodes]

Pre-merged Checklist

• Does the PR include the explanation for the fix or the feature?

• Is the backend code merged?
  The PR is at Add Priority Class setting longhorn-manager#606.

• Is the reproduce steps/test steps documented?

• Which areas/issues this PR might have potential impacts on?
  Areas: deployment, manager
  Issues: [FEATURE] Support PriorityClass for Longhorn deployment #1487

• If the fix introduces the code for backward compatibility Has an separate issue been filed with the label release/obsolete-compatibility?

• If labeled: area/ui Has the UI issue filed or ready to be merged?

• if labeled: require/doc Has the necessary document PR submitted or merged?
  The Doc issue/PR is at Add documentation for Priority Class setting website#157.

• If labeled: require/automation-e2e Has the end-to-end test plan been merged? Have QAs agreed on the automation test case?
  The automation skeleton PR is at Add tests for Priority Class setting longhorn-tests#360.
  The automation test case PR is at Add tests for Priority Class setting longhorn-tests#360.

• if labeled: require/automation-engine Has the engine integration test been merged?

• if labeled: require/manual-test-plan Has the manual test plan been documented?
  The updated manual test plan is here

[ttpcodes]

Test Steps:

1. Set up a Longhorn installation.
2. Verify that the default Priority of all the Pods in the longhorn-system namespace is 0:

~ kubectl -n longhorn-system describe pods | grep Priority
# should be repeated many times
Priority:     0 

3. In the Longhorn UI, under the Settings menu, set the Priority Class to a Priority Class that exists on the cluster. (By default, Kubernetes comes with system-cluster-critical and system-node-critical, both of which can be used for testing.
4. The output of kubectl -n longhorn-system get pods should indicate that Pods are being terminated or created as a result of us changing this setting. Wait for all Pods to be running again.
5. Check the Priority of the Pods now. The output of kubectl should indicate that the Priority Class has been applied to all of the Pods in longhorn-system.

~ kubectl -n longhorn-system describe pods | grep Priority
# should be repeated many more times
Priority:             2000001000  
Priority Class Name:  system-node-critical

6. (Step for manually verifying new Engine Image DaemonSets) Add a new Engine Image. The output of kubectl (like above) should indicate that the new Pods created from the Engine Image have the Priority Class set properly.
7. Attempt to set the Priority Class to the name of a non-existent Priority Class. When attempting to save the new Setting, the UI should return an error and none of the workloads should be updated.
8. Set the Priority Class back to nothing. Wait for Pods to be terminated and recreated with the new setting. The priority of the new Pods should match the output of Step 2.

[yasker]

@ttpcodes I've updated the required labels to add automation e2e and docs. I thought it was a modification on the yaml level but since it's in the manager, we need e2e test and docs. It's similar to the taint tolerance feature.

[yasker]

Also, make sure we've updated the default setting yaml/chart for any new settings.

Make sure to test the default setting in the manual test as well.

[ttpcodes]

Since the previous Manual Test Plan is now covered by the longhorn-manager integration tests, there only remains manual tests for the default setting. Here are the three test cases:

1. Install Longhorn with no priority-class set in the default settings. The Priority Class setting should be empty after the installation completes according to the longhorn-ui, and the default Priority of all Pods in the longhorn-system namespace should be 0:

~ kubectl -n longhorn-system describe pods | grep Priority
# should be repeated many times
Priority:     0 

2. Install Longhorn with a nonexistent priority-class in the default settings. The system should fail to come online. The Priority Class setting should be set and the status of the Daemon Set for the longhorn-manager should indicate that the reason it failed was due to an invalid Priority Class:

~ kubectl -n longhorn-system describe lhs priority-class
Name:         priority-class
...
Value:                 nonexistent-priority-class
...
~ kubectl -n longhorn-system describe daemonset.apps/longhorn-manager
Name:           longhorn-manager
...
  Priority Class Name:  nonexistent-priority-class
Events:
  Type     Reason            Age                From                  Message
  ----     ------            ----               ----                  -------
  Normal   SuccessfulCreate  23s                daemonset-controller  Created pod: longhorn-manager-gbskd
  Normal   SuccessfulCreate  23s                daemonset-controller  Created pod: longhorn-manager-9s7mg
  Normal   SuccessfulCreate  23s                daemonset-controller  Created pod: longhorn-manager-gtl2j
  Normal   SuccessfulDelete  17s                daemonset-controller  Deleted pod: longhorn-manager-9s7mg
  Normal   SuccessfulDelete  17s                daemonset-controller  Deleted pod: longhorn-manager-gbskd
  Normal   SuccessfulDelete  17s                daemonset-controller  Deleted pod: longhorn-manager-gtl2j
  Warning  FailedCreate      4s (x14 over 15s)  daemonset-controller  Error creating: pods "longhorn-manager-" is forbidden: no PriorityClass with name nonexistent-priority-class was found

3. Install Longhorn with a valid priority-class in the default settings. The Priority Class setting should be set according to the longhorn-ui, and all the Pods in the longhorn-system namespace should have the right Priority set:

~ kubectl -n longhorn-system describe pods | grep Priority
# should be repeated many times
Priority:             2000001000  
Priority Class Name:  system-node-critical

[khushboo-rancher]

Verified on master - 07-02-2020

Validation - Passed

Scenarios tested:

1. Priority class with no value - If memory is exhausted, longhorn pod gets evicted.
2. Priority class with system-cluster-critical and system-node-critical - Longhorn pod doesn't evict.
3. Priority class with dummy value. - UI doesn't allow to set dummy value.

[runningman84]

Btw. this wont work at AWS or other public providers which do not have the latest k8s version:
kubernetes/kubernetes#76310

[yasker]

@ttpcodes Regarding the comment #1487 (comment) , can you check if PriorityClass has Kubernetes version restriction?

[ttpcodes]

Priority Classes have been a stable feature in Kubernetes since 1.14. However, the case that's being referred to here seems to be the fact that before 1.17, Pods could not set the Priority Class to system-node-critical or system-cluster-critical, which are built-in Priority Classes that are shipped with Kubernetes. I validated this on a 1.16 cluster and confirmed that setting the Priority Class to system-node-critical causes the Longhorn workloads to fail to come back online.

[yasker]

@ttpcodes Thanks for clarifying. That should be fine since none of them is the Longhorn default.

Also, thanks @runningman84 for reporting. Are you referring to the test steps like @ttpcodes mentioned above, or something else?

[khushboo-rancher]

As @ttpcodes mentioned, before 1.17 Pods could not set the Priority Class to system-node-critical or system-cluster-critical.
EKS has latest kubernetes version as 1.15. Latest master of longhorn is getting deployed properly but we cannot set Priority Class to system-node-critical or system-cluster-critical, it causes longhorn pods to fail to come up healthy.

[runningman84]

Maybe you can make the priority only default if k8s version is high enough. What you could also do is creating a own priority class with a similar numeric value as system node critical....
Or just wait a few months until 1.17 is default anyway

[yasker]

@runningman84 you know AWS might take a year or two to move to 1.17 :)

@ttpcodes we don't have default priority class right? Users should be able to just leave it blank.

[ttpcodes]

Yes, there's no default value for the Priority Class. The user can just run Longhorn without this set, or they can run with their own Priority Class until 1.17 when system-node-critical and system-cluster-critical can be used.

[yasker]

That should be good enough for us.

[drewhemm]

Have set the priority-class config value to the name of a custom Priority Class and yet none of the Longhorn workloads are using it. Priority for all LH pods remains at 0 - any ideas why?

[yasker]

@drewhemm Can you file a new issue and add the reproduce steps?

[drewhemm]

Sure - here it is:
#1838