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

📖 Draft documentation for initial upgrade support #559

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

m1kola
Copy link
Member

@m1kola m1kola commented Nov 29, 2023

Description

Draft document to cover initial upgrade support.

Closes #470

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 29, 2023
Copy link

codecov bot commented Nov 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6cb7029) 83.93% compared to head (2f7d599) 83.93%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #559   +/-   ##
=======================================
  Coverage   83.93%   83.93%           
=======================================
  Files          20       20           
  Lines         809      809           
=======================================
  Hits          679      679           
  Misses         90       90           
  Partials       40       40           
Flag Coverage Δ
e2e 63.78% <ø> (ø)
unit 78.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@m1kola m1kola force-pushed the semver_docs_draft branch 3 times, most recently from 151b3a3 to 7e7440a Compare November 30, 2023 10:57
@m1kola m1kola marked this pull request as ready for review November 30, 2023 10:58
@m1kola m1kola requested a review from a team as a code owner November 30, 2023 10:58
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 30, 2023
Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

Great job! I left some suggestions for you to consider.

docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
docs/drafts/initial-upgrade-support.md Outdated Show resolved Hide resolved
@m1kola m1kola force-pushed the semver_docs_draft branch 3 times, most recently from db98441 to 70ce226 Compare December 1, 2023 10:16
Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

Just a couple of small follow up questions and ideas that didn't occur to me on the first review.

This is looking really good 🚀

docs/drafts/upgrade-support.md Show resolved Hide resolved
* [Semantic Versioning](https://semver.org/) (Semver)
* The `replaces` directive from the [legacy OLM 0 semantics](https://olm.operatorframework.io/docs/concepts/olm-architecture/operator-catalog/creating-an-update-graph/#methods-for-specifying-updates)

The Kubernetes manifests in this repo enable Semver support by default. Cluster admins can control which semantics to use by passing one of the following arguments to the `manager` binary:
Copy link
Contributor

@michaelryanpeter michaelryanpeter Dec 1, 2023

Choose a reason for hiding this comment

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

This might be out of scope for this issue, but it might be worth explaining in a follow up issue why a cluster admin would want to deviate from the upgrade constraint semantics specified by the package author.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added the following:

We plan to remove the ForceSemverUpgradeConstraints feature gate and enable package authors to specify which upgrade constraint semantics they wish to use on the catalog level.

Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

One last suggestion, but otherwise LGTM.

docs/drafts/upgrade-support.md Outdated Show resolved Hide resolved
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
Co-authored-by: Michael Peter <mipeter@redhat.com>
Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

LGTM

@m1kola m1kola enabled auto-merge December 4, 2023 11:46

## Upgrades

OLM supports Semver to provide a simplified way for package authors to define compatible upgrades. According to the Semver standard, releases within a major version (e.g. `>=1.0.0 <2.0.0`) must be compatible. As a result, package authors can publish a new package version following the Semver specification, and OLM assumes compatibility. Package authors do not have to explicitly define upgrade edges in the catalog.
Copy link
Contributor

@tmshort tmshort Dec 4, 2023

Choose a reason for hiding this comment

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

Nit: There are various levels of Semver compatibility, as specified by all three component versions. There's no mention of patch compatibility here. But, we don't want to get ourselves too deep in the weeds with explanations here, as we reference Semver above.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tmshort Here we say that anything witing the major version is expected to be compatible. E.g. 1.0.3 is compatible with 1.3.0. So pacakge authors can release accordingly. I think it is not necessary to go into the details of semver spec itself here (e.g. patch versions are for bug fixes, minor versions are fore backward compatible features).

But I'm happy to add more info if you think we should stress some specific aspect of semver here. Let me know.

Copy link
Contributor

@tmshort tmshort left a comment

Choose a reason for hiding this comment

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

/lgtm
Just one nit, but these are engineering/draft docs.

@m1kola m1kola added this pull request to the merge queue Dec 4, 2023
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 4, 2023
Merged via the queue into operator-framework:main with commit 5ae5404 Dec 4, 2023
15 checks passed

OLM supports Semver to provide a simplified way for package authors to define compatible upgrades. According to the Semver standard, releases within a major version (e.g. `>=1.0.0 <2.0.0`) must be compatible. As a result, package authors can publish a new package version following the Semver specification, and OLM assumes compatibility. Package authors do not have to explicitly define upgrade edges in the catalog.

**Note:** Currently, OLM 1.0 does not support automatic upgrades to the next major version. You must manually verify and perform major version upgrades. For more information about major version upgrades, see [Manually verified upgrades and downgrades](#manually-verified-upgrades-and-downgrades).
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Not sure if we've landed on a standard or anything like that for how we want to do notes, warnings, etc. in our docs but I find that combining the quote (>) markdown syntax with the bolded "Note", "Warning", etc. is more eye catching than only the bolded text and helps with readability (at least for me).

Not sure how the docs site we use renders them, but in GitHub's markdown you can even do:

Note

Something of great importance

Warning

Something of even greater importance

How ^ is done:

>[!NOTE]
>Something of great importance

>[!WARNING]
>Something of _even greater_ importance

Examples

With Quote/Note syntax

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

Note

Something worthy of catching my eye due to it's importance

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

With Bolded Text

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

Note: Something worthy of catching my eye due to it's importance

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice one! I'll submit a follow up PR to use this markdown. Thanks for the suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

PR: #562

@m1kola m1kola deleted the semver_docs_draft branch December 4, 2023 14:58
@m1kola m1kola mentioned this pull request Dec 4, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for semver upgrade edge constraints support
4 participants