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

signpost: cancel-upgrade behavior fix #950

Merged

Conversation

etungsten
Copy link
Contributor

@etungsten etungsten commented Jun 19, 2020

Issue number: Fixes #949

Description of changes:

Author: Erikson Tung <[email protected]>
Date:   Fri Jun 19 11:55:10 2020 -0700

    signpost: `cancel-upgrade` behavior update
    
    Changes `cancel-upgrade` so that it only reverts actions done by
    `upgrade-to-inactive` as the usage information implies.
    `cancel-upgrade` now preserves the number of tries left on the
    inactive partition instead of just wiping all bits

Testing done:
Initial status:

bash-5.0# signpost status
OS disk: /dev/nvme0n1
Set A:   boot=/dev/nvme0n1p2 root=/dev/nvme0n1p3 hash=/dev/nvme0n1p4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/nvme0n1p6 root=/dev/nvme0n1p7 hash=/dev/nvme0n1p8 priority=0 tries_left=0 successful=false
Active:  Set A
Next:    Set A

Mark inactive partition as valid, see tries_left set to 1

bash-5.0# signpost mark-inactive-valid
bash-5.0# signpost status             
OS disk: /dev/nvme0n1
Set A:   boot=/dev/nvme0n1p2 root=/dev/nvme0n1p3 hash=/dev/nvme0n1p4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/nvme0n1p6 root=/dev/nvme0n1p7 hash=/dev/nvme0n1p8 priority=0 tries_left=1 successful=false
Active:  Set A
Next:    Set A

Make the inactive as next to boot, see priority bumped to 2

bash-5.0# signpost upgrade-to-inactive
bash-5.0# signpost status             
OS disk: /dev/nvme0n1
Set A:   boot=/dev/nvme0n1p2 root=/dev/nvme0n1p3 hash=/dev/nvme0n1p4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/nvme0n1p6 root=/dev/nvme0n1p7 hash=/dev/nvme0n1p8 priority=2 tries_left=1 successful=false
Active:  Set A
Next:    Set B

cancel-upgrade then properly rolls back upgrade-to-inactive by setting the priority back to 0 while keeping tries_left at 1

bash-5.0# signpost cancel-upgrade                                                                                                                                             
bash-5.0# signpost status        
OS disk: /dev/nvme0n1
Set A:   boot=/dev/nvme0n1p2 root=/dev/nvme0n1p3 hash=/dev/nvme0n1p4 priority=2 tries_left=0 successful=true
Set B:   boot=/dev/nvme0n1p6 root=/dev/nvme0n1p7 hash=/dev/nvme0n1p8 priority=0 tries_left=1 successful=false
Active:  Set A
Next:    Set A

Subsequent upgrade-to-inactive call is successful because the inactive partition is still marked valid (tries_left > 0)

bash-5.0# signpost upgrade-to-inactive
bash-5.0# signpost status             
OS disk: /dev/nvme0n1
Set A:   boot=/dev/nvme0n1p2 root=/dev/nvme0n1p3 hash=/dev/nvme0n1p4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/nvme0n1p6 root=/dev/nvme0n1p7 hash=/dev/nvme0n1p8 priority=2 tries_left=1 successful=false
Active:  Set A
Next:    Set B

cancel-upgrade then restores active partition as next to boot.
Another clear-inactive call then wipes the inactive partition priority bits.

bash-5.0# signpost cancel-upgrade     
bash-5.0# signpost status             
OS disk: /dev/nvme0n1
Set A:   boot=/dev/nvme0n1p2 root=/dev/nvme0n1p3 hash=/dev/nvme0n1p4 priority=2 tries_left=0 successful=true
Set B:   boot=/dev/nvme0n1p6 root=/dev/nvme0n1p7 hash=/dev/nvme0n1p8 priority=0 tries_left=1 successful=false
Active:  Set A
Next:    Set A
bash-5.0# signpost clear-inactive
bash-5.0# signpost status        
OS disk: /dev/nvme0n1
Set A:   boot=/dev/nvme0n1p2 root=/dev/nvme0n1p3 hash=/dev/nvme0n1p4 priority=2 tries_left=0 successful=true
Set B:   boot=/dev/nvme0n1p6 root=/dev/nvme0n1p7 hash=/dev/nvme0n1p8 priority=0 tries_left=0 successful=false
Active:  Set A
Next:    Set A
bash-5.0# 

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Changes `cancel-upgrade` so that it only reverts actions done by
`upgrade-to-inactive` as the usage information implies.
`cancel-upgrade` now preserves the number of tries left on the
inactive partition instead of just wiping all bits
@etungsten
Copy link
Contributor Author

Push above addresses @tjkirch 's comment.

Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Would like to make sure @iliana approves.

@etungsten etungsten merged commit 759c8c3 into bottlerocket-os:develop Jun 20, 2020
@etungsten etungsten deleted the signpost-cancel-upgrade branch June 20, 2020 00:52
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.

signpost: cancel-upgrade behavior misleading
3 participants