Remove Restic code path from PodVolumeRestore#9732
Remove Restic code path from PodVolumeRestore#9732blackpiglet merged 1 commit intovelero-io:mainfrom
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
This might cause upgrade failure on our side.
Have we considered adding another version of the CRD ie. v2?
We can then implement conversion webhook to immediately deprecate/remove older v1 CRD.
https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#upgrade-existing-objects-to-a-new-stored-version
There was a problem hiding this comment.
For V1, we can keep same enums but edit description like https://github.com/vmware-tanzu/velero/pull/9676/changes#diff-e710ccbdbd4149cab26deb6b55961c09fdf6ef984d8ebfe252a6e30c40e8a31f
There was a problem hiding this comment.
Thanks. I add the restic type back.
shubham-pampattiwar
left a comment
shubham-pampattiwar
left a comment
There was a problem hiding this comment.
Thanks for putting this together, the restic code path cleanup is great to see.
One thing I wanted to flag: removing existing enum values (restic, "") from the CRD schemas is a breaking change per the https://github.com/operator-framework/operator-controller/blob/main/docs/concepts/crd-upgrade-safety.md policy. This would block upgrades for OLM-managed deployments, and existing CRs in etcd with those values would fail validation against the new schema.
Could we keep the enum as-is and instead add controller-level validation that rejects restic / "" at reconcile time (e.g., fail with "restic uploader is no longer supported, use kopia")? Same end result without the schema-level break.
+1 to @kaovilai's concern as well.
|
+1 to avoiding the breaking change to CRDs. Without version bumps, it's ok to add to enum values but not to remove them. Failing with a validation error when Restic is specified should be sufficient. |
blackpiglet
force-pushed
the
9468_fix
branch
2 times, most recently
from
c15345f to
af616d2
Compare
Thanks. I add the restic type back. |
Thanks. I add the restic type back. |
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
6 participants
Thank you for contributing to Velero!
Please add a summary of your change
Does your change fix a particular issue?
Fixes #9468
Please indicate you've done the following:
make new-changelog) or comment/kind changelog-not-requiredon this PR.site/content/docs/main.