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

add test for upgrade polkadot version in node #269

Merged
merged 4 commits into from
Sep 5, 2022
Merged

Conversation

pepoviola
Copy link
Collaborator

@pepoviola pepoviola commented Jul 14, 2022

POC: Add upgrade node test. (see paritytech/polkadot-sdk#851)
cc @chevdor, this is something like we talk last time?

Thanks!

@pepoviola pepoviola changed the title add test for upgrade polkadot version i node add test for upgrade polkadot version in node Jul 15, 2022
tests/k8s/downloadPolkadot.sh Outdated Show resolved Hide resolved
@@ -106,7 +106,15 @@ function make_transfer_containter(): any {
command: [
"ash",
"-c",
`until [ -f ${FINISH_MAGIC_FILE} ]; do echo waiting for tar to finish; sleep 1; done; echo copy files has finished`,
[ "wget https://github.com/moparisthebest/static-curl/releases/download/v7.83.1/curl-amd64 -O /cfg/curl",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you bother downloading curl when you have wget already ?

Copy link
Collaborator Author

@pepoviola pepoviola Jul 15, 2022

Choose a reason for hiding this comment

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

Hi @chevdor, this is an init container since the polkadot image don't have wget or curl and the image run as non-root user I make curl (downloading the static version) available through init containers.

@chevdor
Copy link
Contributor

chevdor commented Jul 15, 2022

cc @EgorPopelyaev: this is something we were missing.

Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
@sandreim
Copy link
Contributor

Thanks @pepoviola . This looks heading to the right direction, but the actual test should live in the polkadot repo under the misc folder. Additionally, it should be able to determine the last release version and start with that, then after doing some work and populating the DB it should switch the client binary to the one from the PR artifacts. Testing both rocksDB and parityDB at same time (2 nodes) would be awesome.

@pepoviola
Copy link
Collaborator Author

Thanks @pepoviola . This looks heading to the right direction, but the actual test should live in the polkadot repo under the misc folder. Additionally, it should be able to determine the last release version and start with that, then after doing some work and populating the DB it should switch the client binary to the one from the PR artifacts. Testing both rocksDB and parityDB at same time (2 nodes) would be awesome.

Hi @sandreim, sounds good. We can start the nodes with the image from parity/polkadot:latest (the registry were we publish the releases). What do you have in mind to populate the DB?

Thanks!

@ordian
Copy link
Member

ordian commented Sep 2, 2022

anything blocking this?

@pepoviola
Copy link
Collaborator Author

anything blocking this?

No, the feature is already working in zombienet. I can write a test in polkadot repo, did you have a list changes to make in the db before running the upgrade?

Thanks!

@sandreim
Copy link
Contributor

sandreim commented Sep 2, 2022

@pepoviola i think just letting it run for 2-3 sessions with 2 paras should populate the db enough for a PR CI test. @ordian do you think this is enough?

@ordian
Copy link
Member

ordian commented Sep 2, 2022

@pepoviola i think just letting it run for 2-3 sessions with 2 paras should populate the db enough for a PR CI test. @ordian do you think this is enough?

We discussed the test scenario in DMs. But I think we don't need to wait for 2-3 sessions for this, just produce and import a few blocks. The main point to test if we can open the db from the new version to check if there any breaking changes to the db layout.

@sandreim
Copy link
Contributor

sandreim commented Sep 2, 2022

So, no reason to go deeper than the layout?, I was thinking it is worth to also catch breaking changes at the content level - decoding a value fails.

@ordian
Copy link
Member

ordian commented Sep 2, 2022

I was thinking it is worth to also catch breaking changes at the content level - decoding a value fails.

I don't think we need to go for more than a few blocks (+ maybe a dispute) to catch this.

@pepoviola pepoviola merged commit 06eb72d into main Sep 5, 2022
@pepoviola pepoviola deleted the poc-upgrade-node branch September 5, 2022 16:37
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.

4 participants