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

[E2E - Upgrade Testing] logic for testing upgrade process #1333

Closed
13 tasks done
czarcas7ic opened this issue Apr 23, 2022 · 6 comments · Fixed by #1389
Closed
13 tasks done

[E2E - Upgrade Testing] logic for testing upgrade process #1333

czarcas7ic opened this issue Apr 23, 2022 · 6 comments · Fixed by #1389
Assignees
Labels
T:task ⚙️ A task belongs to a story T:tests

Comments

@czarcas7ic
Copy link
Member

czarcas7ic commented Apr 23, 2022

This issue will lay out initial thoughts on what the upgrade process must entail to complete issue #1235 based on discussion with @p0mvn

  • Ensure version X+1's upgrades.go and constants.go exist in the current branch
  • Figure out how long gov submission / passing takes
    • Ensure we modify genesis to have fast voting periods
    • Transfer this time to number of blocks, then add some for a buffer. This will be our upgrade height
  • Chain will be initialized on current stable prod release (version X)
  • Start chain with halt height = upgrade height
  • Gov logic executes
    • Prop submission
    • Prop fund
    • Prop vote
    • Prop passes
  • Once daemon stops due to halt height, start new docker container using osmosis:debug (locally compiled docker image based off main).
    • Ensure volume used on version X is mounted to version X+1

From here, we can run tests to ensure nothing broke.

@alexanderbez
Copy link
Contributor

Awesome, thanks for writing this up @czarcas7ic.

Question though, why do we need this step:

Start chain with halt height = upgrade height

@czarcas7ic
Copy link
Member Author

@alexanderbez From what I remember, the chain doesn't actually halt on upgrade height right? It displays a message saying the that the new binary is needed and will just sit idly until the daemon is stopped and the binary is changed (I could be wrong and it could panic and halt). When I thought up this process, it just seemed simpler to use the halt height flag since the upgrade height must be hardcoded via the gov prop anyway and wont change. I'm sure there are other ways to do this, this was just the first one that came to mind.

@alexanderbez
Copy link
Contributor

@czarcas7ic you might be right. Either way, it can't hurt to add a halt-height configuration, so I'm all for it 👍

@czarcas7ic
Copy link
Member Author

Just noting here that the chain does in fact not panic, you get a log like the following

6:03PM ERR UPGRADE "v8" NEEDED at height: 100: 
6:03PM ERR CONSENSUS FAILURE!!! err="UPGRADE \"v8\" NEEDED at height: 100:

and then it goes back to searching for peers. No issues with this, just noting for future reference.

@alexanderbez
Copy link
Contributor

Well that is a panic, but the node does not halt. So yes, I guess we need to add that config 👍

@p0mvn p0mvn changed the title logic for testing upgrade process [Upgrade Testing] logic for testing upgrade process May 4, 2022
@p0mvn
Copy link
Member

p0mvn commented May 4, 2022

@czarcas7ic please add correct labels and update the status when you have time

@czarcas7ic czarcas7ic moved this from 🔍 Needs Review to 🏃 In Progress in Osmosis Chain Development May 4, 2022
@czarcas7ic czarcas7ic self-assigned this May 4, 2022
@p0mvn p0mvn added the T:task ⚙️ A task belongs to a story label May 4, 2022
@p0mvn p0mvn changed the title [Upgrade Testing] logic for testing upgrade process [E2E - Upgrade Testing] logic for testing upgrade process May 4, 2022
@czarcas7ic czarcas7ic moved this from 🏃 In Progress to 🔍 Needs Review in Osmosis Chain Development May 4, 2022
Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:task ⚙️ A task belongs to a story T:tests
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants