Feature/ppm 30 boardfarm docker compose #1587

odkq · 2020-08-06T16:27:18Z

This branch wraps the boardfarm tests using a python 3.5 script, dctest.py, that runs docker-compose.

The script allows for setting up the bridge with the physical device and cleaning up testing images seamlessly.

As the script is not running inside docker itself, it only uses Python 3.5 (introduced in 2015) features and no external libraries.

Use directly the hostname instead of querying for the ip Signed-off-by: pablo <[email protected]>

Entrypoint calls the json for "in compose" tests Signed-off-by: pablo <[email protected]>

Dockerfile to create boardfarm docker image and docker-compose.yml with the configuration for boardfarm, controller and extender Signed-off-by: pablo <[email protected]>

It wraps docker-compose tests - Verifies versions of docker and docker-compose - Creates and launches boardfarm image with the correct environment variables - Cleans images after building - Returns value from boardfarm process inside the image Signed-off-by: pablo <[email protected]>

Signed-off-by: pablo <[email protected]>

The status are failing in CI. The name of the container was hardcoded by mistake, use docker_name instead. Signed-off-by: Raphaël Mélotte <[email protected]>

Signed-off-by: odkq <[email protected]> Signed-off-by: Raphaël Mélotte <[email protected]>

rmelotte

Lots of small comments, but I marked most of them as nitpicks because we anyway need a follow-up PR to remove the old boardfarm job among other things.

dctest.py

tests/environment.py

tools/docker/boardfarm-ci/Dockerfile

tools/docker/boardfarm-ci/docker-compose.yml

rmelotte · 2020-08-10T15:34:23Z

Also I didn't mention it, but flake8 is complaining so you'll have to address that before you can merge: https://gitlab.com/prpl-foundation/prplMesh/-/jobs/674432838

rmelotte · 2020-08-10T15:57:24Z

I also didn't repeat it here (yet?) but in the future please try to describe what you do in the commit description.
In addition to the contributing guide you can check in the git history to see what we usually do (reviewing other pull request is also a good way to get familiar with it :-) )

Signed-off-by: pablo <[email protected]>

Reordered the file with this ordering, from the less likely to change to the most: - Dependencies to install docker and docker-compose - Installation of docker and docker-compose - Installation of boardfarm (from github) - Debian dependencies to install python packages (for tests) - Installation of the python packages (for tests) - Debian dependencies used by the tests themselves Signed-off-by: pablo <[email protected]>

This change scans prplmesh_config_compose.js for device names and creates the log directory for each of them found. The scanning is also used to look for the previous run ID when running locally, by looking for logs/<first device name>-<X>, and then using X+1 as the new ID. Signed-off-by: pablo <[email protected]>

rmelotte · 2020-08-11T16:11:06Z

The boardfarm tests were passing (in the sense that there were 2 failing tests and 2 passing ones, the 2 failing ones were disabled on master) with this commit:
boardfarm-ci: Commented and reordered Dockerfile : https://gitlab.com/prpl-foundation/prplMesh/-/jobs/680364644

Then all 4 started failing, among which initial AP config failed with Can't find '\(WSC M2 Encrypted Settings\)' with this commit:
boardfarm-ci: Scan device names: https://gitlab.com/prpl-foundation/prplMesh/-/jobs/680577567

run-tests however has been failing since the beginning of this branch's history.

abelog

Not approving, as tests/boardfarm_plugins/boardfarm_prplmesh/devices/prplmesh_compose.py::prprlmesh_status_check should be fixed before merge of PR in my opinon. It may be reason for fails mentioned in @rmelotte comment.

dctest.py

tests/boardfarm_plugins/boardfarm_prplmesh/devices/prplmesh_compose.py

abelog · 2020-08-12T10:57:19Z

tests/environment.py

-    # NOTE: name arg can be also extracted from the device class itself, but test_flows.py
-    # don't have it. We can remove this arg as soon, as we drop test_flows.py
-    def __init__(self, name: str, device: None = None, is_controller: bool = False):
+    def __init__(self, name: str, device: None = None, is_controller: bool = False,


This comment is still required.

tools/docker/boardfarm-ci/Dockerfile

Signed-off-by: pablo <[email protected]>

Use relative path on docker-compose invocation, not using /usr/local/bin/... Signed-off-by: pablo <[email protected]>

This prevents more than one --clean, --test, --build, etc being ran at the same time Signed-off-by: pablo <[email protected]>

When working on the docker-compose version of the boardfarm tests, we thought the ALEntity names had to be the same as the name boardfarm uses for boardfarm to be able to access them. It turns out that boardfarm only needs PrplMeshCompose to get its name from the boardfarm configuration file, and the ALEntity can use another one. Since test_flows doesn't pass a "device" parameter when constructing an ALEntity, it HAS to use "name" instead of going through the device to access the docker_name. environment.py: - Use the name of the ALEntityDocker and RadioDocker in instead of accessing "device". prplmesh_compose.py: - When creating the ALEntity, use docker_name as the name to make the different docker commands work. This will make the ALEntity usable again regardless of whether the tests runs with boardfarm or test_flows.py. Signed-off-by: Raphaël Mélotte <[email protected]>

With apt-get upgrade before Signed-off-by: pablo <[email protected]>

Renamed CURRENT_UID to CURRENT_UID_GID as suggested and removed CURRENT_ID wich was not used by agent/controller images Signed-off-by: pablo <[email protected]>

As suggested from PR comment. Signed-off-by: pablo <[email protected]>

Signed-off-by: pablo <[email protected]>

Pass self.docker_name instead of fixed "controller-rme" when performing device_get_info, as suggested by PR comment. Signed-off-by: pablo <[email protected]>

On prplmesh_compose entry of prplmesh_config_compose.json Signed-off-by: pablo <[email protected]>

Was removed but it is still pertinent Signed-off-by: pablo <[email protected]>

Signed-off-by: pablo <[email protected]>

aciliketcap · 2020-08-14T07:25:26Z

tools/docker/boardfarm-ci/Dockerfile

+software-properties-common curl gcc libsnmp-dev wireshark-common tshark \
+-y && apt-get clean


Do we still need wireshark and tshark inside the boardfarm-ci image? We already know that sniffer doesn't work when we try that way. Maybe we can keep it and I'll exclude it in the PR fixing PPM-331.

Yes, I can think it is better removed on the same branch that is depending on having dumpcap installed alongside python 3.5 on the host for dctest to work (and checking the dependency -- that the executable exists -- as it is done for docker and docker-compose)

Missing the n when restoring the original format string with \n ... Signed-off-by: pablo <[email protected]>

An unused include and some spacess missing for PEP-8 compliance Signed-off-by: pablo <[email protected]>

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Merging master into PPM-30-boardfarm-docker-compose Fixing two conflicts in tests/environment.py Signed-off-by: pablo <[email protected]>

Signed-off-by: pablo <[email protected]>

odkq and others added 9 commits August 6, 2020 17:17

PPM-30: Changes on class ALEntityDocker for compose

2a778b5

Use directly the hostname instead of querying for the ip Signed-off-by: pablo <[email protected]>

PPM-30: Define test entrypoint and json config

9f26b8a

Entrypoint calls the json for "in compose" tests Signed-off-by: pablo <[email protected]>

PPM-30: Dockerfile and docker-compose.yml

bd59982

Dockerfile to create boardfarm docker image and docker-compose.yml with the configuration for boardfarm, controller and extender Signed-off-by: pablo <[email protected]>

PPM-30: Add test to boardfarm-ci

856854b

Signed-off-by: pablo <[email protected]>

PPM-30: Check return error on _run_shell

bb1645d

Signed-off-by: pablo <[email protected]>

Added device prplmesh_compose.py

3d819a8

Signed-off-by: pablo <[email protected]>

boardfarm: use the name of the container for the status check

c108e2f

The status are failing in CI. The name of the container was hardcoded by mistake, use docker_name instead. Signed-off-by: Raphaël Mélotte <[email protected]>

Use original name and docker-name for hostname in environment.py

7ec7a92

Signed-off-by: odkq <[email protected]> Signed-off-by: Raphaël Mélotte <[email protected]>

odkq requested review from aciliketcap and abelog August 6, 2020 16:27

odkq requested review from adam1985d, arnout, kantera800, mariomaz, morantr, rmelotte, tomereli and vitalybu as code owners August 6, 2020 16:27

github-actions bot assigned odkq Aug 6, 2020

rmelotte mentioned this pull request Aug 10, 2020

Ppm 30 #1583

Closed

rmelotte approved these changes Aug 10, 2020

View reviewed changes

odkq added 3 commits August 11, 2020 10:32

boardfarm-ci: Remove unused code

61cc3b7

Signed-off-by: pablo <[email protected]>

abelog reviewed Aug 12, 2020

View reviewed changes

odkq added 2 commits August 12, 2020 13:40

boardfarm-ci: Fixes for PEP-8 compliance

d53c32a

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Remove unused code

a0d8fe7

Signed-off-by: pablo <[email protected]>

odkq force-pushed the feature/PPM-30-boardfarm-docker-compose branch from f383add to a0d8fe7 Compare August 12, 2020 11:41

odkq and others added 16 commits August 12, 2020 13:42

boardfarm-ci: Remove commented out line

727d836

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Removed more unused code

9355d53

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Removed commented out code

44af58b

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Removed commented-out code

28e53f2

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Use relative path

1df2526

Use relative path on docker-compose invocation, not using /usr/local/bin/... Signed-off-by: pablo <[email protected]>

boardfarm-ci: Added mutual exclusion in argv

0042f0e

This prevents more than one --clean, --test, --build, etc being ran at the same time Signed-off-by: pablo <[email protected]>

boardfarm-ci: Group all apt-get commands together

a999708

With apt-get upgrade before Signed-off-by: pablo <[email protected]>

boardfarm-ci: Removed unused CURRENT_ID

a0ae26c

Renamed CURRENT_UID to CURRENT_UID_GID as suggested and removed CURRENT_ID wich was not used by agent/controller images Signed-off-by: pablo <[email protected]>

boardfarm-ci: Commented on issue PPM-208

157e5e4

As suggested from PR comment. Signed-off-by: pablo <[email protected]>

boardfarm-ci: Better error message on _run_cmd

344ebfe

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Fixed check_status

b4adb53

Pass self.docker_name instead of fixed "controller-rme" when performing device_get_info, as suggested by PR comment. Signed-off-by: pablo <[email protected]>

boardfarm-ci: Set delay to the default 7 seconds

d0e1415

On prplmesh_compose entry of prplmesh_config_compose.json Signed-off-by: pablo <[email protected]>

boardfarm-ci: Restored comment on ALEntityDocker's contructor

c34ceeb

Was removed but it is still pertinent Signed-off-by: pablo <[email protected]>

boardfarm-ci: Added license header to dctest.py

599fc48

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Reorder lines to improve readability

356793b

Signed-off-by: pablo <[email protected]>

rmelotte mentioned this pull request Aug 13, 2020

Feature/issue 1381 boardfarm in gitlab ci #1483

Closed

aciliketcap reviewed Aug 14, 2020

View reviewed changes

odkq added 6 commits August 14, 2020 11:57

boardfarm-ci: Fixed typo

af13310

Missing the n when restoring the original format string with \n ... Signed-off-by: pablo <[email protected]>

boardfarm-ci: Aesthetical changes to pass PEP-8

09e27fb

An unused include and some spacess missing for PEP-8 compliance Signed-off-by: pablo <[email protected]>

boardfarm-ci: Resolved merge conflict in Dockerfile

a13b772

Signed-off-by: pablo <[email protected]>

Merge branch 'master' into feature/PPM-30-boardfarm-docker-compose

62a8125

boardfarm-ci: Merging master into PPM-30-boardfarm-docker-compose Fixing two conflicts in tests/environment.py Signed-off-by: pablo <[email protected]>

boardfarm-ci: Added missing jq

edc4b0a

Signed-off-by: pablo <[email protected]>

boardfarm-ci: Fixes for the new prplmesh/prplmesh URL

df60f75

Signed-off-by: pablo <[email protected]>

odkq requested review from abelog and aciliketcap August 14, 2020 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/ppm 30 boardfarm docker compose #1587

Feature/ppm 30 boardfarm docker compose #1587

odkq commented Aug 6, 2020 •

edited

Loading

rmelotte left a comment

rmelotte commented Aug 10, 2020

rmelotte commented Aug 10, 2020

rmelotte commented Aug 11, 2020 •

edited

Loading

abelog left a comment

abelog Aug 12, 2020

aciliketcap Aug 14, 2020

odkq Aug 14, 2020

		software-properties-common curl gcc libsnmp-dev wireshark-common tshark \
		-y && apt-get clean

Feature/ppm 30 boardfarm docker compose #1587

Are you sure you want to change the base?

Feature/ppm 30 boardfarm docker compose #1587

Conversation

odkq commented Aug 6, 2020 • edited Loading

rmelotte left a comment

Choose a reason for hiding this comment

rmelotte commented Aug 10, 2020

rmelotte commented Aug 10, 2020

rmelotte commented Aug 11, 2020 • edited Loading

abelog left a comment

Choose a reason for hiding this comment

abelog Aug 12, 2020

Choose a reason for hiding this comment

aciliketcap Aug 14, 2020

Choose a reason for hiding this comment

odkq Aug 14, 2020

Choose a reason for hiding this comment

odkq commented Aug 6, 2020 •

edited

Loading

rmelotte commented Aug 11, 2020 •

edited

Loading