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

doc: engine version enforcement #766

Merged

Conversation

c3y1huang
Copy link
Contributor

@c3y1huang c3y1huang self-assigned this Aug 17, 2023
@c3y1huang c3y1huang force-pushed the feat-engine-version-enforcement branch 3 times, most recently from f3de2e6 to d4c253e Compare August 17, 2023 05:33
Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

@c3y1huang Please check the suggested wording.

Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

Let me know if these changes work.

content/docs/1.6.0/deploy/important-notes/index.md Outdated Show resolved Hide resolved
content/docs/1.6.0/deploy/important-notes/index.md Outdated Show resolved Hide resolved
content/docs/1.6.0/deploy/important-notes/index.md Outdated Show resolved Hide resolved
content/docs/1.6.0/deploy/important-notes/index.md Outdated Show resolved Hide resolved
@c3y1huang c3y1huang force-pushed the feat-engine-version-enforcement branch from d4c253e to a2c7ede Compare October 18, 2023 08:38
@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for longhornio ready!

Name Link
🔨 Latest commit 12099a6
🔍 Latest deploy log https://app.netlify.com/sites/longhornio/deploys/658a95844d992a0008e4ad87
😎 Deploy Preview https://deploy-preview-766--longhornio.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@c3y1huang c3y1huang force-pushed the feat-engine-version-enforcement branch 3 times, most recently from 2a9bea6 to 44cbdb8 Compare October 18, 2023 09:01
Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@c3y1huang c3y1huang force-pushed the feat-engine-version-enforcement branch 2 times, most recently from 888d62a to 97d0561 Compare December 26, 2023 05:39
@c3y1huang c3y1huang marked this pull request as ready for review December 26, 2023 05:40
@c3y1huang c3y1huang requested a review from a team as a code owner December 26, 2023 05:40
Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

@c3y1huang I moved the text that you added to make the meaning clearer.


When upgrading through Helm, a component compatibility check is automatically performed. If the new Longhorn is not compatible with the engine images that are currently in use, the upgrade path is blocked through a pre-hook mechanism.

If you installed Longhorn using the manifests, engine upgrades are enforced by the Longhorn Manager. Attempts to upgrade Longhorn Manager may cause unsuccessful pod launches and generate corresponding error logs, although it poses no harm. If you encounter such errors, you must revert to the previous Longhorn version and then upgrade the engines that are using the incompatible engine images before the next upgrade.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If you installed Longhorn using the manifests, engine upgrades are enforced by the Longhorn Manager. Attempts to upgrade Longhorn Manager may cause unsuccessful pod launches and generate corresponding error logs, although it poses no harm. If you encounter such errors, you must revert to the previous Longhorn version and then upgrade the engines that are using the incompatible engine images before the next upgrade.
If you installed Longhorn using a manifest file, engine upgrades are enforced by Longhorn Manager. Attempts to upgrade Longhorn Manager may cause unsuccessful pod launches (that pose no harm to the system) and generate corresponding error logs. If you encounter such errors, you must revert to the previous Longhorn Manager version and then upgrade engines with incompatible engine images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the top section of the suggestion. However, the lower part looks a bit odd to me.

you must revert to the previous Longhorn Manager version

The user should revert the Longhorn version, not just the manager's version.

upgrade engines with incompatible engine images

This sounds like we are asking the user to upgrade the engine using an incompatible engine image.

Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

@c3y1huang Apologies, I was working off of an older version of the text. Let's revert to what we had before the latest addition.

content/docs/1.6.0/deploy/important-notes/index.md Outdated Show resolved Hide resolved
ref: longhorn-5842

Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>
@c3y1huang c3y1huang force-pushed the feat-engine-version-enforcement branch from 0c40298 to 12099a6 Compare December 26, 2023 08:57
@c3y1huang c3y1huang merged commit e461d58 into longhorn:master Dec 27, 2023
6 checks passed
@c3y1huang c3y1huang deleted the feat-engine-version-enforcement branch December 27, 2023 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants