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.

Sign up for GitHub

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

Jump to bottom

Execution of remote parsyncfp2 on SEND host fails if user's shell does not support export #7

Open
gt3M opened this issue Mar 19, 2023 · 13 comments
Open

Execution of remote parsyncfp2 on SEND host fails if user's shell does not support export #7

gt3M opened this issue Mar 19, 2023 · 13 comments

Comments

@gt3M
Copy link
Contributor

gt3M commented Mar 19, 2023

parsyncfp2/parsyncfp2

Lines 277 to 279 in 476e3b6

if ( !defined $RPATH ) {
$RPATHPFX = "export PATH=${parsync_dir}:~/bin:/bin:/usr/sbin:/sbin:/usr/bin:\$PATH;";
} else { $RPATHPFX = "export PATH=${RPATH}:${parsync_dir}:/usr/sbin:/sbin:/usr/bin:/bin:\$PATH;" }

@hjmangalam I'm curious as to your thoughts on approaches to support the case in which the remote user does not have a default shell that supports export (so, neither sh not bash nor ash nor ksh nor zsh, etc, but a C-based shell). I'm willing to submit a patch. One approach would be to get the remote shell when performing a checkhost, then building RPATHPFX using the appropriate command for setting environment variables. Another approach, though more invasive, could be to use ssh's SendEnv option to pass PATH to the remote host; downside is that the remote sshd has to be configured to AcceptEnv PATH, which some might consider a security problem. Lastly, wrapping the whole remote command in bash -c might be possible, though it could also introduce more quoting issues. I suppose one last option could be to not include the setting of PATH in the remote command at all and just assume that the requisite commands are in the remote user's default path.
@hjmangalam
Copy link
Owner

hjmangalam commented Mar 19, 2023 via email

@gt3M
Copy link
Contributor Author

gt3M commented Mar 19, 2023

As far as I can tell, --rpath allows one to specify a file path that is prepended to PATH, but PATH is still being set using export. I should have noticed it earlier, having seen the parsyncfp2 print the command it would execute via ssh many times, but I only encountered the issue when attempting to use a remote user whose shell is tcsh. I did some experiments, including trying to use /usr/bin/env PATH=... instead, but have not yet gotten it to work (the ssh commands exit, either because parsyncfp2 on the send hosts is failing or because ssh is failing; it's not yet clear). I will continue to experiment.

@gt3M
Copy link
Contributor Author

gt3M commented Mar 19, 2023 •
edited
Loading

Here, the check for Host in the remote user's .ssh/config file is also sh-centric. Fortunately, it's a simple check and could use test instead, I suspect:

qq(ssh $fqrhs 'test -e ~/.ssh/config && grep Host ~/.ssh/config' 2> /dev/null);

There will still be a problem if the user that calls initial parsyncfp2 calls it in (t)csh as the 2> won't work. If the goal is to just check if Host is in ~/.ssh/config and the lines containing Host don't need to be printed (i.e. the return code of grep is what's important), then redirecting stderr to /dev/null could be omitted. Else, both bash and tcsh support >& to redirect to a file, e.g.

qq(ssh $fqrhs 'test -e ~/.ssh/config && grep Host ~/.ssh/config' >& /dev/null);

@gt3M
Copy link
Contributor Author

gt3M commented Mar 19, 2023 •
edited
Loading

Turns out I'm wrong on 2> not working if the initial parsyncfp2 is called from tcsh. Both system and backticks in perl will use sh -c, so 2> really does redirect stderr even if the perl program is called from tcsh. I do think that in the case in which the check is only for Host in the remote ~/.ssh/config can be done by just checking the return code of grep, i.e.

$checkcmd = qq(ssh $fqrhs 'test -e ~/.ssh/config && grep Host ~/.ssh/config' 2>/dev/null);
if ( system($checkcmd) == 0 ) {
  WARN("[$fqrhs] has a ~/.ssh/config file containing 'Host' definitions, ...")
} else {
  if ( $VERBOSE > 2 ) { INFO("SEND Host [$fqrhs] No conflicting ~/.ssh/config.\n") }
}

@hjmangalam
Copy link
Owner

hjmangalam commented Mar 21, 2023 via email

@gt3M
Copy link
Contributor Author

gt3M commented Mar 21, 2023 •
edited
Loading

@hjmangalam Thanks! The csh support looks promising. Unfortunately, it seems that the --ro changes may now have made it such that --ro is a required option. I get this when attempting to use 2.56, having not specified --ro at all:

** FATAL ERROR **: You didn't have a '-' in your '--ro' string.
  You need to supply the options exactly as you would with rsync, with
  leading '-' or '--'. Try again..

Command I'm using is like:

parsyncfp2 --verbose=3 --nowait --NP=4 --chunksize=100M --commondir=${PWD}/pfp2 --startdir=${PWD} --hosts=${HOSTNAME}=hpcc-dx01-pr Downloads POD::/homes/01/${USER}

I know that this is a multihost-style command-line but a singlehost-style transfer, but it was an easy way to reproduce the problem. If I use the singlehost-style command, like the following, I do not get the --ro error:

parsyncfp2 --verbose=3 --nowait --NP=4 --chunksize=100M --commondir=${PWD}/pfp2 --startdir=${PWD} Downloads hpcc-dx01-pr:/homes/01/${USER}

@hjmangalam
Copy link
Owner

hjmangalam commented Mar 21, 2023 via email

@gt3M
Copy link
Contributor Author

gt3M commented Mar 21, 2023

@hjmangalam No worries. It's working with tcsh!

I'm still testing the --ro fixes. I'm running it right now with --ro="'-asl --excludes-from=${excludes_file_path}'" and rsync does not seem to be complaining. As you can see, the one downside to using a singe-quoted value is one needs to then wrap that in double-quotes to do any variable interpolation for the arguments being passed to rsync. I think it's working as all of my rsyncs are being run via ssh. When I get a chance, I can also test with a singlehost copy to see if double-quoting the single quotes will be an issue. It's possible I may need to eval and echo of the string to strip the double-quotes.

@hjmangalam
Copy link
Owner

hjmangalam commented Mar 21, 2023 via email

@gt3M
Copy link
Contributor Author

gt3M commented Mar 21, 2023

At least for the rsync options, another approach could be to give up on trying to preserve the default whitespace delimiter and have the user pass them in as a comma-delimited list, e.g.

parsyncfp2 --ro=-asl,--exclude-from=/path/to/file ...

It's been a long time since I've used Perl's GetOpt::Long, so I'm not sure if that will trip it up, or if it would correctly know that because --ro is preceded by a word boundary everything after the = is the value.

@hjmangalam
Copy link
Owner

hjmangalam commented Mar 21, 2023 via email

@gt3M
Copy link
Contributor Author

gt3M commented Mar 22, 2023

@hjmangalam Apologies for bothering you again. With your latest commit, I am again getting the following:

** FATAL ERROR **: You didn't have a '-' in your '--ro' string.
  You need to supply the options exactly as you would with rsync, with
  leading '-' or '--'. Try again..

So it seems that the fix for the --ro fix got unfixed :)

@hjmangalam
Copy link
Owner

hjmangalam commented Mar 22, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hjmangalam @gt3M