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

🌱 add api upgrade flow #265

Merged

Conversation

haoqing0110
Copy link
Member

Summary

Related issue(s)

Fixes #

Signed-off-by: haoqing0110 <qhao@redhat.com>
@haoqing0110
Copy link
Member Author

cc @zhujian7 we might work together to refine the API migration flow.

@haoqing0110
Copy link
Member Author

cc @qiujian16 @elgnay

@haoqing0110
Copy link
Member Author

/assign @deads2k

Hello @deads2k , we have a summary of the API upgrade flow based on our experience in previous releases, hope you can help review and provide some comments, thanks a lot!

docs/development.md Outdated Show resolved Hide resolved
docs/development.md Outdated Show resolved Hide resolved
docs/development.md Outdated Show resolved Hide resolved

### Relase N (add new version and depracated old version)
- Add new version v1beta2 API and mark v1beta1 as deprecated. For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is we mark it deprecated when we change the stored version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds makes sense.

In our previous releases, we tended to add new API and mark the old version deprecated first, letting the user know it's deprecated for at least 1 release and then start migration in the next release.

Now we have some experience in migration, it's okay to mark it deprecated when changing the stored version in release N+1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have updated the doc, also with some tips on how to set the served and storage version.

@qiujian16
Copy link
Member

@haoqing0110
Copy link
Member Author

https://github.com/open-cluster-management-io/community/blob/main/sig-architecture/api_changes.md is a useful addition to this flow, append some URLs to the related part.

Signed-off-by: haoqing0110 <qhao@redhat.com>
### Relase N+1 (deprecated old version and start migration)
- Mark v1beta1 as deprecated, served versions are v1beta1 and v1beta2, set storage: true on v1beta2.

This comment was marked as off-topic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qiujian16 Please take another review, should we put this flow into best-practice?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The risk we concerns here is this flow hasn't been verified by history. We never process in this way before, it's just a design.

@haoqing0110
Copy link
Member Author

@qiujian16 @elgnay @xuezhaojun plz help do a final review.


In registration-operator it has a migration controller and crd status controller to handle the progress, code example:
- https://github.com/open-cluster-management-io/registration-operator/pull/315
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xuezhaojun thanks, have updated this example in the doc.

### Relase N+x (remove old version)

when the new version API becomes mature, and ready to remove old version (for example, the consumers have adopted the new version API), remove the old version related code:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to clarify what READY means. What needs to be checked to ensure it is ready to remove the old version.

Signed-off-by: haoqing0110 <qhao@redhat.com>
@qiujian16
Copy link
Member

/approve

@xuezhaojun leave to you for final review

Copy link
Contributor

openshift-ci bot commented Dec 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haoqing0110, qiujian16

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@xuezhaojun
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Dec 22, 2023
@openshift-merge-bot openshift-merge-bot bot merged commit 04e951f into open-cluster-management-io:main Dec 22, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants