Add support for custom local file transfer commands via PBS_CP variable

[jdburton]

Describe Bug or Feature

Currently, administrators can specify a custom command for the PBS MoM to use for remote file transfers by specifying the command in the PBS_SCP and PBS_RCP variables in /etc/pbs.conf. If these variables are unset, PBS will default to /bin/scp or <PBS_HOME>/sbin/pbs_rcp, respectively.

There is no way to specify a custom command for local copies, however. PBS will always use /bin/cp on *nix based systems and xcopy on Windows systems. Administrators should have the ability to specify custom commands for local copies like they do for remote copies.

Describe Your Change

This change adds support for the PBS_CP variable in /etc/pbs.conf. If PBS_CP is unset, the MoM will default to /bin/cp on *nix systems and xcopy on Windows systems.

Link to Design Doc

https://pbspro.atlassian.net/wiki/spaces/PD/pages/1521680421/Add+support+for+custom+local+copy+via+the+PBS+CP+variable

Attach Test and Valgrind Logs/Output

[bhroam]

@jdburton thank you for your contribution to PBS. A feature (even a small one like this) has a requirement to come with one or more PTL automated tests. If you could write them, that would be great.

[jdburton]

PBS_CP has been added to the pbs_testlib.py tests. The functionality of PBS_CP is similar to PBS_SCP, therefore, the test is similar.

Also PBS_CP documentation has been added to the appropriate man pages.

[vstumpf]

You've added PBS_CP to the framework, but we have our tests in test/tests/, there should be a new test written somewhere in the functional directory. If you could write one and put it there, that would be great.

[jdburton]

Perhaps I am missing something, but I don't see any tests for PBS_SCP or PBS_RCP in the test/tests directory. The PBS_CP code is based on the PBS_SCP code, so the tests should be similar.

[agrawalravi90]

PBS_CP has been added to the pbs_testlib.py tests. The functionality of PBS_CP is similar to PBS_SCP, therefore, the test is similar.

pbs_testlib.py or pbs_testsuite.py are not tests:

• pbs_testlib.py contains Python routines which talk to PBS, it's used by multiple programs, one of which is pbs_benchpress, which is the program that we use to run tests.
• pbs_testsuite.py provides a Python testing framework that we use to write and run tests for PBS written in Python, it is a wrapper around Python's unittest: https://docs.python.org/3.8/library/unittest.html

test/tests/ is the directory that contains the actual tests, most of them go inside tests/functional directory.

Perhaps I am missing something, but I don't see any tests for PBS_SCP or PBS_RCP in the test/tests directory. The PBS_CP code is based on the PBS_SCP code, so the tests should be similar.

You are correct, there are currently no tests for PBS_SCP/PBS_RCP. The reason being, PBS has existed for way longer than our test framework has. We started writing tests based off of our Python framework much later. So, we write tests for any new features added to PBS, and any bugfixes made to existing features. Each new test adds better coverage.

Anyways, since you are adding a new feature, it would be great if you could add a PTL test for it. If you need pointers about PTL, we can help. Here's some documentation if you'd like to learn more about how to write tests: https://pbspro.atlassian.net/wiki/spaces/DG/pages/1324417323/PTL+How-Tos

[CLAassistant]

All committers have signed the CLA.

[hirenvadalia]

@jdburton Are you still working on this PR?

[jdburton]

I still think this is a worthwhile change, I just haven't had the time to commit to learning the testing system in order to write the tests that you want for it.

…

On Fri, Jul 24, 2020 at 1:01 PM Hiren Vadalia ***@***.***> wrote: @jdburton <https://github.com/jdburton> Are you still working on this PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1515 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAM26456EUDX7SY3SOZBM2DR5G47XANCNFSM4KT5SHPQ> .

[agrawalravi90]

@jdburton the maintainers discussed and it's ok if you'd not like to write a PTL test for this. Please address any other review comments on the PR so that this can be signed-off and merged.

[agrawalravi90]

@jdburton it seems like something went wrong again. Please make sure that your local 'master' branch is exactly in sync with the master branch from the main repository and doesn't have any of your changes. Then, follow these steps:
git checkout pbs_cp
git checkout master -- test/fw/ptl/utils/pbs_testsuite.py
git commit -m "Reverting pbs_testsuite.py"

Then, push to your fork

I think this will fix it, please give it a shot, thanks.

[agrawalravi90]

LGTM
@hirenvadalia @vstumpf and @bhroam do you guys have any further comments on this?

[bhroam]

Not me. My only comment was about the PTL tests and that has been resolved.

[vstumpf]

LGTM