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

fix(upgrade): make sure trap will throw no errors #3176

Merged
merged 1 commit into from Nov 18, 2022

Conversation

starbops
Copy link
Member

Signed-off-by: Zespre Chang zespre.chang@suse.com

IMPORTANT: Please do not create a Pull Request without creating an issue first.

Problem:

When upgrading with the same ISO, the cleanup trap in the upgrade_os will throw out an error that triggers the post-drain to retry again. This is because the line:

[[ -n "$NEW_OS_SQUASHFS_IMAGE_FILE" ]] && \rm -vf "$NEW_OS_SQUASHFS_IMAGE_FILE"

will short-circuit with a non-zero return code if the variable is not set.

Solution:

A trap should never exit with a non-zero return code, all cases should be handled carefully.

Related Issue:

#3175

Test plan:

  1. Create a cluster with master ISO after v1.1.1-rc1
  2. Upgrade to the same ISO again
  3. Check if the post-drain job ended successfully

Signed-off-by: Zespre Chang <zespre.chang@suse.com>
Copy link
Contributor

@guangbochen guangbochen 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.

Copy link
Contributor

@futuretea futuretea left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

LGTM

@bk201 bk201 merged commit e5b72aa into harvester:master Nov 18, 2022
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.

None yet

4 participants