-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Propose a deprecation process for velero #5532
Conversation
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. |
Closing the stale issue. |
Not really stale. This needs to be resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In today's community, folks discussed to merge this PR and get the deprecation process in place. Found there are a few remaining comments. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5532 +/- ##
===========================================
+ Coverage 48.18% 58.84% +10.66%
===========================================
Files 227 346 +119
Lines 23342 28891 +5549
===========================================
+ Hits 11247 17002 +5755
+ Misses 11244 10458 -786
- Partials 851 1431 +580 ☔ View full report in Codecov by Sentry. |
6ac33dd
to
24f75b6
Compare
I have updated the PR, please take another look @reasonerjt @sseago @Lyndon-Li @weshayutin @ihcsim @OrlinVasilev @yanji09 |
As discussed in the velero community call [1] This is a proposed deprecation policy for the velero project based on the goharbor project. [1] https://hackmd.io/Jq6F5zqZR7S80CeDWUklkA Update GOVERNANCE.md definitive deprecation times, well done Co-authored-by: Orlix <[email protected]> Signed-off-by: Wesley Hayutin <[email protected]> Update GOVERNANCE.md Co-authored-by: Ivan Sim <[email protected]> Signed-off-by: Orlix <[email protected]> Update GOVERNANCE.md Co-authored-by: Ivan Sim <[email protected]> Signed-off-by: Orlix <[email protected]> add note regarding deprecation window Signed-off-by: Shubham Pampattiwar <[email protected]> add changelog file Signed-off-by: Shubham Pampattiwar <[email protected]>
24f75b6
to
8e8c340
Compare
it's a good start |
GOVERNANCE.md
Outdated
|
||
The deprecation window is the period from the release in which the deprecation takes effect through the release in which the feature is removed. During this period, only critical security vulnerabilities and catastrophic bugs should be fixed. | ||
|
||
**Note:** If a deprecated feature must remain available for general use for two releases after deprecation, perhaps a backup that uses this deprecated feature must be fully-supported for restore for four releases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"must be fully supported for restore for four releases" perhaps we want to keep the code for backup but only allow it for generating and running test cases, not general use by end user.
Running more than one version to test would be ideal but more difficult imo especially in unit testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what use case does the "*Note:**" section try to solve? thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@renmaosheng
See this comment , if a feature is essential to the backup which for example, impacts the backed up data, when we deprecate the feature, we should guarantee that users are still able to access their backups, e.g., restore from them (in a longer period of time).
Here Restic deprecation is right an example, users may have some previous backups created by Restic, and they may want to preserve the backups for quite long time, remaining able to restore from them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restic is the main example we had in mind. If we deprecate Restic in 1.13, then restic must be available for new backups for N+2 releases, but prior restic backups must be available for restore for longer. Otherwise, if we removed Restic completely in 1.15 (rather than just on the backup side), this would mean that a user could make a Restic backup in 1.14, upgrade to 1.15 and that "one release old" backup would no longer be restorable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also find the sentence
perhaps a backup that uses this deprecated feature must be fully-supported for restore for four releases.
is a little confusing.
How about we just explicitly mention that a rule of thumb for velero is to be able to restore a backup that is created in version n-2
, and rule must not be violated when a deprecated feature is removed from velero?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reasonerjt @weshayutin Something like this?
"If a backup relies on a deprecated feature, then backups made with the last Velero release before this feature is removed must still be restorable in version n+2
. For something like restic support, that might mean that restic is removed from the list of supported uploader types in version n
but the underlying implementation required to restore from a restic backup won't be removed until release n+2
"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 @sseago
@reasonerjt @weshayutin I can update the PR if we agree upon what @sseago suggested here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ I agree w/ what @sseago said there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham-pampattiwar
Sorry for the late response, the proposed wording by @sseago looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reasonerjt @weshayutin @sseago Done, updated the PR.
Signed-off-by: Shubham Pampattiwar <[email protected]>
Signed-off-by: Shubham Pampattiwar <[email protected]>
As discussed in the velero community call [1]
This is a proposed deprecation policy for the
velero project based on the goharbor project.
[1] https://hackmd.io/Jq6F5zqZR7S80CeDWUklkA