PriorityClass Support Design Proposal

[kaovilai]

Design for #8869

Signed-off-by: Tiger Kaovilai tkaovila@redhat.com

Thank you for contributing to Velero!

Please add a summary of your change

Architecture Diagram

graph TB
    subgraph "User Configuration"
        CLI[velero install CLI]
        CLI -->|--server-priority-class-name| ServerFlag[Server Priority Class]
        CLI -->|--node-agent-priority-class-name| NodeAgentFlag[Node Agent Priority Class]
    end

    subgraph "Velero Components"
        ServerFlag --> Deployment[Velero Server Deployment]
        NodeAgentFlag --> DaemonSet[Node Agent DaemonSet]
        
        Deployment -->|stores config| ConfigMap1[velero-config ConfigMap]
        DaemonSet -->|stores config| ConfigMap2[node-agent-config ConfigMap]
    end

    subgraph "Data Movement Pods"
        ConfigMap2 -->|reads priority class| DataUpload[Data Upload Controller]
        ConfigMap2 -->|reads priority class| DataDownload[Data Download Controller]
        DataUpload --> DataMoverPod1[Data Mover Pods]
        DataDownload --> DataMoverPod2[Data Mover Pods]
    end

    subgraph "Maintenance Jobs"
        ConfigMap1 -->|global config only| MaintenanceConfig[Maintenance Job Config]
        MaintenanceConfig --> MaintenanceJob[Repository Maintenance Jobs]
    end

    subgraph "CSI Snapshot Operations"
        ConfigMap2 -->|reads priority class| CSISnapshot[CSI Snapshot Exposer]
        ConfigMap2 -->|reads priority class| GenericRestore[Generic Restore Exposer]
        CSISnapshot --> ExposeePod1[Exposee Pods]
        GenericRestore --> ExposeePod2[Restore Pods]
    end

    style CLI fill:#e1f5fe
    style Deployment fill:#81c784
    style DaemonSet fill:#81c784
    style DataMoverPod1 fill:#ffb74d
    style DataMoverPod2 fill:#ffb74d
    style MaintenanceJob fill:#ce93d8
    style ExposeePod1 fill:#ffb74d
    style ExposeePod2 fill:#ffb74d

Loading

Does your change fix a particular issue?

Implementation: #8883

Please indicate you've done the following:

• Accepted the DCO. Commits without the DCO will delay acceptance.
• Created a changelog file (make new-changelog) or comment /kind changelog-not-required on this PR.
• Updated the corresponding documentation in site/content/docs/main.

[kaovilai]

/kind changelog-not-required

[Lyndon-Li]

Data movers should be with low priority in most cases, in another word, when resource is limited, data movers should not run.

If we decide to inherit from node-agent, we should make sure the priority class configuration is separate for node-agent and Velero server. cc @ywk253100

[kaovilai]

Do we want to have a low default? Or leave as k8s default but allow user to set themselves?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.05%. Comparing base (8ce513c) to head (ea20550).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8882      +/-   ##
==========================================
+ Coverage   60.03%   60.05%   +0.01%     
==========================================
  Files         379      379              
  Lines       43496    43496              
==========================================
+ Hits        26114    26122       +8     
+ Misses      15814    15807       -7     
+ Partials     1568     1567       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ywk253100]

Seems we don't need this because we'll define the priority class in the repo maintenance configmap?

[kaovilai]

The velero install can create configmap containing maintenance priority class for you. WDYT?

[kaovilai]

ok will move to existing configmap

[ywk253100]

I don't think there is requirement to set the priority class in backup repository level? All maintenance jobs should have the same priority? @Lyndon-Li WDYT?

[kaovilai]

agree.

[ywk253100]

LGTM

[kaovilai]

I think we'll still need install flag for daemonset priorityclass since it will be set to daemonset during install.

[blackpiglet]

for both the node agent daemonset and data mover pods.
This doesn't seem right. The PriorityClassName in Configs is only used for the data mover pods, not for the node-agent DaemonSet.