Run samples against Docker net-tools container #19677

pfl · 2019-10-08T08:16:13Z

Provide a script that runs the sample and starts a 'net-tools' container using the correct sample test Linux binary as counterpart. The Docker container executable will then return a success/not success value, whereby the script can deduce whether the sample completed successfully or not.

zephyrbot · 2019-10-08T10:06:24Z

All checks are passing now.

checkpatch (informational only, not a failure)

-:442: WARNING:LONG_LINE_STRING: line over 80 characters
#442: FILE: scripts/net/run-sample-tests.sh:296:
+	    start_docker "/net-tools/libcoap/examples/etsi_coaptest.sh -i eth0 192.0.2.1"

- total: 0 errors, 1 warnings, 559 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

pfalcon

If the script introduced is supposed to perform heavy-weight operations, like running something in docker, can it please named in way which hints of that, instead of innocent name of "run-sample". If you don't have your own ideas, "run-sample-in-docker" would sound good to me.

samples/net/run-sample.sh

jukkar

Looks good. I think we need to think more about how to override the default IP numbering scheme.

samples/net/sockets/echo_client/prj.conf

scripts/net/run-sample-tests.sh

CODEOWNERS

jukkar

Sanitychecker does not like MIT license, should we change this to Apache-2?

samples/net/sockets/echo_client/Kconfig

jukkar

LGTM

jukkar · 2019-10-30T07:59:29Z

ping @nashif @pfalcon could you re-review the PR please

pfalcon · 2019-10-31T18:42:30Z

I don't see my comment #19677 (review) being addressed or replied to. However if there's no desire to make things easier to users (just today results of Zephyr UX study were presented at the TSC F2F meeting, and the results aren't stellar), let it be so.

Dismissing to avoid blocking from my side

pfl · 2019-11-01T06:38:17Z

I don't see my comment #19677 (review) being addressed or replied to. However if there's no desire to make things easier to users (just today results of Zephyr UX study were presented at the TSC F2F meeting, and the results aren't stellar), let it be so.

Here I tried to keep the naming generic/balanced. We might figure out something else than a Docker container to run the tests in; if so the script needs a major overhaul, but the "familiar" name would be kept. So I'm inclined to keep the name as not very docker specific as it is now.

pfalcon · 2019-11-01T08:32:29Z

@pfl: Sounds good, thanks for reply.

jukkar · 2019-11-04T08:55:27Z

ping @nashif can you recheck and give +1 if ok.

pfl · 2019-11-05T11:46:39Z

ping @nashif were there any concerns still?

nashif · 2019-11-05T12:39:14Z

few things:

Can you squash all commits related to the script in one single commit? There is no value in getting all of this into the tree when the script is being submitted the first time.
There is no documentation or any guidance on how this script can or should be used.
when I run it without any options, I am getting very cryptic output:

scripts/net/run-sample-tests.sh
$ZEPHYR_BASE /home/anashif/zephyrproject/zephyr
$NET_TOOLS_BASE /home/anashif/zephyrproject/zephyr/../net-tools
'zephyr' not supported
Sample 'zephyr' failed with return value '1'
Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Get http://%2Fvar%2Frun%2Fdocker.sock/v1.40/containers/json: dial unix /var/run/docker.sock: connect: permission denied
Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Get http://%2Fvar%2Frun%2Fdocker.sock/v1.40/networks: dial unix /var/run/docker.sock: connect: permission denied

Don't you need to update west.yml to point to net-tools with docker image?

pfl · 2019-11-22T11:30:14Z

Can you squash all commits related to the script in one single commit? There is no value in getting all of this into the tree when the script is being submitted the first time.

Done, there is now one commit for the script itself and another for the echo_client sample.

There is no documentation or any guidance on how this script can or should be used.

Added a README to help with that.

when I run it without any options, I am getting very cryptic output:

Fixed wording of cryptic output, now it mentions that it could not figure out which sample is wanted.

pfl · 2019-11-27T09:24:55Z

ping @nashif ?

The network sample automatic testing feature in PR zephyrproject-rtos#19677 is moved to 2.2 so removing it from 2.1 release note. Signed-off-by: Jukka Rissanen <[email protected]>

The network sample automatic testing feature in PR #19677 is moved to 2.2 so removing it from 2.1 release note. Signed-off-by: Jukka Rissanen <[email protected]>

pfl · 2019-12-12T12:41:57Z

ping @nashif ?

jukkar · 2020-01-07T18:21:56Z

@nashif can you recheck this one?

nashif · 2020-01-07T19:15:24Z

scripts/net/README.txt

+
+ * Install Docker
+ * Check out the net-tools project from github
+ * Change working directory to docker/ in the net-tools repository


is this step needed? I can't run the following command as specified from within the docker/ directory.

Documentation fixed...

nashif · 2020-01-07T19:15:47Z

scripts/net/README.txt

+ * Install Docker
+ * Check out the net-tools project from github
+ * Change working directory to docker/ in the net-tools repository
+ * Run './net-setup.sh --config docker.conf'


Maybe 'sudo ./net-setup.sh --config docker.conf'

The script will do sudo if needed.

nashif · 2020-01-07T19:17:49Z

scripts/net/README.txt

+ * Change working directory to docker/ in the net-tools repository
+ * Run './net-setup.sh --config docker.conf'
+
+This creates a Docker image called 'net-tools' on your local machine.


Hmm, nothing happens here.. I get:

i9:net-tools((HEAD detached at 354fd05)): sudo ./net-setup.sh --config docker.conf Using docker.conf configuration file. Creating zeth Error response from daemon: network with name net-tools0 already exists bridge br- does not exist! br-

This is an issue with net-tools, fix is zephyrproject-rtos/net-tools#40

nashif · 2020-01-07T19:19:14Z

scripts/net/README.txt

+In essence, the following needs to be done:
+
+ * Install Docker
+ * Check out the net-tools project from github


net-tools is part of the west manifest, the current sha in west.yml is old and does not have docker/ directory, you need to update west.yml with the latest net-tools and point the user to it, i.e. ../tools/net-tools. there is no need to checkout a new instance of it.

nashif · 2020-01-07T19:20:27Z

-:322: WARNING:TYPO_SPELLING: 'childs' may be misspelled - perhaps 'children'?

maybe you want to address this as well :)

Add Kconfig option NET_SAMPLE_SEND_ITERATIONS that sets the number of times the Zephyr echo client sample sends its data. By default the value is zero, which means indefinite, and demonstrates the same behavior as before. Signed-off-by: Patrik Flykt <[email protected]>

Net-tools revision 4bff01084d225996e4aae84b98be5969e2f9f33d is needed for running network sample test scripts. Signed-off-by: Patrik Flykt <[email protected]>

Add a script that sets up Docker networking and starts the net-tools Docker container. If successful, run Zephyr with native_posix and execute the appropriate net-tools container executable. The proper net-tools executable and arguments is selected depending on the basename of the sample directory. This script needs to be updated if the net-tools Docker image is updated in some incompatible way. The net-tools directory is assumed to exist at the same level as the Zephyr base directory, but its location can be set using the '-N' command line argument. Likewise, '-Z' sets the Zephyr top level directory. When stopping Zephyr, go through all child processes since running a native posix or other variant of Zephyr may cause a hierarchy of processes to be created. Fixes: zephyrproject-rtos#19540 Signed-off-by: Patrik Flykt <[email protected]>

pfl · 2020-01-24T11:07:45Z

maybe you want to address this as well :)

Yes, it is horrible with misspelled function names :-)

jukkar · 2020-01-24T14:27:47Z

ping @nashif could you recheck?

pfl · 2020-01-27T07:29:09Z

ping @nashif

jhedberg

Looks fine to me. Any chance @nashif that you could take another look before we merge?

pfl · 2020-02-03T07:47:44Z

Ping @nashif ?

pfl requested review from jukkar, pfalcon and tbursztyka as code owners October 8, 2019 08:16

zephyrbot added area: Networking area: Samples Samples labels Oct 8, 2019

pfalcon previously requested changes Oct 8, 2019

View reviewed changes

pfl force-pushed the docker_samples branch from 85cb5f7 to 99c58d6 Compare October 11, 2019 09:26

nashif requested changes Oct 16, 2019

View reviewed changes

samples/net/run-sample.sh Outdated Show resolved Hide resolved

pfl force-pushed the docker_samples branch 2 times, most recently from a4a2370 to 16d5112 Compare October 18, 2019 10:38

jukkar requested changes Oct 18, 2019

View reviewed changes

samples/net/sockets/echo_client/prj.conf Outdated Show resolved Hide resolved

scripts/net/run-sample-tests.sh Outdated Show resolved Hide resolved

scripts/net/run-sample-tests.sh Show resolved Hide resolved

pfl force-pushed the docker_samples branch from de83e8b to dd30f0c Compare October 23, 2019 10:23

nashif reviewed Oct 23, 2019

View reviewed changes

scripts/net/run-sample-tests.sh Show resolved Hide resolved

nashif reviewed Oct 23, 2019

View reviewed changes

CODEOWNERS Outdated Show resolved Hide resolved

pfl force-pushed the docker_samples branch 3 times, most recently from 324786a to b2d8d9b Compare October 28, 2019 11:11

jukkar reviewed Oct 29, 2019

View reviewed changes

samples/net/sockets/echo_client/Kconfig Outdated Show resolved Hide resolved

pfl force-pushed the docker_samples branch from 799b5bc to 04765d8 Compare October 29, 2019 12:54

jukkar approved these changes Oct 30, 2019

View reviewed changes

jukkar requested review from nashif and pfalcon October 30, 2019 07:59

pfl mentioned this pull request Nov 5, 2019

Allow running and testing network samples in automatic way #19540

Closed

5 tasks

pfl force-pushed the docker_samples branch from 04765d8 to b55312c Compare November 22, 2019 11:27

jukkar mentioned this pull request Dec 3, 2019

doc: release: Remove net sample testing feature #21151

Merged

dleach02 pushed a commit that referenced this pull request Dec 3, 2019

doc: release: Remove net sample testing feature

26db793

The network sample automatic testing feature in PR #19677 is moved to 2.2 so removing it from 2.1 release note. Signed-off-by: Jukka Rissanen <[email protected]>

nashif requested changes Jan 7, 2020

View reviewed changes

pfl added 3 commits January 24, 2020 11:04

west: Update required net-tools revision

979abcf

Net-tools revision 4bff01084d225996e4aae84b98be5969e2f9f33d is needed for running network sample test scripts. Signed-off-by: Patrik Flykt <[email protected]>

pfl force-pushed the docker_samples branch from b55312c to c5c48ac Compare January 24, 2020 11:10

zephyrbot added the area: Modules label Jan 24, 2020

jukkar requested a review from nashif January 24, 2020 14:27

jhedberg approved these changes Jan 30, 2020

View reviewed changes

nashif approved these changes Feb 3, 2020

View reviewed changes

nashif merged commit 65ffc71 into zephyrproject-rtos:master Feb 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run samples against Docker net-tools container #19677

Run samples against Docker net-tools container #19677

pfl commented Oct 8, 2019 •

edited

Loading

zephyrbot commented Oct 8, 2019 •

edited

Loading

pfalcon left a comment

jukkar left a comment

jukkar left a comment

jukkar left a comment

jukkar commented Oct 30, 2019

pfalcon commented Oct 31, 2019

pfl commented Nov 1, 2019

pfalcon commented Nov 1, 2019

jukkar commented Nov 4, 2019

pfl commented Nov 5, 2019

nashif commented Nov 5, 2019

pfl commented Nov 22, 2019

pfl commented Nov 27, 2019

pfl commented Dec 12, 2019

jukkar commented Jan 7, 2020

nashif Jan 7, 2020

pfl Jan 24, 2020

nashif Jan 7, 2020

jukkar Jan 7, 2020

nashif Jan 7, 2020

pfl Jan 24, 2020

nashif Jan 7, 2020

pfl Jan 24, 2020

nashif commented Jan 7, 2020

pfl commented Jan 24, 2020

jukkar commented Jan 24, 2020

pfl commented Jan 27, 2020

jhedberg left a comment

pfl commented Feb 3, 2020

Run samples against Docker net-tools container #19677

Run samples against Docker net-tools container #19677

Conversation

pfl commented Oct 8, 2019 • edited Loading

zephyrbot commented Oct 8, 2019 • edited Loading

checkpatch (informational only, not a failure)

pfalcon left a comment

Choose a reason for hiding this comment

jukkar left a comment

Choose a reason for hiding this comment

jukkar left a comment

Choose a reason for hiding this comment

jukkar left a comment

Choose a reason for hiding this comment

jukkar commented Oct 30, 2019

pfalcon commented Oct 31, 2019

pfl commented Nov 1, 2019

pfalcon commented Nov 1, 2019

jukkar commented Nov 4, 2019

pfl commented Nov 5, 2019

nashif commented Nov 5, 2019

pfl commented Nov 22, 2019

pfl commented Nov 27, 2019

pfl commented Dec 12, 2019

jukkar commented Jan 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nashif commented Jan 7, 2020

pfl commented Jan 24, 2020

jukkar commented Jan 24, 2020

pfl commented Jan 27, 2020

jhedberg left a comment

Choose a reason for hiding this comment

pfl commented Feb 3, 2020

pfl commented Oct 8, 2019 •

edited

Loading

zephyrbot commented Oct 8, 2019 •

edited

Loading