From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
"david.marchand@redhat.com" <david.marchand@redhat.com>,
"ronan.randles@intel.com" <ronan.randles@intel.com>,
"Honnappa.Nagarahalli@arm.com" <Honnappa.Nagarahalli@arm.com>,
"ohilyard@iol.unh.edu" <ohilyard@iol.unh.edu>,
"lijuan.tu@intel.com" <lijuan.tu@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: RE: [PATCH v4 2/9] dts: add developer tools
Date: Tue, 13 Sep 2022 12:38:46 +0000 [thread overview]
Message-ID: <5fa9cd15e3204c4bbbcaa3b2d53a9736@pantheon.tech> (raw)
In-Reply-To: <YxjIx/47s8yG4nkJ@bricha3-MOBL.ger.corp.intel.com>
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Wednesday, September 7, 2022 6:37 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; david.marchand@redhat.com;
> ronan.randles@intel.com; Honnappa.Nagarahalli@arm.com;
> ohilyard@iol.unh.edu; lijuan.tu@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v4 2/9] dts: add developer tools
>
> On Fri, Jul 29, 2022 at 10:55:43AM +0000, Juraj Linkeš wrote:
> > The Dockerfile contains basic image for CI and developers. There's
> > also an integration of the Dockerfile with Visual Studio.
> >
> > The formatter script uses Black and Isort to format the Python code.
> >
> > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>
> Comments inline below.
>
> Thanks,
> /Bruce
>
> > ---
> > dts/.devcontainer/devcontainer.json | 30 ++++++++++++
> > dts/Dockerfile | 38 +++++++++++++++
> > dts/README.md | 74 ++++++++++++++++++++++++++++-
> > dts/format.sh | 45 ++++++++++++++++++
> > 4 files changed, 186 insertions(+), 1 deletion(-) create mode 100644
> > dts/.devcontainer/devcontainer.json
> > create mode 100644 dts/Dockerfile
> > create mode 100755 dts/format.sh
> >
> > diff --git a/dts/.devcontainer/devcontainer.json
> > b/dts/.devcontainer/devcontainer.json
> > new file mode 100644
> > index 0000000000..41ca28fc17
> > --- /dev/null
> > +++ b/dts/.devcontainer/devcontainer.json
> > @@ -0,0 +1,30 @@
> > +// For format details, see https://aka.ms/devcontainer.json. For config
> options, see the README at:
> > +//
> > +https://github.com/microsoft/vscode-dev-containers/tree/v0.241.1/cont
> > +ainers/docker-existing-dockerfile
> > +{
> > + "name": "Existing Dockerfile",
> > +
> > + // Sets the run context to one level up instead of the .devcontainer
> folder.
> > + "context": "..",
> > +
> > + // Update the 'dockerFile' property if you aren't using the standard
> 'Dockerfile' filename.
> > + "dockerFile": "../Dockerfile",
> > +
> > + // Use 'forwardPorts' to make a list of ports inside the container
> available locally.
> > + // "forwardPorts": [],
> > +
> > + // Uncomment the next line to run commands after the container is
> created - for example installing curl.
> > + "postCreateCommand": "poetry install",
> > +
> > + "extensions": [
> > + "ms-python.vscode-pylance",
> > + ]
> > +
> > + // Uncomment when using a ptrace-based debugger like C++, Go, and
> Rust
> > + // "runArgs": [ "--cap-add=SYS_PTRACE", "--security-opt",
> > +"seccomp=unconfined" ],
> > +
> > + // Uncomment to use the Docker CLI from inside the container. See
> https://aka.ms/vscode-remote/samples/docker-from-docker.
> > + // "mounts": [
> > +"source=/var/run/docker.sock,target=/var/run/docker.sock,type=bind"
> > +],
> > +
> > + // Uncomment to connect as a non-root user if you've added one. See
> https://aka.ms/vscode-remote/containers/non-root.
> > + // "remoteUser": "vscode"
> > +}
> > diff --git a/dts/Dockerfile b/dts/Dockerfile new file mode 100644
> > index 0000000000..6700aa45b8
> > --- /dev/null
> > +++ b/dts/Dockerfile
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > +University of New Hampshire #
> > +
> > +FROM ubuntu:22.04 AS base
> > +
> > +RUN apt-get -y update && apt-get -y upgrade && \
> > + apt-get -y install --no-install-recommends \
> > + python3 \
> > + python3-pip \
> > + python3-pexpect \
> > + python3-poetry \
> > + python3-cachecontrol \
> > + openssh-client
> > +
> > +
> > +FROM base AS runner
> > +
> > +# This container is intended to be used as the base for automated systems.
> > +# It bakes DTS into the container during the build.
> > +
> > +RUN mkdir /dts
> > +COPY ./pyproject.toml /dts/pyproject.toml COPY ./poetry.lock
> > +/dts/poetry.lock WORKDIR /dts RUN poetry install --no-dev COPY . /dts
>
> Two questions here:
> * if we copy over the current folder, does it re-copy the same two files
> above, or do we get a new subfolder with the same name as the current
> one (and the two files in that instead)?
> * Can the commands be re-ordered so that we have all the copies together
> rather than being split either side of the workdir and run commands?
>
Yea, we don't need to copy the two files individually - we only need to copy the whole dts folder. I'll move the commands.
> > +
> > +CMD ["poetry", "run", "python", "main.py"]
> > +
> > +FROM base AS dev
> > +
> > +# This container is intended to be used as a development environment.
> > +
> > +RUN apt-get -y install --no-install-recommends \
> > + vim emacs git
> > +
> If it's to be used as a development environment, do we not need build-essential
> installed?
>
It's meant to be a DTS development environment and we don't need to build anything for that, so no need for build-essential.
> > +WORKDIR /dts
>
> Is this needed twice in the file, since it appears above too?
>
It appears in the definitions of two separate images, but we can actually move it to the base image to have it in the file only once.
> > diff --git a/dts/README.md b/dts/README.md index
> > d8f88f97fe..55a272d767 100644
> > --- a/dts/README.md
> > +++ b/dts/README.md
> > @@ -12,4 +12,76 @@ The Python Version required by DTS is specified in
> > [DTS python config file](./pyproject.toml) in the
> > **[tool.poetry.dependencies]** section. Poetry doesn't install
> > Python, so you may need to satisfy this requirement if your Python is
> > not up to date. A tool such as [Pyenv](https://github.com/pyenv/pyenv)
> > -is a good way to get Python, though not the only one.
> > +is a good way to get Python, though not the only one. However, DTS
> > +includes a development environment in the form of a Docker image.
> > +
> > +# Expected Environment
> > +
> > +The expected execution and development environments for DTS are the
> > +same, the container defined by [Dockerfile](./Dockerfile). Using a
> > +container for the development environment helps with a few things.
> > +
> > +1. It helps enforce the boundary between the tester and the traffic
> > + generator/sut, something which has experienced issues in the past.
>
> s/experienced/caused/
>
Ack.
> > +2. It makes creating containers to run DTS inside automated tooling
> > + much easier, since they can be based off of a known-working environment
> > + that will be updated as DTS is.
> > +3. It abstracts DTS from the server it is running on. This means that the
> > + bare-metal os can be whatever corporate policy or your personal
> preferences
> > + dictate, and DTS does not have to try to support all 15 distros that
> > + are supported by DPDK CI.
>
> Remove the "15".
>
Ack, this will make it accurate even when thing change slightly in the lab.
> > +4. It makes automated testing for DTS easier, since new dependencies can be
> > + sent in with the patches.
> > +5. It fixes the issue of undocumented dependencies, where some test suites
> > + require python libraries that are not installed.
> > +6. Allows everyone to use the same python version easily, even if they are
> > + using an LTS distro or Windows.
>
> Presumably the LTS distro is an *older* LTS distribution with possibly out-of-date
> packages? That should perhaps be made clearer.
>
I'll change it to "even if they are using a distribution or Windows with out-of-date packages", that should be clear enough.
> > +7. Allows you to run the tester on Windows while developing via Docker for
> > + Windows.
> > +
> > +## Tips for setting up a development environment
> > +
> > +### Getting a docker shell
> > +
> > +These commands will give you a bash shell inside the container with
> > +all the python dependencies installed. This will place you inside a
> > +python virtual environment. DTS is mounted via a volume, which is
> > +essentially a symlink from the host to the container. This enables
> > +you to edit and run inside the container and then delete the container when
> you are done, keeping your work.
> > +
> > +```shell
> > +docker build --target dev -t dpdk-dts .
> > +docker run -v $(pwd):/dts -it dpdk-dts bash $ poetry install $ poetry
> > +shell ```
> > +
> > +### Vim/Emacs
> > +
> > +Any editor in the ubuntu repos should be easy to use. You can add
> > +your normal config files as a volume, enabling you to use your preferred
> settings.
> > +
> > +```shell
> > +apt install vim
> > +apt install emacs
> > +```
>
> Were these not already installed in the image created using the dockerfile
> above?
>
They were. I'll remove the install commands and instead add a modified docker command mounting vim config file as volume.
> > +
> > +### Visual Studio Code
> > +
> > +VSCode has first-class support for developing with containers. You
> > +may need to run the non-docker setup commands in the integrated
> > +terminal. DTS contains a .devcontainer config, so if you open the
> > +folder in vscode it should prompt you to use the dev container
> > +assuming you have the plugin installed. Please refer to [VS
> > +Development Containers
> > +Docs](https://code.visualstudio.com/docs/remote/containers)
> > +to set it all up.
> > +
> > +### Other
> > +
> > +Searching for '$IDE dev containers' will probably lead you in the
> > +right direction.
> > +
> > +# Python Formatting
> > +
> > +The tools used to format Python code in DTS are Black and Isort.
> > +There's a shell script, function.sh, which runs the formatters.
> > +Poetry will install these tools, so once you have that set up, you should run it
> before submitting patches.
> > diff --git a/dts/format.sh b/dts/format.sh new file mode 100755 index
> > 0000000000..7d72335470
> > --- /dev/null
> > +++ b/dts/format.sh
> > @@ -0,0 +1,45 @@
> > +#!/usr/bin/env bash
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > +University of New Hampshire # Copyright(c) 2022 PANTHEON.tech s.r.o.
> > +#
> > +
> > +function main() {
> > + # The directory to work on is either passed in as argument 1,
> > + # or is the current working directory
> > + DIRECTORY=${1:-$(pwd)}
> > + LINE_LENGTH=88
> > +
> > + BLACK_VERSION=$(awk '/\[tool.poetry.dev-dependencies\]/,/$^/'
> pyproject.toml |\
> > + grep black | grep -o '[0-9][^"]*')
> > +
> > + PYTHON_VERSION=$(awk '/\[tool.poetry.dependencies\]/,/$^/'
> pyproject.toml |\
> > + grep python | grep -o '[0-9][^"]*' | tr -d '.')
> > +
> > + isort \
> > + --overwrite-in-place \
> > + --profile black \
> > + -j "$(nproc)" \
> > + --line-length $LINE_LENGTH \
> > + --python-version auto \
> > + "$DIRECTORY"
> > +
> > + black \
> > + --line-length $LINE_LENGTH \
> > + --required-version "${BLACK_VERSION}" \
> > + --target-version "py${PYTHON_VERSION}" \
> > + --safe \
> > + "$DIRECTORY"
> > +}
> > +
> > +function help() {
> > + echo "usage: format.sh <directory>"
> > +}
> > +
> > +if [ "$1" == "-h" ] || [ "$1" == "--help" ]; then
> > + help
> > + exit 0
> > +fi
> > +
> > +main "$1"
> > +
> > --
> > 2.30.2
> >
next prev parent reply other threads:[~2022-09-13 12:38 UTC|newest]
Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-22 12:14 [PATCH v1 0/8] dts: ssh connection to a node Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 1/8] dts: add ssh pexpect library Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 2/8] dts: add locks for parallel node connections Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 3/8] dts: add ssh connection extension Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 4/8] dts: add basic logging facility Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 5/8] dts: add Node base class Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 6/8] dts: add config parser module Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 7/8] dts: add dts runtime workflow module Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 8/8] dts: add main script for running dts Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 0/8] ssh connection to a node Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 1/8] dts: add basic logging facility Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 2/8] dts: add ssh pexpect library Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 3/8] dts: add locks for parallel node connections Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 4/8] dts: add ssh connection extension Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 5/8] dts: add config parser module Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 6/8] dts: add Node base class Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 7/8] dts: add dts workflow module Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 8/8] dts: add dts executable script Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 0/9] dts: ssh connection to a node Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 1/9] dts: add project tools config Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 2/9] dts: add developer tools Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 3/9] dts: add basic logging facility Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 4/9] dts: add ssh pexpect library Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 5/9] dts: add ssh connection extension Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 6/9] dts: add config parser module Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 7/9] dts: add Node base class Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 8/9] dts: add dts workflow module Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 9/9] dts: add dts executable script Juraj Linkeš
2022-07-29 10:55 ` [PATCH v4 0/9] dts: ssh connection to a node Juraj Linkeš
2022-07-29 10:55 ` [PATCH v4 1/9] dts: add project tools config Juraj Linkeš
2022-08-10 6:30 ` Tu, Lijuan
2022-09-07 16:16 ` Bruce Richardson
2022-09-09 13:38 ` Juraj Linkeš
2022-09-09 13:52 ` Bruce Richardson
2022-09-09 14:13 ` Juraj Linkeš
2022-09-12 14:06 ` Owen Hilyard
2022-09-12 15:15 ` Bruce Richardson
2022-09-13 12:08 ` Juraj Linkeš
2022-09-13 14:18 ` Bruce Richardson
2022-09-13 19:03 ` Honnappa Nagarahalli
2022-09-13 19:19 ` Honnappa Nagarahalli
2022-09-14 9:37 ` Thomas Monjalon
2022-09-14 12:55 ` Juraj Linkeš
2022-09-14 13:11 ` Bruce Richardson
2022-09-14 14:28 ` Thomas Monjalon
2022-09-21 10:49 ` Juraj Linkeš
2022-09-13 19:11 ` Honnappa Nagarahalli
2022-07-29 10:55 ` [PATCH v4 2/9] dts: add developer tools Juraj Linkeš
2022-08-10 6:30 ` Tu, Lijuan
2022-09-07 16:37 ` Bruce Richardson
2022-09-13 12:38 ` Juraj Linkeš [this message]
2022-09-13 20:38 ` Honnappa Nagarahalli
2022-09-14 7:37 ` Bruce Richardson
2022-09-14 12:45 ` Juraj Linkeš
2022-09-14 13:13 ` Bruce Richardson
2022-09-14 14:26 ` Thomas Monjalon
2022-09-14 19:08 ` Honnappa Nagarahalli
2022-09-20 12:14 ` Juraj Linkeš
2022-09-20 12:22 ` Tu, Lijuan
2022-07-29 10:55 ` [PATCH v4 3/9] dts: add basic logging facility Juraj Linkeš
2022-08-10 6:31 ` Tu, Lijuan
2022-09-08 8:31 ` Bruce Richardson
2022-09-13 12:52 ` Juraj Linkeš
2022-09-13 23:31 ` Honnappa Nagarahalli
2022-09-14 12:51 ` Juraj Linkeš
2022-07-29 10:55 ` [PATCH v4 4/9] dts: add ssh pexpect library Juraj Linkeš
2022-08-10 6:31 ` Tu, Lijuan
2022-09-08 9:53 ` Bruce Richardson
2022-09-13 13:36 ` Juraj Linkeš
2022-09-13 14:23 ` Bruce Richardson
2022-09-13 14:59 ` Stanislaw Kardach
2022-09-13 17:23 ` Owen Hilyard
2022-09-14 0:03 ` Honnappa Nagarahalli
2022-09-14 7:42 ` Bruce Richardson
2022-09-14 7:58 ` Stanislaw Kardach
2022-09-14 19:57 ` Honnappa Nagarahalli
2022-09-19 14:21 ` Owen Hilyard
2022-09-20 17:54 ` Honnappa Nagarahalli
2022-09-21 1:01 ` Tu, Lijuan
2022-09-21 5:37 ` Jerin Jacob
2022-09-22 9:03 ` Juraj Linkeš
2022-09-14 9:42 ` Stanislaw Kardach
2022-09-22 9:41 ` Juraj Linkeš
2022-09-22 14:32 ` Stanislaw Kardach
2022-09-23 7:22 ` Juraj Linkeš
2022-09-23 8:15 ` Bruce Richardson
2022-09-23 10:18 ` Stanislaw Kardach
2022-07-29 10:55 ` [PATCH v4 5/9] dts: add ssh connection extension Juraj Linkeš
2022-08-10 6:32 ` Tu, Lijuan
2022-09-13 17:04 ` Bruce Richardson
2022-09-13 17:32 ` Owen Hilyard
2022-09-14 7:46 ` Bruce Richardson
2022-09-14 12:02 ` Owen Hilyard
2022-09-14 13:15 ` Bruce Richardson
2022-07-29 10:55 ` [PATCH v4 6/9] dts: add config parser module Juraj Linkeš
2022-08-10 6:33 ` Tu, Lijuan
2022-09-13 17:19 ` Bruce Richardson
2022-09-13 17:47 ` Owen Hilyard
2022-09-14 7:48 ` Bruce Richardson
2022-07-29 10:55 ` [PATCH v4 7/9] dts: add Node base class Juraj Linkeš
2022-08-10 6:33 ` Tu, Lijuan
2022-07-29 10:55 ` [PATCH v4 8/9] dts: add dts workflow module Juraj Linkeš
2022-08-10 6:34 ` Tu, Lijuan
2022-07-29 10:55 ` [PATCH v4 9/9] dts: add dts executable script Juraj Linkeš
2022-08-10 6:35 ` Tu, Lijuan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5fa9cd15e3204c4bbbcaa3b2d53a9736@pantheon.tech \
--to=juraj.linkes@pantheon.tech \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=lijuan.tu@intel.com \
--cc=ohilyard@iol.unh.edu \
--cc=ronan.randles@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).