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

Use bash to run dm commands #111

Merged
merged 1 commit into from
May 31, 2023
Merged

Use bash to run dm commands #111

merged 1 commit into from
May 31, 2023

Conversation

bdevcich
Copy link
Contributor

This allows for more debugging when running data movement commands. For instance:

time mpirun ...

This allows for more debugging when running data movement commands. For
instance:

```
time mpirun ...
```

Signed-off-by: Blake Devcich <[email protected]>
@bdevcich
Copy link
Contributor Author

@mcfadden8

@mcfadden8
Copy link
Contributor

@bdevcich-hpe, do I need to wait for this to be deployed? Or is this something that I may run locally somehow? Please advise

@@ -273,7 +273,7 @@ func (r *DataMovementReconciler) Reconcile(ctx context.Context, req ctrl.Request
log.Info("MPI Hostfile preview", "first line", peekMpiHostfile(mpiHostfile))
}

cmd := exec.CommandContext(ctxCancel, cmdArgs[0], cmdArgs[1:]...)
cmd := exec.CommandContext(ctxCancel, "/bin/bash", "-c", strings.Join(cmdArgs, " "))
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, how are arguments combined via strings.Join here? Space-separated I would guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, spaces

Copy link
Contributor

Choose a reason for hiding this comment

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

The result of strings.Join() then needs to be sandwiched between single or double quotes; maybe single quotes so people can't dip into environment variables.

@bdevcich
Copy link
Contributor Author

@bdevcich-hpe, do I need to wait for this to be deployed? Or is this something that I may run locally somehow? Please advise

@mcfadden8 You can deploy this branch using nnf-deploy. You can checkout this branch in the nnf-dm subdirectory of nnf-deploy. Once you do that, you can deploy just nnf-dm using ./nnf-deploy deploy dm.

@@ -273,7 +273,7 @@ func (r *DataMovementReconciler) Reconcile(ctx context.Context, req ctrl.Request
log.Info("MPI Hostfile preview", "first line", peekMpiHostfile(mpiHostfile))
}

cmd := exec.CommandContext(ctxCancel, cmdArgs[0], cmdArgs[1:]...)
cmd := exec.CommandContext(ctxCancel, "/bin/bash", "-c", strings.Join(cmdArgs, " "))
Copy link
Contributor

Choose a reason for hiding this comment

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

The result of strings.Join() then needs to be sandwiched between single or double quotes; maybe single quotes so people can't dip into environment variables.

@bdevcich bdevcich merged commit 8ab0a13 into master May 31, 2023
@bdevcich bdevcich deleted the dm-allow-bash branch May 31, 2023 20:42
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.

4 participants