Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore LXC spawner task output path handling #5674

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

pevogam
Copy link
Contributor

@pevogam pevogam commented May 2, 2023

Before this adaptation we get simple avocado runs to result in

[root@c5 site-packages]# avocado run --status-server-disable-auto --spawner=lxc -- /bin/true
JOB ID     : 937ec5bb7275feabd48fcf5b5b315ad2cd2b63fc
JOB LOG    : /mnt/local/results/job-2023-05-02T22.51-937ec5b/job.log
 (1/1) /bin/true: STARTED
 (1/1) /bin/true: ERROR: [Errno 2] No such file or directory: '/mnt/local/results/job-2023-05-02T22.51-937ec5b/test-results/1-_bin_true/stdout' (0.00 s)
RESULTS    : PASS 0 | ERROR 1 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /mnt/local/results/job-2023-05-02T22.51-937ec5b/results.html
JOB TIME   : 2.69 s

while after this fix we finally get

[root@c5 site-packages]# avocado run --status-server-disable-auto --spawner=lxc -- /bin/true
JOB ID     : 1b736d172710a2d0d85d63b181cd65d0bc8ff518
JOB LOG    : /mnt/local/results/job-2023-05-02T22.54-1b736d1/job.log
 (1/1) /bin/true: STARTED
 (1/1) /bin/true: PASS (0.01 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /mnt/local/results/job-2023-05-02T22.54-1b736d1/results.html
JOB TIME   : 2.66 s

Note that all of this requires mounting the results directory into the LXC container, similarly to the way it is done for podman containers.

Before this adaptation we get simple avocado runs to result in
```
[root@c5 site-packages]# avocado run --status-server-disable-auto --spawner=lxc -- /bin/true
JOB ID     : 937ec5bb7275feabd48fcf5b5b315ad2cd2b63fc
JOB LOG    : /mnt/local/results/job-2023-05-02T22.51-937ec5b/job.log
 (1/1) /bin/true: STARTED
 (1/1) /bin/true: ERROR: [Errno 2] No such file or directory: '/mnt/local/results/job-2023-05-02T22.51-937ec5b/test-results/1-_bin_true/stdout' (0.00 s)
RESULTS    : PASS 0 | ERROR 1 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /mnt/local/results/job-2023-05-02T22.51-937ec5b/results.html
JOB TIME   : 2.69 s
```
while after this fix we finally get
```
[root@c5 site-packages]# avocado run --status-server-disable-auto --spawner=lxc -- /bin/true
JOB ID     : 1b736d172710a2d0d85d63b181cd65d0bc8ff518
JOB LOG    : /mnt/local/results/job-2023-05-02T22.54-1b736d1/job.log
 (1/1) /bin/true: STARTED
 (1/1) /bin/true: PASS (0.01 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /mnt/local/results/job-2023-05-02T22.54-1b736d1/results.html
JOB TIME   : 2.66 s
```

Note that all of this requires mounting the results directory into
the LXC container, similarly to the way it is done for podman containers.

Signed-off-by: Plamen Dimitrov <[email protected]>
@pevogam
Copy link
Contributor Author

pevogam commented May 2, 2023

This pull request addresses parts of #5666.

@pevogam
Copy link
Contributor Author

pevogam commented May 4, 2023

Interestingly the previous diff worked just fine for me for years now using LXC containers. Turns out the reason was that the Avocado I2N plugin was providing None for the output directory which somehow resulted in both local task output directory being created just fine and the same directory then being copied or mapped back to the externally created output directory.

@richtja I can see Avocado VT has the output_dir parameter in its vt_params here when constructing the runnable:

    def _parameters_to_runnable(self, params):
        params = self.convert_parameters(params)
        uri = params.get('name')
        vt_params = params.get('vt_params')

        # Flatten the vt_params, discarding the attributes that are not
        # scalars, and will not be used in the context of nrunner
        for key in ('_name_map_file', '_short_name_map_file', 'dep'):
            if key in vt_params:
                del(vt_params[key])

        return Runnable('avocado-vt', uri, **vt_params)

However, I fail to understand where is this params["output_dir"] set. I could not find is with keyword searching and it has to be provided to the runnable so that its runnable.output_dir is not None and the above change works also for our test scheduler. Do you happen to know how I should immitate Avocado VT here so that this key is set correctly and in the most natural way possible?

@richtja
Copy link
Contributor

richtja commented May 4, 2023

Interestingly the previous diff worked just fine for me for years now using LXC containers. Turns out the reason was that the Avocado I2N plugin was providing None for the output directory which somehow resulted in both local task output directory being created just fine and the same directory then being copied or mapped back to the externally created output directory.

@richtja I can see Avocado VT has the output_dir parameter in its vt_params here when constructing the runnable:

    def _parameters_to_runnable(self, params):
        params = self.convert_parameters(params)
        uri = params.get('name')
        vt_params = params.get('vt_params')

        # Flatten the vt_params, discarding the attributes that are not
        # scalars, and will not be used in the context of nrunner
        for key in ('_name_map_file', '_short_name_map_file', 'dep'):
            if key in vt_params:
                del(vt_params[key])

        return Runnable('avocado-vt', uri, **vt_params)

However, I fail to understand where is this params["output_dir"] set. I could not find is with keyword searching and it has to be provided to the runnable so that its runnable.output_dir is not None and the above change works also for our test scheduler. Do you happen to know how I should immitate Avocado VT here so that this key is set correctly and in the most natural way possible?

Hi @pevogam if you can see output_dir parameter in vt_params in here, then it should be because of your Cartesian config (I can't see in my configuration with simple boot test). The vt_params are coming from the Cartesian config or command-line.

But, I did some changes to Runnable.output_dir parameter which might be related to this in here. After this change, avocado will set the Runnable.output_dir to job_base_dir/test-results/test_name if the Runnable.output_dir is None And the Runnable.output_dir is respected when the log files are initialized.

@pevogam
Copy link
Contributor Author

pevogam commented May 4, 2023

Hi @pevogam if you can see output_dir parameter in vt_params in here, then it should be because of your Cartesian config (I can't see in my configuration with simple boot test). The vt_params are coming from the Cartesian config or command-line.

In fact I didn't phrase my wording well enough - I meant that I cannot find it anywhere in our configs and is also missing during the test run resulting in output_dir = None for all tests and thus missing folders.

But, I did some changes to Runnable.output_dir parameter which might be related to this in here. After this change, avocado will set the Runnable.output_dir to job_base_dir/test-results/test_name if the Runnable.output_dir is None And the Runnable.output_dir is respected when the log files are initialized.

I guess you mean some part of the code of the default avocado scheduler since the Runnable.output_dir is not set in any way with our scheduler (which is normal since it wasn't aware of it). If you don't see the output_dir in your simple boot VT test run, then how does the directory get set in Avocado VT? We clearly need some magic to combine the base directory with the job ID and then with test-results and the test name. I guess for now I can simply imitate a path.join like the ones in the commit you pointed out, I was just wondering what would be the most natural way to do it (i.e. how does Avocado VT do it).

@richtja
Copy link
Contributor

richtja commented May 4, 2023

But, I did some changes to Runnable.output_dir parameter which might be related to this in here. After this change, avocado will set the Runnable.output_dir to job_base_dir/test-results/test_name if the Runnable.output_dir is None And the Runnable.output_dir is respected when the log files are initialized.

I guess you mean some part of the code of the default avocado scheduler since the Runnable.output_dir is not set in any way with our scheduler (which is normal since it wasn't aware of it). If you don't see the output_dir in your simple boot VT test run, then how does the directory get set in Avocado VT? We clearly need some magic to combine the base directory with the job ID and then with test-results and the test name. I guess for now I can simply imitate a path.join like the ones in the commit you pointed out, I was just wondering what would be the most natural way to do it (i.e. how does Avocado VT do it).

Avocado VT is not setting output_dir if it is not part of any configuration. The output_dir is set by Avocado before the test task is spawned. It is the part of the changes which I mentioned earlier.

@pevogam
Copy link
Contributor Author

pevogam commented May 4, 2023

But, I did some changes to Runnable.output_dir parameter which might be related to this in here. After this change, avocado will set the Runnable.output_dir to job_base_dir/test-results/test_name if the Runnable.output_dir is None And the Runnable.output_dir is respected when the log files are initialized.

I guess you mean some part of the code of the default avocado scheduler since the Runnable.output_dir is not set in any way with our scheduler (which is normal since it wasn't aware of it). If you don't see the output_dir in your simple boot VT test run, then how does the directory get set in Avocado VT? We clearly need some magic to combine the base directory with the job ID and then with test-results and the test name. I guess for now I can simply imitate a path.join like the ones in the commit you pointed out, I was just wondering what would be the most natural way to do it (i.e. how does Avocado VT do it).

Avocado VT is not setting output_dir if it is not part of any configuration. The output_dir is set by Avocado before the test task is spawned. It is the part of the changes which I mentioned earlier.

Then I assume you imply that Avocado VT uses from_runnable() in order for the line you pointed at to be executed at some time before the test run.

@richtja
Copy link
Contributor

richtja commented May 5, 2023

But, I did some changes to Runnable.output_dir parameter which might be related to this in here. After this change, avocado will set the Runnable.output_dir to job_base_dir/test-results/test_name if the Runnable.output_dir is None And the Runnable.output_dir is respected when the log files are initialized.

I guess you mean some part of the code of the default avocado scheduler since the Runnable.output_dir is not set in any way with our scheduler (which is normal since it wasn't aware of it). If you don't see the output_dir in your simple boot VT test run, then how does the directory get set in Avocado VT? We clearly need some magic to combine the base directory with the job ID and then with test-results and the test name. I guess for now I can simply imitate a path.join like the ones in the commit you pointed out, I was just wondering what would be the most natural way to do it (i.e. how does Avocado VT do it).

Avocado VT is not setting output_dir if it is not part of any configuration. The output_dir is set by Avocado before the test task is spawned. It is the part of the changes which I mentioned earlier.

Then I assume you imply that Avocado VT uses from_runnable() in order for the line you pointed at to be executed at some time before the test run.

Yes I expect that avocado-vt uses from_runnable() method, because the Avocado VT only creates Runnables by VTResolver, but the from_runnable() method is related to RuntimeTask which is avocado.core.task.runtime class and it is not related to any avocado plugins. Of course, if you are not using the standard avocado task creation then you might have issues, but such scenario is not supported by avocado.

@pevogam
Copy link
Contributor Author

pevogam commented May 8, 2023

But, I did some changes to Runnable.output_dir parameter which might be related to this in here. After this change, avocado will set the Runnable.output_dir to job_base_dir/test-results/test_name if the Runnable.output_dir is None And the Runnable.output_dir is respected when the log files are initialized.

I guess you mean some part of the code of the default avocado scheduler since the Runnable.output_dir is not set in any way with our scheduler (which is normal since it wasn't aware of it). If you don't see the output_dir in your simple boot VT test run, then how does the directory get set in Avocado VT? We clearly need some magic to combine the base directory with the job ID and then with test-results and the test name. I guess for now I can simply imitate a path.join like the ones in the commit you pointed out, I was just wondering what would be the most natural way to do it (i.e. how does Avocado VT do it).

Avocado VT is not setting output_dir if it is not part of any configuration. The output_dir is set by Avocado before the test task is spawned. It is the part of the changes which I mentioned earlier.

Then I assume you imply that Avocado VT uses from_runnable() in order for the line you pointed at to be executed at some time before the test run.

Yes I expect that avocado-vt uses from_runnable() method, because the Avocado VT only creates Runnables by VTResolver, but the from_runnable() method is related to RuntimeTask which is avocado.core.task.runtime class and it is not related to any avocado plugins. Of course, if you are not using the standard avocado task creation then you might have issues, but such scenario is not supported by avocado.

Ok, this all makes sense then. The way I fix it in our scheduling then is by adding the line:

         raw_task = Task(node.get_runnable(), node.id_test,
                         [job.config.get('run.status_server_uri')],
                         category=TASK_DEFAULT_CATEGORY,
                         job_id=self.job.unique_id)
+        raw_task.runnable.output_dir = os.path.join(job.test_results_path,
+                                                    raw_task.identifier.str_filesystem)
         task = RuntimeTask(raw_task)
         if spawner == "lxc":
             task.spawner_handle = host
         elif spawner == "remote":
             task.spawner_handle = node.get_session_to_net()
         self.tasks += [task]

Copy link
Contributor

@clebergnu clebergnu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@clebergnu clebergnu merged commit 526e345 into avocado-framework:master Jun 9, 2023
@pevogam pevogam deleted the lxc-spawner-output branch June 13, 2023 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants