-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix Jindo DataLoad without target for multi mount Datasets #5631
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -160,13 +160,27 @@ func (e *JindoEngine) genDataLoadValue(image string, runtime *datav1alpha1.Jindo | |
| } | ||
|
|
||
| targetPaths := []cdataload.TargetPath{} | ||
| for _, target := range dataload.Spec.Target { | ||
| fluidNative := utils.IsTargetPathUnderFluidNativeMounts(target.Path, *targetDataset) | ||
| targetPaths = append(targetPaths, cdataload.TargetPath{ | ||
| Path: target.Path, | ||
| Replicas: target.Replicas, | ||
| FluidNative: fluidNative, | ||
| }) | ||
| if len(dataload.Spec.Target) > 0 { | ||
| for _, target := range dataload.Spec.Target { | ||
| fluidNative := utils.IsTargetPathUnderFluidNativeMounts(target.Path, *targetDataset) | ||
| targetPaths = append(targetPaths, cdataload.TargetPath{ | ||
| Path: target.Path, | ||
| Replicas: target.Replicas, | ||
| FluidNative: fluidNative, | ||
| }) | ||
| } | ||
| } else { | ||
| for _, m := range targetDataset.Spec.Mounts { | ||
| if m.Path == "" { | ||
| continue | ||
| } | ||
| fluidNative := utils.IsTargetPathUnderFluidNativeMounts(m.Path, *targetDataset) | ||
| targetPaths = append(targetPaths, cdataload.TargetPath{ | ||
| Path: m.Path, | ||
| Replicas: 1, | ||
| FluidNative: fluidNative, | ||
| }) | ||
| } | ||
| } | ||
|
Comment on lines
+172
to
184
|
||
| dataloadInfo.TargetPaths = targetPaths | ||
| options := map[string]string{} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When mount.Path is empty, the code skips that mount entirely. According to the UFSPathBuilder.GenUFSPathInUnifiedNamespace convention, if mount.Path is not set, the path should default to
/{mount.Name}. Consider using GenUFSPathInUnifiedNamespace to get the correct path instead of skipping mounts with empty paths, to ensure consistency with how mount paths are handled elsewhere in the codebase.