Update SimNet test to the new format #4051

radupopa2010 · 2021-10-11T13:26:30Z

Change path to SimNet image to GCP hub
Rework test according to the new CiHelper format, add description file, clean config
Modify the call of the script according to the new format

grbIzl · 2021-10-18T12:50:33Z

@TriplEight we need to keep this brunch after the merge because of https://github.com/paritytech/polkadot/pull/4051/files#diff-037ea159eb0a7cb0ac23b851e66bee30fb838ee8d0d99fa331a1ba65283d37f7R577

TriplEight · 2021-10-18T17:01:34Z

.gitlab-ci.yml

  VAULT_SERVER_URL:                "https://vault.parity-mgmt-vault.parity.io"
  VAULT_AUTH_PATH:                 "gitlab-parity-io-jwt"
  VAULT_AUTH_ROLE:                 "cicd_gitlab_parity_${CI_PROJECT_NAME}"
+  SIMNET_IMAGE:                    "europe-west3-docker.pkg.dev/parity-simnet/simnet-images/simnet:v14"


to avoid creating a new PR for updating just simnet version I suggest sticking with the previous plan and using SIMNET_REF var which can be easily edited in the GitLab project's variables.
Currently, it's on v8, so I'm guessing it should be changed in the follow-up PR, right?

The intention was to create a new PR for each version of simnet.
I would prefer to see a green pipeline when we change the version of simnet.

then let's don't forget to remove those vars form gitlab UI

TriplEight · 2021-10-18T17:06:01Z

.gitlab-ci.yml

+        --image="PARACHAINSIMAGE=${PARACHAINS_IMAGE_NAME}:${PARACHAINS_IMAGE_TAG}"
+        --image="SYNTHIMAGE=${PARACHAINS_IMAGE_NAME}:${PARACHAINS_IMAGE_TAG}"
+        --image="COLIMAGE=${COLLATOR_IMAGE_NAME}:${COLLATOR_IMAGE_TAG}"
+        --image="SCRIPTSIMAGE=${SIMNET_IMAGE}"


This variables renaming is a bit misleading you know. Doesn't explain anything, just adds more complication.

Let me explain this interaction (CI running simnet tests, using SimNet) in more details. Every tests set (aka test suit) from the point of view of CI is defined by 2 things:

List of docker images, that are required to run tests from this test suit. Every image has its name, that used inside tests, and the value for the image (path with version), that needs to be provided by CI in launch arguments

List of tags, that specifies every test case. Here, i think, the example will be more illustrative. Currently there is only one test suit in Polkadot: test suit with parachains test case. This test case has tags [smoketest, all]. This means, that the developer of this test case assumed, that this test would be run with every smoke test. So it's critical enough and not very-very long. CI runs parachains test suit and says to SimNet, that it wants to run all tests with tag "smoketest" from this test suit. These tags is the attempt to formalize different situations, that we have, when we run simnet tests (for every commit, for every PR, nightly, something else).

You can also have a look, how these two things look like in test suit description file: https://github.com/paritytech/polkadot/pull/4051/files#diff-9e9392f954fd30ebfc3ca84586ab4eb8547ae1ed4bf52cca7e3df8006807c9b6R3

We did have all these substances (required images, tags) before. But they were not explicitly formalized. That made the addition of new tests problematic. I hope with this new format to make CI<->SimNet interactions more clear.

It's good, that you raised this question. It would be great, if other people from CI team also have a look at it. This type of infrastructure (automation of tests automation) is pretty new for me. So every feedback is appreciated

You can also have a look, how these two things look like in test suit description file: https://github.com/paritytech/polkadot/pull/4051/files#diff-9e9392f954fd30ebfc3ca84586ab4eb8547ae1ed4bf52cca7e3df8006807c9b6R3

This is why I thought it's better to skip renaming the variables and maybe even avoid mentioning them in the script's arguments since they're already mentioned in the test spec.

But I thought you're about to substitute the whole bash script invocation with a CLI binary, right? So if this is close, it's OK to leave a little mess here and come up with the optimizations later.

It's not renaming. Inside image flag we provide key-value pairs for all images, required for the tests.

This is too much of unnecessary stuff to keep in mind while using simnet anyway, this requires a simplification.
If this re-renaming the variables, you can also name it a key-value pair, will not be addressed in the next PR, either removed from flags or configs or named similarly throughout the repo, I'm not OK with accepting this.

I am a bit confused. Please let us know what you find difficult to understand and a suggestion how to improve it.
We don't pretend that simnet is very simple to use, but on the other hand it's not more difficult then other tools
we use, let's say helm for example.

If we look at how we use helm to add dynamic values to charts, it is very similar with what we are doing:
https://gitlab.parity.io/parity/simnet/-/blob/master/.maintain/k8s-resources/gitlab/install-runner-polkadot-simnet.sh

helm upgrade polkadot-simnet-gitlab-runner gitlab/gitlab-runner \ --install \ --namespace gitlab-simnet-ci \ --values values-polkadot.yaml \ --set runnerRegistrationToken="${GITLAB_RUNNER_GLOBAL_TOKEN}"

If we would need to set another secret for this helm chart, we would have to use the --set flag one more time.

With the new config, we require to run simnet with 3 flags in total, that repeat for different values

--gh-remote-dir = place from where to download config of the tests --tag = which test case(s) --image = required image(s) for this test

if you look at the content of the file that describes the network:
https://github.com/paritytech/polkadot/blob/387950f5cddcef94cdc516ade6d2a485fb5ea051/simnet_tests/parachains/configs/simple_rococo_testnet.toml
it looks very similar with a definition of a kubernetes pod file, which requires an image to run.
Those images needs to be dynamic, because they are the images that are being tested.
To specify dynamic content in CI we have 2 options that I can think of:

Import image name from environment variable (which is what we are doing)

Edit the network config file with search and replace for the image names before calling simnet

Do you think it would help to replace --image with --env to make more specific that is an environment variable that is being set?

(Discussed these changes offline with Denis)

simnet_tests/README.md

grbIzl · 2021-11-03T10:10:14Z

bot merge

paritytech-processbot · 2021-11-03T10:10:23Z

Error:
Error: When trying to meet the "Project Owners" approval requirements: this pull request does not belong to a project defined in Process.json.

Approval by "Project Owners" is only attempted if other means defined in the criteria for merge are not satisfied first.

The following errors might have affected the outcome of this attempt:

'Batch: Availability and Validity' does not match any projects in polkadot's Process.json
'Batch: Codebase Restructure' does not match any projects in polkadot's Process.json

Merge failed. Check out the criteria for merge. If you're not meeting the approval count, check if the approvers are members of substrateteamleads or core-devs.

TriplEight · 2021-11-03T10:12:18Z

bot merge

paritytech-processbot · 2021-11-03T10:12:26Z

Error:
Error: When trying to meet the "Project Owners" approval requirements: this pull request does not belong to a project defined in Process.json.

Approval by "Project Owners" is only attempted if other means defined in the criteria for merge are not satisfied first.

The following errors might have affected the outcome of this attempt:

'Batch: Availability and Validity' does not match any projects in polkadot's Process.json
'Batch: Codebase Restructure' does not match any projects in polkadot's Process.json

Merge failed. Check out the criteria for merge. If you're not meeting the approval count, check if the approvers are members of substrateteamleads or core-devs.

* master: Update CI image to use the latest rustc (#4200) Offchain: Disable http requests (#4188) Update SimNet test to the new format (#4051) Bump libc from 0.2.105 to 0.2.106 (#4194) Update GHA to s3krit/[email protected] + doc (#4030) Bump dlmalloc from 0.2.1 to 0.2.2 (#4205)

simnet-v10 update simnet image version

0e0faa9

github-actions bot added the A0-please_review Pull request needs code review. label Oct 11, 2021

radupopa2010 added 4 commits October 11, 2021 16:34

update config

3e3902d

update path to js scripts

02b6f38

use current branch to download config for tests

b041e2f

use current branch to download config for tests

3207c7c

radupopa2010 added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Oct 12, 2021

radupopa2010 and others added 4 commits October 12, 2021 16:42

temp use simnet-v10

b526593

update also the toml file

adb7253

Rework test according to CIHelper format

e1fe396

Call cihelper script for running tests

1a79131

grbIzl changed the title ~~simnet-v10 update simnet image version~~ Update SimNet test to v11 Oct 13, 2021

Switch to v12

0d48670

grbIzl changed the title ~~Update SimNet test to v11~~ Update SimNet test to the new format Oct 13, 2021

grbIzl and others added 3 commits October 13, 2021 20:24

Version 13

003f632

simnet v14

cc42b3e

Merge branch 'master' into simnet-v10

2a32030

grbIzl requested a review from TriplEight October 18, 2021 12:50

TriplEight suggested changes Oct 18, 2021

View reviewed changes

Readme extended

387950f

TriplEight approved these changes Oct 29, 2021

View reviewed changes

Merge branch 'master' into simnet-v10

5251809

ordian merged commit 60d3305 into master Nov 3, 2021

ordian deleted the simnet-v10 branch November 3, 2021 10:34

grbIzl restored the simnet-v10 branch November 3, 2021 12:02

grbIzl mentioned this pull request Nov 4, 2021

Change path for simnet tests to master #4223

Merged

acatangiu deleted the simnet-v10 branch December 12, 2022 10:23

Update SimNet test to the new format #4051

Update SimNet test to the new format #4051

Uh oh!

Conversation

radupopa2010 commented Oct 11, 2021 • edited by grbIzl Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grbIzl commented Oct 18, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radupopa2010 Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

grbIzl commented Nov 3, 2021

Uh oh!

paritytech-processbot bot commented Nov 3, 2021

Uh oh!

TriplEight commented Nov 3, 2021

Uh oh!

paritytech-processbot bot commented Nov 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

radupopa2010 commented Oct 11, 2021 •

edited by grbIzl

Loading

radupopa2010 Oct 20, 2021 •

edited

Loading