Ensure destination path exists prior to data movement #161
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To ensure that dcp behaves the same on each data movement, make sure the destination path exists before attempting data movement. This also ensures that as long as the user provides a valid path with the correct permissions, dcp will have a safe place for the destination. Path validation will need to be done upfront in nnf-sos and the nnf-dm daemon. Signed-off-by: Blake Devcich <blake.devcich@hpe.com>
ajfloeder
reviewed
Mar 1, 2024
| @@ -145,6 +148,10 @@ func (r *DataMovementReconciler) Reconcile(ctx context.Context, req ctrl.Request | |||
| return ctrl.Result{}, client.IgnoreNotFound(err) | |||
| } | |||
|
|
|||
| statusUpdater := updater.NewStatusUpdater[*nnfv1alpha1.NnfDataMovementStatus](dm) | |||
| defer func() { err = statusUpdater.CloseWithStatusUpdate(ctx, r.Client.Status(), err) }() | |||
| // defer func() { dm.Status.SetResourceErrorAndLog(err, log) }() | |||
|
|
||
| func createDestinationDir(dest string, uid, gid uint32) error { | ||
| // Use exec.Command() rather than os.MkDirAll so we can set the UID/GID | ||
| mkdirCmd := exec.Command("mkdir", "-p", dest) |
There was a problem hiding this comment.
You may consider something like https://github.com/NearNodeFlash/nnf-sos/blob/master/pkg/command/cmd.go.
Those Run() methods allow timeout and grab stderr and stdout in the log.
There was a problem hiding this comment.
Awesome idea. Hmm, nnf-dm already has an indirect vendor of EC. Should be easy to use it.
There was a problem hiding this comment.
Good idea. I will extend that to add the ability to run as specific UID/GID.
There was a problem hiding this comment.
PR for that is here: NearNodeFlash/nnf-sos#277
roehrich-hpe
approved these changes
Mar 1, 2024
| Gid: gid, | ||
| }, | ||
| } | ||
| if err := mkdirCmd.Run(); err != nil { |
There was a problem hiding this comment.
Try mkdirCmd.CombinedOutput(), so you can log everything, stdout and stderr.
https://pkg.go.dev/os/exec#Cmd.CombinedOutput
Signed-off-by: Blake Devcich <blake.devcich@hpe.com>
ajfloeder
approved these changes
Mar 1, 2024
Signed-off-by: Blake Devcich <blake.devcich@hpe.com>
ajfloeder
approved these changes
Mar 1, 2024
Signed-off-by: Blake Devcich <blake.devcich@hpe.com>
ajfloeder
approved these changes
Mar 4, 2024
|
I'd like to merge this and continue to find/fix any issues in other PRs |
bdevcich
added a commit
that referenced
this pull request
Apr 2, 2024
This reverts commit fca4131. This has impacts on lustre-lustre data movement. This will be held until the full mkdir + index mount directory solution is implemented via mpirun. See the `idx-mount-dirs` branch.
bdevcich
added a commit
that referenced
this pull request
Apr 2, 2024
This reverts commit fca4131. This has impacts on lustre-lustre data movement. This will be held until the full mkdir + index mount directory solution is implemented via mpirun. See the `idx-mount-dirs` branch.
bdevcich
added a commit
that referenced
this pull request
Apr 8, 2024
This change un-reverts PR #161 with a couple twists: account for index mount directories and use mpirun to perform the actions. Logic was added to prepare the destination directory by performing a `mkdir -p` on the user supplied destination. This logic ensures that the correct pathing is performed by determining the filetype (dir vs file) on both the source and destination paths. In addition to the supplied destination path, if the source filesystem is determined to have index mount directories (i.e. xfs/gfs2), those directories are created on the destination path to ensure that all data is retained. - All necessary filesystem interaction is ran through mpirun so that the proper mpi worker nodes are used to prepare the destination. This required for lustre2lustre data movement. - These file system interactions (i.e. `stat`, `mkdir`) are performed through `setpriv` to ensure that all interactions are done as the supplied UID/GID (from the workflow). - The hostfile createion logic now standsalone since these new mpirun calls need the hostfile before the final data movement command is built - Added in some local system calls when using unit tests since mpirun is not available in textenv Signed-off-by: Blake Devcich <blake.devcich@hpe.com>
bdevcich
added a commit
that referenced
this pull request
Apr 11, 2024
…167) This change un-reverts PR #161 with a couple twists: account for index mount directories and use mpirun to perform the actions. Logic was added to prepare the destination directory by performing a `mkdir -p` on the user supplied destination. This logic ensures that the correct pathing is performed by determining the filetype (dir vs file) on both the source and destination paths. In addition to the supplied destination path, if the source filesystem is determined to have index mount directories (i.e. xfs/gfs2), those directories are created on the destination path to ensure that all data is retained. - All necessary filesystem interaction is ran through mpirun so that the proper mpi worker nodes are used to prepare the destination. This required for lustre2lustre data movement. - These file system interactions (i.e. `stat`, `mkdir`) are performed through `setpriv` to ensure that all interactions are done as the supplied UID/GID (from the workflow). - The hostfile createion logic now standsalone since these new mpirun calls need the hostfile before the final data movement command is built - Added in some local system calls when using unit tests since mpirun is not available in textenv Signed-off-by: Blake Devcich <blake.devcich@hpe.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To ensure that dcp behaves the same on each data movement, make sure the destination path exists before attempting data movement.
This also ensures that as long as the user provides a valid path with the correct permissions, dcp will have a safe place for the destination.
Path validation will need to be done upfront in nnf-sos and the nnf-dm daemon.