[v6,00/10] dts: ssh connection to a node

  All the necessary code needed to connect to a node in a topology with
a bit more, such as basic logging and some extra useful methods.

To run the code, modify the config file, conf.yaml and execute ./main.py
from the root dts folder. Here's an example config:
executions:
  - system_under_test: "SUT 1"
nodes:
  - name: "SUT 1"
    hostname: 127.0.0.1
    user: root

The framework will use the user's SSH key to authenticate. User password
can be specified, in which case it will be used, but it's strongly
discouraged.

There are configuration files with a README that help with setting up
the execution/development environment.

The code only connects to a node. You'll see logs emitted to console
saying where DTS connected.

There's only a bit of documentation, as there's not much to document.
We'll add some real docs when there's enough functionality to document,
when the HelloWorld testcases is in (point 4 in our roadmap below). What
will be documented later is runtime dependencies and how to set up the DTS
control node environment.

This is our current roadmap:
1. Review this patchset and do the rest of the items in parallel, if
possible.
2. We have extracted the code needed to run the most basic testcase,
HelloWorld, which runs the DPDK Hello World application. We'll split
this along logical/functional boundaries and send after 1 is done.
3. Once we have 2 applied, we're planning on adding a basic functional
testcase - pf_smoke. This send a bit of traffic, so the big addition is
the software traffic generator, Scapy. There's some work already done on
Traffic generators we'll be sending as a dependence on this patch
series.
4. After 3, we'll add a basic performance testcase which doesn't use
Scapy, but Trex or Ixia instead.
5. This is far in the future, but at this point we should have all of
the core functionality in place. What then remains is adding the rest of
the testcases.

We're already working on items 2-4 and we may send more patches even
before this patch series is accepted if that's beneficial. The new
patches would then depend on this patch.

This patch, as well as all others in the pipeline, are the result of
extensive DTS workgroup review which happens internally. If you'd like
us to make it more public we'd have no problem with that.

v3:
Added project config files and developer tools.
Removed locks for parallel nodes, which are not needed now and will be
implemented much later (in a different patch).

v4:
Minor fixes - added missing Exception and utils function.

v5:
Reordered commits because the dependencies between commits changed.
Added more developer tools.
Added definitions of DTS testbed elements.
Reworked SSH implementation - split it so that the split between an
abstraction and the actual implementation is clearer.
Modified the directory structure to better organize the current and the
future code.

v6:
Minor code/grammar/style changes and a minor bugfix suggested by
Stanislaw.

Juraj Linkeš (9):
  dts: add project tools config
  dts: add developer tools
  dts: add basic logging facility
  dts: add remote session abstraction
  dts: add ssh session module
  dts: add node base class
  dts: add dts workflow module
  dts: add dts executable script
  maintainers: add dts maintainers

Owen Hilyard (1):
  dts: add config parser module

 .editorconfig                                 |   2 +-
 .gitignore                                    |   9 +-
 MAINTAINERS                                   |   5 +
 devtools/python-checkpatch.sh                 |  39 ++
 devtools/python-format.sh                     |  54 +++
 devtools/python-lint.sh                       |  26 ++
 doc/guides/contributing/coding_style.rst      |   4 +-
 dts/.devcontainer/devcontainer.json           |  30 ++
 dts/Dockerfile                                |  39 ++
 dts/README.md                                 | 154 ++++++++
 dts/conf.yaml                                 |   6 +
 dts/framework/__init__.py                     |   4 +
 dts/framework/config/__init__.py              | 100 +++++
 dts/framework/config/conf_yaml_schema.json    |  65 ++++
 dts/framework/dts.py                          |  68 ++++
 dts/framework/exception.py                    |  57 +++
 dts/framework/logger.py                       | 114 ++++++
 dts/framework/remote_session/__init__.py      |  15 +
 .../remote_session/remote_session.py          | 100 +++++
 dts/framework/remote_session/ssh_session.py   | 185 +++++++++
 dts/framework/settings.py                     | 119 ++++++
 dts/framework/testbed_model/__init__.py       |   8 +
 dts/framework/testbed_model/node.py           |  63 ++++
 dts/framework/utils.py                        |  31 ++
 dts/main.py                                   |  24 ++
 dts/poetry.lock                               | 351 ++++++++++++++++++
 dts/pyproject.toml                            |  55 +++
 27 files changed, 1723 insertions(+), 4 deletions(-)
 create mode 100755 devtools/python-checkpatch.sh
 create mode 100755 devtools/python-format.sh
 create mode 100755 devtools/python-lint.sh
 create mode 100644 dts/.devcontainer/devcontainer.json
 create mode 100644 dts/Dockerfile
 create mode 100644 dts/README.md
 create mode 100644 dts/conf.yaml
 create mode 100644 dts/framework/__init__.py
 create mode 100644 dts/framework/config/__init__.py
 create mode 100644 dts/framework/config/conf_yaml_schema.json
 create mode 100644 dts/framework/dts.py
 create mode 100644 dts/framework/exception.py
 create mode 100644 dts/framework/logger.py
 create mode 100644 dts/framework/remote_session/__init__.py
 create mode 100644 dts/framework/remote_session/remote_session.py
 create mode 100644 dts/framework/remote_session/ssh_session.py
 create mode 100644 dts/framework/settings.py
 create mode 100644 dts/framework/testbed_model/__init__.py
 create mode 100644 dts/framework/testbed_model/node.py
 create mode 100644 dts/framework/utils.py
 create mode 100755 dts/main.py
 create mode 100644 dts/poetry.lock
 create mode 100644 dts/pyproject.toml
  

On Thu, Oct 13, 2022 at 10:35:07AM +0000, Juraj Linkeš wrote:
> All the necessary code needed to connect to a node in a topology with
> a bit more, such as basic logging and some extra useful methods.
> 
> To run the code, modify the config file, conf.yaml and execute ./main.py
> from the root dts folder. Here's an example config:
> executions:
>   - system_under_test: "SUT 1"
> nodes:
>   - name: "SUT 1"
>     hostname: 127.0.0.1
>     user: root
> 
> The framework will use the user's SSH key to authenticate. User password
> can be specified, in which case it will be used, but it's strongly
> discouraged.
> 
> There are configuration files with a README that help with setting up
> the execution/development environment.
> 
> The code only connects to a node. You'll see logs emitted to console
> saying where DTS connected.
> 
> There's only a bit of documentation, as there's not much to document.
> We'll add some real docs when there's enough functionality to document,
> when the HelloWorld testcases is in (point 4 in our roadmap below). What
> will be documented later is runtime dependencies and how to set up the DTS
> control node environment.
> 
> This is our current roadmap:
> 1. Review this patchset and do the rest of the items in parallel, if
> possible.
> 2. We have extracted the code needed to run the most basic testcase,
> HelloWorld, which runs the DPDK Hello World application. We'll split
> this along logical/functional boundaries and send after 1 is done.
> 3. Once we have 2 applied, we're planning on adding a basic functional
> testcase - pf_smoke. This send a bit of traffic, so the big addition is
> the software traffic generator, Scapy. There's some work already done on
> Traffic generators we'll be sending as a dependence on this patch
> series.
> 4. After 3, we'll add a basic performance testcase which doesn't use
> Scapy, but Trex or Ixia instead.
> 5. This is far in the future, but at this point we should have all of
> the core functionality in place. What then remains is adding the rest of
> the testcases.
> 
> We're already working on items 2-4 and we may send more patches even
> before this patch series is accepted if that's beneficial. The new
> patches would then depend on this patch.
> 
> This patch, as well as all others in the pipeline, are the result of
> extensive DTS workgroup review which happens internally. If you'd like
> us to make it more public we'd have no problem with that.
> 

Thanks for all the work on this, it's good to see DTS making its way slowly
towards mainline DPDK.

Series-acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

I was about to merge this series,
and after long thoughts, it deserves a bit more changes.
I would like to work with you for a merge in 22.11-rc3.

13/10/2022 12:35, Juraj Linkeš:
> All the necessary code needed to connect to a node in a topology with
> a bit more, such as basic logging and some extra useful methods.

There is also some developer tooling,
and some documentation.

[...]
> There are configuration files with a README that help with setting up
> the execution/development environment.

I don't want to merge some doc which is not integrated
in the doc/ directory.
It should be in RST format in doc/guides/dts/
I can help with this conversion.

> The code only connects to a node. You'll see logs emitted to console
> saying where DTS connected.
> 
> There's only a bit of documentation, as there's not much to document.
> We'll add some real docs when there's enough functionality to document,
> when the HelloWorld testcases is in (point 4 in our roadmap below). What
> will be documented later is runtime dependencies and how to set up the DTS
> control node environment.
> 
[...]
>  .editorconfig                                 |   2 +-
>  .gitignore                                    |   9 +-

Updating general Python guidelines in these files
should be done separately to get broader agreement.

>  MAINTAINERS                                   |   5 +

You can update this file in the first patch.

>  devtools/python-checkpatch.sh                 |  39 ++

Let's postpone the integration of checkpatch.
It should be integrated with the existing checkpatch.

>  devtools/python-format.sh                     |  54 +++
>  devtools/python-lint.sh                       |  26 ++

Let's postpone the integration of these tools.
We need to discuss what is specific to DTS or not.

>  doc/guides/contributing/coding_style.rst      |   4 +-

It is not specific to DTS.

>  dts/.devcontainer/devcontainer.json           |  30 ++
>  dts/Dockerfile                                |  39 ++

Not sure about Docker tied to some personal choices.

>  dts/README.md                                 | 154 ++++++++

As said above, it should in RST format in doc/guides/dts/

>  dts/conf.yaml                                 |   6 +
>  dts/framework/__init__.py                     |   4 +
>  dts/framework/config/__init__.py              | 100 +++++
>  dts/framework/config/conf_yaml_schema.json    |  65 ++++
>  dts/framework/dts.py                          |  68 ++++
>  dts/framework/exception.py                    |  57 +++
>  dts/framework/logger.py                       | 114 ++++++
>  dts/framework/remote_session/__init__.py      |  15 +
>  .../remote_session/remote_session.py          | 100 +++++
>  dts/framework/remote_session/ssh_session.py   | 185 +++++++++
>  dts/framework/settings.py                     | 119 ++++++
>  dts/framework/testbed_model/__init__.py       |   8 +
>  dts/framework/testbed_model/node.py           |  63 ++++
>  dts/framework/utils.py                        |  31 ++
>  dts/main.py                                   |  24 ++
>  dts/poetry.lock                               | 351 ++++++++++++++++++

A lot of dependencies look not useful in this first series for SSH connection.

>  dts/pyproject.toml                            |  55 +++
>  27 files changed, 1723 insertions(+), 4 deletions(-)
  

On Mon, Oct 31, 2022 at 3:01 PM Thomas Monjalon <thomas@monjalon.net> wrote:

> I was about to merge this series,
> and after long thoughts, it deserves a bit more changes.
> I would like to work with you for a merge in 22.11-rc3.
>
> 13/10/2022 12:35, Juraj Linkeš:
> > All the necessary code needed to connect to a node in a topology with
> > a bit more, such as basic logging and some extra useful methods.
>
> There is also some developer tooling,
> and some documentation.
>
> [...]
> > There are configuration files with a README that help with setting up
> > the execution/development environment.
>
> I don't want to merge some doc which is not integrated
> in the doc/ directory.
> It should be in RST format in doc/guides/dts/
> I can help with this conversion.
>
> > The code only connects to a node. You'll see logs emitted to console
> > saying where DTS connected.
> >
> > There's only a bit of documentation, as there's not much to document.
> > We'll add some real docs when there's enough functionality to document,
> > when the HelloWorld testcases is in (point 4 in our roadmap below). What
> > will be documented later is runtime dependencies and how to set up the
> DTS
> > control node environment.
> >
> [...]
> >  .editorconfig                                 |   2 +-
> >  .gitignore                                    |   9 +-
>
> Updating general Python guidelines in these files
> should be done separately to get broader agreement.
>
> >  MAINTAINERS                                   |   5 +
>
> You can update this file in the first patch.
>
> >  devtools/python-checkpatch.sh                 |  39 ++
>
> Let's postpone the integration of checkpatch.
> It should be integrated with the existing checkpatch.
>

We wanted to specifically ensure that all code met the quality requirements
for DTS from the start. The current formatting requirements are "whatever
these tools run in this order with these settings results in", since the
working group decided that fully automated rectification of minor issues
would help new contributors focus on more important things. I agree that
integrating it into existing checkpatch would be good, but I think that to
do that we would first need to run the tool over existing DPDK python code.
python-checkpatch does do linting like the main checkpatch, but it will
also perform source code changes to automatically fix many formatting
issues. Do we want to keep checkpatch as a read-only script and introduce
another one which makes source-code changes, or merge them together? This
would also mean that all DPDK developers would need to run checkpatch
inside of a python virtual environment since DTS is currently very specific
about what versions are used (via both version number and cryptographic
hash of the source archive). These versions are newer than what are shipped
in many stable linux distributions (Ubuntu, Debian, etc), because we want
the additional capabilities that come with the newer versions and the
version in the Debian Buster repos is old enough that it does not support
the version of python that we are using. We were trying to avoid forcing
all DPDK developers to install a full suite of python tools.

>  devtools/python-format.sh                     |  54 +++
> >  devtools/python-lint.sh                       |  26 ++
>
> Let's postpone the integration of these tools.
> We need to discuss what is specific to DTS or not.
>
> >  doc/guides/contributing/coding_style.rst      |   4 +-
>
> It is not specific to DTS.
>
> >  dts/.devcontainer/devcontainer.json           |  30 ++
> >  dts/Dockerfile                                |  39 ++
>
> Not sure about Docker tied to some personal choices.
>

The reason we are using docker is that it provides a canonical environment
to run the test harness in, which can then connect to the SUT and traffic
generator. It enforces isolation between these three components, and
ensures that everyone is using the exact same environment to test and
deploy the test harness. Although devcontainer started in Visual Studio
Code, it is supported by many IDEs at this point and we wanted to encourage
developers along a path which avoids needing to set up a development
environment before they can start working. The very recent version of
python (3.10) we choose to use for additional type system verification
makes not using containers much harder. It also means that new
dependencies, new dependency versions or additional system dependencies can
be easily handled by the normal patch process and rolled out to everyone in
a mostly automated fashion.

Additionally, although it is named Dockerfile, it is fully compatible with
podman.


> >  dts/README.md                                 | 154 ++++++++
>
> As said above, it should in RST format in doc/guides/dts/
>
> >  dts/conf.yaml                                 |   6 +
> >  dts/framework/__init__.py                     |   4 +
> >  dts/framework/config/__init__.py              | 100 +++++
> >  dts/framework/config/conf_yaml_schema.json    |  65 ++++
> >  dts/framework/dts.py                          |  68 ++++
> >  dts/framework/exception.py                    |  57 +++
> >  dts/framework/logger.py                       | 114 ++++++
> >  dts/framework/remote_session/__init__.py      |  15 +
> >  .../remote_session/remote_session.py          | 100 +++++
> >  dts/framework/remote_session/ssh_session.py   | 185 +++++++++
> >  dts/framework/settings.py                     | 119 ++++++
> >  dts/framework/testbed_model/__init__.py       |   8 +
> >  dts/framework/testbed_model/node.py           |  63 ++++
> >  dts/framework/utils.py                        |  31 ++
> >  dts/main.py                                   |  24 ++
> >  dts/poetry.lock                               | 351 ++++++++++++++++++
>
> A lot of dependencies look not useful in this first series for SSH
> connection.
>

I've put the actual dependency list below. The lock file contains the
entire dependency tree, of which Scapy is responsible for a substantial
portion. pexpect is currently used because of the large amount of code from
the old dts that was moved over, but it will be replaced by a better
solution soon with a rewrite of the dts/framework/remote_session module.
Warlock is used to handle json schema checking, which is how we provide
instant feedback about an incorrect config file to users (as opposed to old
DTS, where you would be informed by a stack trace at an arbitrary point in
the program). PyYAML is used to parse the yaml config file, and
types-PyYAML is used to provide information to the python type checker
about pyyaml. Scapy provides all of the packet manipulation capabilities in
DTS, and I was not able to find a better library for doing packet
manipulation in python, so in my eyes we can excuse its dependency tree due
to how much time it will save.

[tool.poetry.dependencies]
python = "^3.10"
pexpect = "^4.8.0"
warlock = "^2.0.1"
PyYAML = "^6.0"
types-PyYAML = "^6.0.8"
scapy = "^2.4.5"


>
> >  dts/pyproject.toml                            |  55 +++
> >  27 files changed, 1723 insertions(+), 4 deletions(-)
>
>
>
>
  

02/11/2022 13:58, Owen Hilyard:
> On Mon, Oct 31, 2022 at 3:01 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > I was about to merge this series,
> > and after long thoughts, it deserves a bit more changes.
> > I would like to work with you for a merge in 22.11-rc3.
> >
> > 13/10/2022 12:35, Juraj Linkeš:
> > > All the necessary code needed to connect to a node in a topology with
> > > a bit more, such as basic logging and some extra useful methods.
> >
> > There is also some developer tooling,
> > and some documentation.
> >
> > [...]
> > > There are configuration files with a README that help with setting up
> > > the execution/development environment.
> >
> > I don't want to merge some doc which is not integrated
> > in the doc/ directory.
> > It should be in RST format in doc/guides/dts/
> > I can help with this conversion.
> >
> > > The code only connects to a node. You'll see logs emitted to console
> > > saying where DTS connected.
> > >
> > > There's only a bit of documentation, as there's not much to document.
> > > We'll add some real docs when there's enough functionality to document,
> > > when the HelloWorld testcases is in (point 4 in our roadmap below). What
> > > will be documented later is runtime dependencies and how to set up the
> > DTS
> > > control node environment.
> > >
> > [...]
> > >  .editorconfig                                 |   2 +-
> > >  .gitignore                                    |   9 +-
> >
> > Updating general Python guidelines in these files
> > should be done separately to get broader agreement.
> >
> > >  MAINTAINERS                                   |   5 +
> >
> > You can update this file in the first patch.
> >
> > >  devtools/python-checkpatch.sh                 |  39 ++
> >
> > Let's postpone the integration of checkpatch.
> > It should be integrated with the existing checkpatch.
> >
> 
> We wanted to specifically ensure that all code met the quality requirements
> for DTS from the start. The current formatting requirements are "whatever
> these tools run in this order with these settings results in", since the
> working group decided that fully automated rectification of minor issues
> would help new contributors focus on more important things. I agree that
> integrating it into existing checkpatch would be good, but I think that to
> do that we would first need to run the tool over existing DPDK python code.
> python-checkpatch does do linting like the main checkpatch, but it will
> also perform source code changes to automatically fix many formatting
> issues. Do we want to keep checkpatch as a read-only script and introduce
> another one which makes source-code changes, or merge them together? This
> would also mean that all DPDK developers would need to run checkpatch
> inside of a python virtual environment since DTS is currently very specific
> about what versions are used (via both version number and cryptographic
> hash of the source archive). These versions are newer than what are shipped
> in many stable linux distributions (Ubuntu, Debian, etc), because we want
> the additional capabilities that come with the newer versions and the
> version in the Debian Buster repos is old enough that it does not support
> the version of python that we are using. We were trying to avoid forcing
> all DPDK developers to install a full suite of python tools.

So who is running the script?
My wish is to make it automatically run when calling checkpatch.sh.

> >  devtools/python-format.sh                     |  54 +++
> > >  devtools/python-lint.sh                       |  26 ++
> >
> > Let's postpone the integration of these tools.
> > We need to discuss what is specific to DTS or not.
> >
> > >  doc/guides/contributing/coding_style.rst      |   4 +-
> >
> > It is not specific to DTS.
> >
> > >  dts/.devcontainer/devcontainer.json           |  30 ++
> > >  dts/Dockerfile                                |  39 ++
> >
> > Not sure about Docker tied to some personal choices.
> 
> The reason we are using docker is that it provides a canonical environment
> to run the test harness in, which can then connect to the SUT and traffic
> generator. It enforces isolation between these three components, and
> ensures that everyone is using the exact same environment to test and
> deploy the test harness. Although devcontainer started in Visual Studio
> Code, it is supported by many IDEs at this point and we wanted to encourage
> developers along a path which avoids needing to set up a development
> environment before they can start working. The very recent version of
> python (3.10) we choose to use for additional type system verification
> makes not using containers much harder. It also means that new
> dependencies, new dependency versions or additional system dependencies can
> be easily handled by the normal patch process and rolled out to everyone in
> a mostly automated fashion.
> 
> Additionally, although it is named Dockerfile, it is fully compatible with
> podman.

In general we avoid enforcing specific versions in DPDK.
The idea is to make it usable from any system with good compatibility.
Using a container is more convenient in general, I agree.
I just would like to have the choices here discussed specifically in a separate patch.

> > >  dts/README.md                                 | 154 ++++++++
> >
> > As said above, it should in RST format in doc/guides/dts/
> >
> > >  dts/conf.yaml                                 |   6 +
> > >  dts/framework/__init__.py                     |   4 +
> > >  dts/framework/config/__init__.py              | 100 +++++
> > >  dts/framework/config/conf_yaml_schema.json    |  65 ++++
> > >  dts/framework/dts.py                          |  68 ++++
> > >  dts/framework/exception.py                    |  57 +++
> > >  dts/framework/logger.py                       | 114 ++++++
> > >  dts/framework/remote_session/__init__.py      |  15 +
> > >  .../remote_session/remote_session.py          | 100 +++++
> > >  dts/framework/remote_session/ssh_session.py   | 185 +++++++++
> > >  dts/framework/settings.py                     | 119 ++++++
> > >  dts/framework/testbed_model/__init__.py       |   8 +
> > >  dts/framework/testbed_model/node.py           |  63 ++++
> > >  dts/framework/utils.py                        |  31 ++
> > >  dts/main.py                                   |  24 ++
> > >  dts/poetry.lock                               | 351 ++++++++++++++++++
> >
> > A lot of dependencies look not useful in this first series for SSH
> > connection.
> 
> I've put the actual dependency list below. The lock file contains the
> entire dependency tree, of which Scapy is responsible for a substantial
> portion. pexpect is currently used because of the large amount of code from
> the old dts that was moved over, but it will be replaced by a better
> solution soon with a rewrite of the dts/framework/remote_session module.
> Warlock is used to handle json schema checking, which is how we provide
> instant feedback about an incorrect config file to users (as opposed to old
> DTS, where you would be informed by a stack trace at an arbitrary point in
> the program). PyYAML is used to parse the yaml config file, and
> types-PyYAML is used to provide information to the python type checker
> about pyyaml. Scapy provides all of the packet manipulation capabilities in
> DTS, and I was not able to find a better library for doing packet
> manipulation in python, so in my eyes we can excuse its dependency tree due
> to how much time it will save.
> 
> [tool.poetry.dependencies]
> python = "^3.10"
> pexpect = "^4.8.0"
> warlock = "^2.0.1"
> PyYAML = "^6.0"
> types-PyYAML = "^6.0.8"
> scapy = "^2.4.5"

Scapy is not needed for SSH connection.
Is warlock used in this series?