Skip to content
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.

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

Backup skipped PriorityClass even when it is referenced by Pod #3933

Closed
phuongatemc opened this issue Jul 7, 2021 · 8 comments · Fixed by #4740
Closed

Backup skipped PriorityClass even when it is referenced by Pod #3933

phuongatemc opened this issue Jul 7, 2021 · 8 comments · Fixed by #4740
Assignees
Labels
Enhancement/User End-User Enhancement to Velero

Comments

@phuongatemc
Copy link
Contributor

Steps to reproduce:

  • Create a PriorityClass "high-priority" as below:
apiVersion: scheduling.k8s.io/v1
kind: PriorityClass
metadata:
  name: high-priority
value: 1000000
globalDefault: false
description: "This priority class should be used for XYZ service pods only."
  • Create a Pod and specified "priorityClassName" as "high-priority"
  • Run Backup with include-cluster-resources set to nil. log shows that the priorityclasses being skipped
    time="2021-07-06T23:01:42Z" level=info msg="Getting items for resource" backup=velero-ppdm/mysql-2021-07-06-16-01-40-mysql group=scheduling.k8s.io/v1 logSource="pkg/backup/item_collector.go:165" resource=priorityclasses
    time="2021-07-06T23:01:42Z" level=info msg="Skipping resource because it's cluster-scoped and only specific namespaces are included in the backup" backup=velero-ppdm/mysql-2021-07-06-16-01-40-mysql group=scheduling.k8s.io/v1 logSource="pkg/backup/item_collector.go:192" resource=priorityclasses
  • Restore to a different cluster that does not have the PriorityClass "high-priority" ==> Restore failed to restore Pod with error that it cannot find the PriorityClass.

There was an suggestion in Velero community meeting and in the code comments to write a plugin for Pod and return the PriorityClass high-priority as the AdditionalItems to backup. However, I think such plugin would be a workaround because the main problem is the Backup datapath (getResourceItems in specific) should pick up all resources being referenced by the Pod and backup them accordingly.

@reasonerjt
Copy link
Contributor

reasonerjt commented Jul 14, 2021

Per discussion, this can be fixed by introducing a backupItemAction in core velero.

@stale
Copy link

stale bot commented Sep 12, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the staled label Sep 12, 2021
@phuongatemc
Copy link
Contributor Author

We actually still need this. Instead of writing new plugin, we will enhance the currently internal plugin the backup the PriorityClass if the Pod Spec has reference to it.

@stale
Copy link

stale bot commented Sep 26, 2021

Closing the stale issue.

@stale stale bot closed this as completed Sep 26, 2021
@reasonerjt reasonerjt reopened this Sep 27, 2021
@stale stale bot removed the staled label Sep 27, 2021
@reasonerjt reasonerjt added the Enhancement/User End-User Enhancement to Velero label Sep 27, 2021
@phuongatemc
Copy link
Contributor Author

This is discussed in the meeting today. We still need it.

@phuongatemc
Copy link
Contributor Author

Currently, in the internal plugin for Backup's PodAction, https://github.com/vmware-tanzu/velero/blob/main/pkg/backup/pod_action.go we only add PVCs to the "additionalItems" array. We could add PriorityClass as in that array as well.
Similarly, the internal plugin for restore's PodAction, https://github.com/vmware-tanzu/velero/blob/main/pkg/restore/pod_action.go we could also add PriorityClass as well.

@sseago
Copy link
Collaborator

sseago commented Mar 8, 2022

@phuongatemc Yes, that makes sense. While "make it a BackupItemAction" was proposed above, I think using an existing BackupItemAction that's already being run for pods probably makes even more sense, as we don't really need to run a completely separate action to add more than one type of resources as additional items.

@phuongatemc
Copy link
Contributor Author

PR: #4740
I will do a little more tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement/User End-User Enhancement to Velero
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants