[RFC PATCH 0/1] dts: add SUT cleanup to framework

[RFC PATCH 0/1] dts: add SUT cleanup to framework

[Dean Marx]

Adds/modifies methods in sut_node, node, and runner to
remove dpdk tarball/tree from remote server after execution ends.
If the user specifies the remote option in the configuration, the
removal step is skipped. This attempts to fix the problem of
multiple users running on the same server, where shared names
of tarballs/trees can cause permission errors.

Dean Marx (1):
  dts: add SUT cleanup to framework

 dts/framework/runner.py                 |  2 +-
 dts/framework/testbed_model/sut_node.py | 23 ++++++++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

-- 
2.44.0

[RFC PATCH 1/1] dts: add SUT cleanup to framework

[Dean Marx]

Add method cleanup_sut to framework that removes a
DPDK source from a SUT node at the conclusion of a testrun.
This will only run when the DPDK source is being copied
from the DTS engine node during the testrun
(when remote=false in the conf.yaml).

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/runner.py                 |  2 +-
 dts/framework/testbed_model/sut_node.py | 23 ++++++++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index f91c462ce5..7f58cd01ce 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -340,7 +340,7 @@ def _run_test_run(
         finally:
             try:
                 self._logger.set_stage(DtsStage.test_run_teardown)
-                sut_node.tear_down_test_run()
+                sut_node.tear_down_test_run(test_run_config.dpdk_config)
                 tg_node.tear_down_test_run()
                 test_run_result.update_teardown(Result.PASS)
             except Exception as e:
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 14d77c50a6..d39c1bd632 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -175,6 +175,21 @@ def path_to_devbind_script(self) -> PurePath | str:
             )
         return self._path_to_devbind_script
 
+    def cleanup_sut(
+        self, dpdk_build_config: DPDKBuildConfiguration, remote_tree: str | PurePath | None
+    ) -> None:
+        """Removes the DPDK tree and/or build directory/tarball depending on the configuration."""
+        match dpdk_build_config.dpdk_location:
+            case LocalDPDKTreeLocation(dpdk_tree=dpdk_tree):
+                tree_path = self.main_session.join_remote_path(self._remote_tmp_dir, dpdk_tree.name)
+                self.main_session.remove_remote_dir(tree_path)
+            case LocalDPDKTarballLocation(tarball=tarball):
+                self.main_session.remove_remote_dir(str(remote_tree))
+                tarball_path = self.main_session.join_remote_path(
+                    self._remote_tmp_dir, tarball.name
+                )
+                self.main_session.remove_remote_file(tarball_path)
+
     def get_dpdk_build_info(self) -> DPDKBuildInfo:
         """Get additional DPDK build information.
 
@@ -203,11 +218,11 @@ def set_up_test_run(
             self.virtual_devices.append(VirtualDevice(vdev))
         self._set_up_dpdk(dpdk_build_config)
 
-    def tear_down_test_run(self) -> None:
+    def tear_down_test_run(self, dpdk_build_config: DPDKBuildConfiguration) -> None:
         """Extend the test run teardown with virtual device teardown and DPDK teardown."""
         super().tear_down_test_run()
         self.virtual_devices = []
-        self._tear_down_dpdk()
+        self._tear_down_dpdk(dpdk_build_config)
 
     def _set_up_dpdk(
         self,
@@ -243,14 +258,16 @@ def _set_up_dpdk(
 
         self.bind_ports_to_driver()
 
-    def _tear_down_dpdk(self) -> None:
+    def _tear_down_dpdk(self, dpdk_build_config: DPDKBuildConfiguration) -> None:
         """Reset DPDK variables and bind port driver to the OS driver."""
         self._env_vars = {}
+        remote_tree = self.__remote_dpdk_tree_path
         self.__remote_dpdk_tree_path = None
         self._remote_dpdk_build_dir = None
         self._dpdk_version = None
         self.compiler_version = None
         self.bind_ports_to_driver(for_dpdk=False)
+        self.cleanup_sut(dpdk_build_config, remote_tree)
 
     def _set_remote_dpdk_tree_path(self, dpdk_tree: PurePath):
         """Set the path to the remote DPDK source tree based on the provided DPDK location.
-- 
2.44.0

Re: [RFC PATCH 1/1] dts: add SUT cleanup to framework

[Patrick Robb]

[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]

On Mon, Dec 9, 2024 at 2:58 PM Dean Marx <dmarx@iol.unh.edu> wrote:

>
> -                sut_node.tear_down_test_run()
> +                sut_node.tear_down_test_run(test_run_config.dpdk_config)
>

My first question was why is there a need to pass all of dpdk_config
through from here all the way to cleanup_sut()? Is it sufficient to extract
dpdk_location from dpdk_config at the beginning here, and pass along just
dpdk_location?


>                  tg_node.tear_down_test_run()
>                  test_run_result.update_teardown(Result.PASS)
>              except Exception as e:
> diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> index 14d77c50a6..d39c1bd632 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -175,6 +175,21 @@ def path_to_devbind_script(self) -> PurePath | str:
>              )
>          return self._path_to_devbind_script
>
> +    def cleanup_sut(
> +        self, dpdk_build_config: DPDKBuildConfiguration, remote_tree: str
> | PurePath | None
>

I understand the need for str or PurePath, but can you explain why None is
included in the union?


> --
> 2.44.0
>
>
There was also discussion at the previous DTS meeting about appending the
datetime to the dpdk artifacts when they're copied over to the SUT (a way
to create artifact uniqueness). It's not really within the scope of this
patchseries, but it wouldn't hurt for us to touch base - maybe you can do
this next.

Looks good to me overall. Once you submit the v2 I will leave a little time
for other comments, otherwise I will merge.

Thanks Dean.

[-- Attachment #2: Type: text/html, Size: 2509 bytes --]

Re: [RFC PATCH 1/1] dts: add SUT cleanup to framework

[Patrick Robb]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

Tested-by: Patrick Robb <probb@iol.unh.edu>

[-- Attachment #2: Type: text/html, Size: 114 bytes --]