Convert bootstrap shell and bat files to a common Python file. #208

ScottBailey · 2023-06-01T01:21:00Z

Convert bootstrap.sh file to python.
Update tests.
- Added a step to clear the images to ensure testing always runs with a fresh test as well as testing the --release flag.
- Added steps to further test cdt and leap versions and combos.
- Added requirement for tests to be marked either destructive or safe.
- Added slow marker for disabling tests in CI.
Update CI testing workflow.
- Remove bootstrap command, allow tests to generate the image and container.
- Added --run-destructive to pytest command so tests could be developed to execute non-ci or periodically only.
Update docs.
Rename Dockerfile.unix to Dockerfile.
Remove Dockerfile.win (it's a 1:1 duplicate of Dockerfile.unix).

Will close #202

ScottBailey · 2023-06-01T01:22:23Z

@iamveritas Please review (run) OSX tests.

mikelik

That code is so pleasant to read now, thanks Scott.

However it seems, that if we have already existing image then running bootstrap.py doesn't change the leap version.

Example:

1st run works ok:
mikel@msi:~/repo/DUNE$ ./bootstrap.py --leap 3.2.2
[+] Building 320.3s (26/26) FINISHED                                                                                                    
(...) => => naming to docker.io/library/dunes                                                                                           0.0s 

mikel@msi:~/repo/DUNE$ ./dunes --version-all
Creating docker container [dunes_container]
DUNES v1.1.2
Listing... Done
leap/now 3.2.2 amd64 [installed,local]
Listing... Done
cdt/now 4.0.0-1 amd64 [installed,local]

# now the second run seems not to change leap version:
mikel@msi:~/repo/DUNE$ ./bootstrap.py --leap 3.2.3
[+] Building 291.3s (26/26) FINISHED                                                                                                    
(...)
 => => naming to docker.io/library/dunes                                                                                           0.0s 
mikel@msi:~/repo/DUNE$ ./dunes --version-all
DUNES v1.1.2
Listing... Done
leap/now 3.2.2 amd64 [installed,local]
Listing... Done
cdt/now 4.0.0-1 amd64 [installed,local]
mikel@msi:~/repo/DUNE$

In the second bootstrap.py I'm trying to change leap 3.2.2 to leap 3.2.3. However, old 3.2.2 is still installed.

/edit
Tested on Windows. It seems the same issue is for leap on Windows.
However --cdt on Windows worked properly.

bootstrap.bat

bootstrap.py

ScottBailey · 2023-06-01T22:16:03Z

That code is so pleasant to read now, thanks Scott.

However it seems, that if we have already existing image then running bootstrap.py doesn't change the leap version.

/edit Tested on Windows. It seems the same issue is for leap on Windows. However --cdt on Windows worked properly.

I'm not sure you are describing a defect. A running container uses the image it's started with. That's not necessarily something we can or should change...

I have added tests to start multiple containers and test that they include the correct versions of leap and cdt.

tests/z9999_image_bootstrap_test.py

mikelik · 2023-06-02T11:44:39Z

However it seems, that if we have already existing image then running bootstrap.py doesn't change the leap version.

/edit Tested on Windows. It seems the same issue is for leap on Windows. However --cdt on Windows worked properly.

I'm not sure you are describing a defect. A running container uses the image it's started with. That's not necessarily something we can or should change...

You are right, bootstrap shouldn't change the existing container. I have checked this works as expected on Linux and Windows now - thanks.

…t more self contained in image bootstrap test.

mikelik · 2023-06-05T07:31:30Z

tests/z9999_image_bootstrap_test.py

+This script tests that the bootstrap command can create an image with a given version.
+
+This test contains resource intense tests that should NOT be run on every commit.
+To disable during ci testing, add `-k "not non_ci"` to your pytest command (e.g. `pytest -k "not non_ci" tests`)


Suggested change

To disable during ci testing, add `-k "not non_ci"` to your pytest command (e.g. `pytest -k "not non_ci" tests`)

To disable during ci testing, add `-k "not expensive"` to your pytest command (e.g. `pytest -k "not expensive" tests`)

This is overcome by the latest change, I believe.

mikelik · 2023-06-05T08:08:32Z

tests/a100_generate_container_image_test.py

+    images = subprocess.check_output(['docker', 'images', '-q', repo_name], stderr=None, encoding='utf-8').strip().split('\n')
+    for myimg in images:
+        #   pylint: disable=subprocess-run-check
+        subprocess.run(['docker', 'image', 'rm', myimg, '--force'], stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL, check=False)


I think this is very dangerous for the user - it will remove all dunes images, even the running ones.
Remember, we are attaching those tests to the release as well.
I preferred the previous solution, where bootstrap.py had to be executed manually (or by CI script). And even if it was executed, it did not alter running images. What was the issue with it?

A number of the existing tests are also destructive and will destroy a container. I believe the latest commit (with destructive marking) corrects this issue.

iamveritas

I ran python3 bootstrap.py on this PR's branch locally without errors.

Adding conftest.py to disable auto run of desctrictive and slow tests. Updating workflows to run destructive tests, but not slow ones.

docs/RELEASE.md

tests/conftest.py

jolly-fellow · 2023-06-05T20:59:32Z

README.md

@@ -107,7 +107,7 @@ $ echo "PATH=<LocationOfDUNES>:$PATH" >> .bashrc
 If you want to rebuild the DUNES image pick your preferred terminal application and input the following command:

 ```console
-<PathToDUNES>/DUNES$ ./bootstrap.sh
+<PathToDUNES>/DUNES$ python3 ./bootstrap.py


If bootstrap.py has an "execute" permission may be it would be better to remove python3 here?

I think we should probably leave it here for windows users? It doesn't hurt to have it, I don't think.

Co-authored-by: Alexander Molchevsky <[email protected]>

mikelik · 2023-06-06T07:46:32Z

I run bootstrap.py successfully (it has created dunes:latest image) and after that when I'm starting the container:

mikel@msi:~/repo/DUNE$ docker start dunes_container
Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec: "exit": executable file not found in $PATH: unknown
Error: failed to start containers: dunes_container

The same happens when I'm starting tests or running ./dunes --start my_node.
I'm running Ubuntu 20.

mikelik · 2023-06-06T13:00:38Z

I run bootstrap.py successfully (it has created dunes:latest image) and after that when I'm starting the container:
mikel@msi:~/repo/DUNE$ docker start dunes_container
Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec: "exit": executable file not found in $PATH: unknown
Error: failed to start containers: dunes_container
The same happens when I'm starting tests or running ./dunes --start my_node. I'm running Ubuntu 20.

I managed to fix this, so please disregard above comment. I think the reason was that I had some leftovers from tests in src/plugin (probably because I cancelled a running test and it didn't clean up).
Anyway after starting from scratch it works.

mikelik · 2023-06-06T13:17:36Z

How does it work currently if I simply run pytest?
I see that by default test_bootstrap.py is executed if no flag is provided (this test is destructive). I thought it shouldn't be executed by default, should it?

mikelik · 2023-06-06T13:49:14Z

How does it work currently if I simply run pytest? I see that by default test_bootstrap.py is executed if no flag is provided (this test is destructive). I thought it shouldn't be executed by default, should it?

This is fixed in 620ba5f

mikelik

Nice work Scott!

ScottBailey added 5 commits May 31, 2023 15:55

Initial development.

ab901c6

Update tests and docs.

e8fa07c

pylint fixes.

a88356f

Update for bootstrap.py change.

4351213

Small cleanups

732c0ac

ScottBailey requested review from mikelik and jolly-fellow June 1, 2023 01:21

mikelik suggested changes Jun 1, 2023

View reviewed changes

mikelik reviewed Jun 1, 2023

View reviewed changes

bootstrap.bat Show resolved Hide resolved

bootstrap.py Outdated Show resolved Hide resolved

bootstrap.py Show resolved Hide resolved

ScottBailey added 4 commits June 1, 2023 09:20

Suppress the '--refcon' argument for now.

58af79f

Correct the values sent to tag_image() and push_image().

f6936b2

Making test more thorough, BUT only enabling the simple test for CI.

65cc5a8

Improve tests.

ca25f22

ScottBailey requested review from mikelik and iamveritas June 1, 2023 21:53

mikelik reviewed Jun 2, 2023

View reviewed changes

tests/z9999_image_bootstrap_test.py Outdated Show resolved Hide resolved

Small improvements to tests. Changed some names, made some steps a bi…

825d522

…t more self contained in image bootstrap test.

ScottBailey mentioned this pull request Jun 2, 2023

Periodically Run Expensive Tests #211

Closed

aesthetic

6d0fe4c

ScottBailey requested a review from mikelik June 2, 2023 15:50

Merge branch 'main' into sbailey/python_bootstrap

56193b9

mikelik suggested changes Jun 5, 2023

View reviewed changes

mikelik reviewed Jun 5, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

iamveritas approved these changes Jun 5, 2023

View reviewed changes

Adding 'slow' to the expensive tests so they don't run in CI.

22557c3

Adding conftest.py to disable auto run of desctrictive and slow tests. Updating workflows to run destructive tests, but not slow ones.

ScottBailey requested a review from mikelik June 5, 2023 20:14

ScottBailey mentioned this pull request Jun 5, 2023

Fix tests on Windows #212

Merged

jolly-fellow reviewed Jun 5, 2023

View reviewed changes

docs/RELEASE.md Outdated Show resolved Hide resolved

tests/conftest.py Outdated Show resolved Hide resolved

jolly-fellow reviewed Jun 5, 2023

View reviewed changes

ScottBailey and others added 2 commits June 5, 2023 16:13

Address review comment.

a2c2406

Co-authored-by: Alexander Molchevsky <[email protected]>

Take review suggestion for more pythonic array management.

f48a991

Co-authored-by: Alexander Molchevsky <[email protected]>

We found a mislabled test.

620ba5f

mikelik approved these changes Jun 6, 2023

View reviewed changes

ScottBailey merged commit 2628a7f into main Jun 6, 2023

ScottBailey deleted the sbailey/python_bootstrap branch June 6, 2023 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert bootstrap shell and bat files to a common Python file. #208

Convert bootstrap shell and bat files to a common Python file. #208

ScottBailey commented Jun 1, 2023 •

edited

Loading

ScottBailey commented Jun 1, 2023

mikelik left a comment •

edited

Loading

ScottBailey commented Jun 1, 2023 •

edited

Loading

mikelik commented Jun 2, 2023

mikelik Jun 5, 2023

ScottBailey Jun 5, 2023

mikelik Jun 5, 2023

ScottBailey Jun 5, 2023

This comment was marked as duplicate.

iamveritas left a comment

jolly-fellow Jun 5, 2023

ScottBailey Jun 5, 2023

mikelik commented Jun 6, 2023

mikelik commented Jun 6, 2023

mikelik commented Jun 6, 2023

mikelik commented Jun 6, 2023

mikelik left a comment

	To disable during ci testing, add `-k "not non_ci"` to your pytest command (e.g. `pytest -k "not non_ci" tests`)
	To disable during ci testing, add `-k "not expensive"` to your pytest command (e.g. `pytest -k "not expensive" tests`)

Convert bootstrap shell and bat files to a common Python file. #208

Convert bootstrap shell and bat files to a common Python file. #208

Conversation

ScottBailey commented Jun 1, 2023 • edited Loading

ScottBailey commented Jun 1, 2023

mikelik left a comment • edited Loading

Choose a reason for hiding this comment

ScottBailey commented Jun 1, 2023 • edited Loading

mikelik commented Jun 2, 2023

mikelik Jun 5, 2023

Choose a reason for hiding this comment

ScottBailey Jun 5, 2023

Choose a reason for hiding this comment

mikelik Jun 5, 2023

Choose a reason for hiding this comment

ScottBailey Jun 5, 2023

Choose a reason for hiding this comment

This comment was marked as duplicate.

iamveritas left a comment

Choose a reason for hiding this comment

jolly-fellow Jun 5, 2023

Choose a reason for hiding this comment

ScottBailey Jun 5, 2023

Choose a reason for hiding this comment

mikelik commented Jun 6, 2023

mikelik commented Jun 6, 2023

mikelik commented Jun 6, 2023

mikelik commented Jun 6, 2023

mikelik left a comment

Choose a reason for hiding this comment

ScottBailey commented Jun 1, 2023 •

edited

Loading

mikelik left a comment •

edited

Loading

ScottBailey commented Jun 1, 2023 •

edited

Loading