feat: integration of argoexec rock #10

i-chvets · 2023-09-14T12:59:18Z

Description

Argo exec ROCK rockcraft.yaml was updated in #8
Changes in this PR are part of this ROCK integration efforts. Another related PR canonical/argo-operators#131

Summary of changes:

Added requirements* files.
Added unit tests.
Added tox.ini to setup test environment.

Related issues:
#11

Testing

ROCK building

$ rockcraft pack
Retrieved base bare for amd64                                                                                          
Extracted bare:latest                                                                                                  
Executed: pull base-deps                                                                                               
Executed: pull base-snaps                                                                                              
Executed: pull builder                                                                                                 
Executed: pull copy-files                                                                                              
Executed: pull pebble                                                                                                  
Executed: pull security-team-requirement                                                                               
Executed: overlay base-deps                                                                                            
Executed: overlay base-snaps                                                                                           
Executed: overlay builder                                                                                              
Executed: overlay copy-files                                                                                           
Executed: overlay pebble                                                                                               
Executed: overlay security-team-requirement                                                                            
Executed: build base-deps                                                                                              
Executed: build base-snaps                                                                                             
Executed: skip pull base-deps (already ran)                                                                            
Executed: skip overlay base-deps (already ran)                                                                         
Executed: skip build base-deps (already ran)                                                                           
Executed: stage base-deps (required to build 'builder')                                                                
Executed: build builder                                                                                                
Executed: skip pull builder (already ran)                                                                              
Executed: skip overlay builder (already ran)                                                                           
Executed: skip build builder (already ran)                                                                             
Executed: stage builder (required to build 'copy-files')                                                               
Executed: build copy-files                                                                                             
Executed: build pebble                                                                                                 
Executed: build security-team-requirement                                                                              
Executed: skip stage base-deps (already ran)                                                                           
Executed: stage base-snaps                                                                                             
Executed: skip stage builder (already ran)                                                                             
Executed: stage copy-files                                                                                             
Executed: stage pebble                                                                                                 
Executed: stage security-team-requirement                                                                              
Executed: prime base-deps                                                                                              
Executed: prime base-snaps                                                                                             
Executed: prime builder                                                                                                
Executed: prime copy-files                                                                                             
Executed: prime pebble                                                                                                 
Executed: prime security-team-requirement                                                                              
Executed parts lifecycle                                                                                               
Exported to OCI archive 'argoexec_v3.3.8_22.04_1_amd64.rock'

Unit tests

$ tox -e unit
unit: commands[0]> bash -c 'NAME=$(yq eval .name rockcraft.yaml) && VERSION=$(yq eval .version rockcraft.yaml) && ARCH=$(yq eval ".platforms | keys" rockcraft.yaml | awk -F " " '"'"'{ print $2 }'"'"') && ROCK="${NAME}_${VERSION}_${ARCH}" && sudo skopeo --insecure-policy copy oci-archive:$ROCK.rock docker-daemon:$ROCK:$VERSION && docker save $ROCK > $ROCK.tar'
Getting image source signatures
Copying blob ca1dad5e8d05 done
Copying blob 4ad868852cfc done
Copying blob 1608a994aec6 done
Copying blob 85e553a86153 done
Copying config fa9ff79c40 done
Writing manifest to image destination
Storing signatures
unit: commands[1]> pytest -v --tb native --show-capture=all --log-cli-level=INFO /home/ubuntu/rocks/argo-workflows-rocks/argoexec/tests
================================================= test session starts ==================================================
platform linux -- Python 3.8.10, pytest-7.4.0, pluggy-1.2.0 -- /home/ubuntu/rocks/argo-workflows-rocks/argoexec/.tox/unit/bin/python
cachedir: .tox/unit/.pytest_cache
rootdir: /home/ubuntu/rocks/argo-workflows-rocks/argoexec
plugins: anyio-3.7.1, asyncio-0.21.1, operator-0.29.0
asyncio: mode=strict
collected 1 item                                                                                                       

tests/test_rock.py::test_rock_artefacts 
---------------------------------------------------- live log setup ----------------------------------------------------
INFO     pytest_operator.plugin:plugin.py:647 Adding model uk8s:test-rock-qmr5 on cloud microk8s
PASSED                                                                                                           [100%]
-------------------------------------------------- live log teardown ---------------------------------------------------
INFO     pytest_operator.plugin:plugin.py:783 Model status:

Model           Controller  Cloud/Region        Version  SLA          Timestamp
test-rock-qmr5  uk8s        microk8s/localhost  2.9.44   unsupported  09:04:22-04:00


INFO     pytest_operator.plugin:plugin.py:789 Juju error logs:


INFO     pytest_operator.plugin:plugin.py:877 Resetting model test-rock-qmr5...
INFO     pytest_operator.plugin:plugin.py:882 Not waiting on reset to complete.
INFO     pytest_operator.plugin:plugin.py:855 Forgetting main...


=================================================== warnings summary ===================================================
tests/test_rock.py::test_rock_artefacts
tests/test_rock.py::test_rock_artefacts
  /home/ubuntu/rocks/argo-workflows-rocks/argoexec/.tox/unit/lib/python3.8/site-packages/websockets/protocol.py:218: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
    self._drain_lock = asyncio.Lock(loop=loop)

tests/test_rock.py::test_rock_artefacts
tests/test_rock.py::test_rock_artefacts
  /home/ubuntu/rocks/argo-workflows-rocks/argoexec/.tox/unit/lib/python3.8/site-packages/websockets/protocol.py:977: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
    yield from asyncio.sleep(self.ping_interval, loop=self.loop)

tests/test_rock.py: 21 warnings
  /home/ubuntu/rocks/argo-workflows-rocks/argoexec/.tox/unit/lib/python3.8/site-packages/websockets/protocol.py:911: DeprecationWarning: 'with (yield from lock)' is deprecated use 'async with lock' instead
    with (yield from self._drain_lock):

tests/test_rock.py: 19 warnings
  /home/ubuntu/rocks/argo-workflows-rocks/argoexec/.tox/unit/lib/python3.8/site-packages/websockets/protocol.py:416: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
    yield from asyncio.wait(

tests/test_rock.py::test_rock_artefacts
tests/test_rock.py::test_rock_artefacts
  /home/ubuntu/rocks/argo-workflows-rocks/argoexec/.tox/unit/lib/python3.8/site-packages/websockets/protocol.py:532: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
    yield from asyncio.wait_for(

tests/test_rock.py::test_rock_artefacts
tests/test_rock.py::test_rock_artefacts
  /home/ubuntu/rocks/argo-workflows-rocks/argoexec/.tox/unit/lib/python3.8/site-packages/websockets/protocol.py:554: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
    yield from asyncio.wait_for(

tests/test_rock.py::test_rock_artefacts
tests/test_rock.py::test_rock_artefacts
  /home/ubuntu/rocks/argo-workflows-rocks/argoexec/.tox/unit/lib/python3.8/site-packages/websockets/protocol.py:1077: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
    yield from asyncio.wait_for(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================================== 1 passed, 50 warnings in 11.47s ============================================
  unit: OK (20.66=setup[0.09]+cmd[6.76,13.82] seconds)
  congratulations :) (20.74 seconds)

Summary of changes: - Added unit tests. - Added tox.ini to setup unit test environment.

DnPlas

Thanks @i-chvets !

Regarding this PR, and similar ones:

I agree on the implementation of the "unit" tests, so having this testing environment and running this kind of checks makes sense. I would rather name this differently, we can call them sanity checks or smoke tests.
About this testing environment, I am a bit too concerned with the fact that we have to essentially duplicate the integration testing environment of our charms repos, which means that whatever change we do to workflows in charms repos, we would have to do it in rocks repos.
Also about those integration files, I think we are overkilling this by running one integration tests per rock, I'd rather have one integration test at the top level. Essentially what we discussed the other day.

These two last points will really help us prevent technical debt if we nail them correctly.

i-chvets · 2023-09-20T15:47:22Z

I will remove integration environment for now. We can add it as part of workflow work, if we decide it fits.
I will rename unit into sanity check. Not big fan of word smoke in the code.

Summary of changes: - Removed integration env. - Renamed unit to sanity.

i-chvets · 2023-09-21T17:53:04Z

@DnPlas Ready for final review.

DnPlas

This LGTM, but we need to add at least a workflow to ensure this runs as expected. Are you sending another PR for that?

i-chvets · 2023-09-26T17:44:27Z

This LGTM, but we need to add at least a workflow to ensure this runs as expected. Are you sending another PR for that?

There is a manual testing log.
For workflow, there will be work done under this issue #11

DnPlas

Thanks @i-chvets, some comments.

About this:

There is a manual testing log.
For workflow, there will be work done under this issue #11

I think it is important to see this running in the CI as well, not just manual logs. I would suggest that instead of pointing this PR to main, you point to a feature branch, that way we can merge this PR, send a follow up PR for adding GH workflows and see everything running; otherwise we may run into issues we couldn't catch because we did not see things running.

Also, the issue you are linking in this comment is also linked in the description of the PR.

argoexec/requirements-sanity.in

DnPlas · 2023-09-27T07:08:46Z

argoexec/tests/test_rock.py

+    yield container_name
+
+    try:
+        subprocess.run(["docker", "rm", container_name], check=True, capture_output=True)


There is a docker Python SDK, maybe you want to explore that option

The issue is the size of the dependencies needed for testing. Last time we introduced Chisme for testing and it was rejected, because it was a large dependency. IMO the same rationale applies to this Python Docker SDK.

I don't think that rationale applies here, as this is in fact, a Python script, and not a command that runs inside a tox file. It's up to you if you want to use the SDK, it was a suggestion to make this script cleaner.

argoexec/tox.ini

argoexec/rockcraft.yaml

argoexec/tox.ini

DnPlas

Just a comment on the requirements, other than that LGTM.

argoexec/requirements-sanity.in

DnPlas

Thanks @i-chvets !

feat: integration of argoexec rock

606d9db

Summary of changes: - Added unit tests. - Added tox.ini to setup unit test environment.

i-chvets requested a review from a team as a code owner September 14, 2023 12:59

i-chvets mentioned this pull request Sep 14, 2023

feat: update reference to image to point to rock canonical/argo-operators#131

Merged

Ivan Chvets added 2 commits September 14, 2023 09:58

fix: re-arranged test file

ea929ff

feat: added requirements files

9703a1f

DnPlas reviewed Sep 20, 2023

View reviewed changes

fix: removed integration env

d4baf6f

Summary of changes: - Removed integration env. - Renamed unit to sanity.

DnPlas reviewed Sep 26, 2023

View reviewed changes

DnPlas reviewed Sep 27, 2023

View reviewed changes

fix: updated according to comments

4c948b2

i-chvets changed the base branch from main to kf-4451-feat-workflows September 27, 2023 20:08

Ivan Chvets added 3 commits September 27, 2023 16:15

fix: added readme and cleanup tox.ini

998c54d

fix: set proper version in rockcraft

6830859

fix: typo

5438e4b

DnPlas reviewed Oct 2, 2023

View reviewed changes

argoexec/requirements-sanity.in Outdated Show resolved Hide resolved

fix: removed not needed dependencies

49f0f7e

DnPlas approved these changes Oct 4, 2023

View reviewed changes

i-chvets merged commit 91da61e into kf-4451-feat-workflows Oct 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: integration of argoexec rock #10

feat: integration of argoexec rock #10

i-chvets commented Sep 14, 2023 •

edited

Loading

DnPlas left a comment

i-chvets commented Sep 20, 2023

i-chvets commented Sep 21, 2023

DnPlas left a comment

i-chvets commented Sep 26, 2023

DnPlas left a comment

DnPlas Sep 27, 2023

i-chvets Sep 27, 2023

DnPlas Oct 2, 2023

DnPlas left a comment

DnPlas left a comment

feat: integration of argoexec rock #10

feat: integration of argoexec rock #10

Conversation

i-chvets commented Sep 14, 2023 • edited Loading

Description

Summary of changes:

Testing

DnPlas left a comment

Choose a reason for hiding this comment

i-chvets commented Sep 20, 2023

i-chvets commented Sep 21, 2023

DnPlas left a comment

Choose a reason for hiding this comment

i-chvets commented Sep 26, 2023

DnPlas left a comment

Choose a reason for hiding this comment

DnPlas Sep 27, 2023

Choose a reason for hiding this comment

i-chvets Sep 27, 2023

Choose a reason for hiding this comment

DnPlas Oct 2, 2023

Choose a reason for hiding this comment

DnPlas left a comment

Choose a reason for hiding this comment

DnPlas left a comment

Choose a reason for hiding this comment

i-chvets commented Sep 14, 2023 •

edited

Loading