> -----Original Message----- > From: Bruce Richardson <bruce.richard...@intel.com> > Sent: Wednesday, September 7, 2022 6:37 PM > To: Juraj Linkeš <juraj.lin...@pantheon.tech> > Cc: tho...@monjalon.net; david.march...@redhat.com; > ronan.rand...@intel.com; honnappa.nagaraha...@arm.com; > ohily...@iol.unh.edu; lijuan...@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 <ohily...@iol.unh.edu> > > Signed-off-by: Juraj Linkeš <juraj.lin...@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 > >
