Skip to content

fix(alluxio): propagate shared flag for direct UFS mounts #5526

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

Open
mrhapile wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(alluxio): propagate shared flag for direct UFS mounts #5526

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/ddc/alluxio/transform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ func (e *AlluxioEngine) transformCommonPart(runtime *datav1alpha1.AlluxioRuntime
value.Properties[k] = v
}
}

if m.Shared {
value.Properties["alluxio.master.mount.table.root.shared"] = "true"
}
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

GenAlluxioUFSRootPath() only returns a non-nil m when there is exactly one non-Fluid-native mount whose unified namespace path is / (see pkg/utils/ufs_path_builder.go). For common “direct UFS” setups where the root UFS is the local storage root (/underFSStorage) and the Dataset mount is pvc:// or local://, m will be nil, so this change still won’t set alluxio.master.mount.table.root.shared and the mount will remain not shared (the behavior described in #998). Consider deriving the root shared setting from the Dataset mounts when uRootPath is the local storage root (e.g., set it if any Fluid-native mount is marked shared, or at least when there is a single Fluid-native mount with shared: true).

Suggested change
}
}
} else if uRootPath == common.RootDirPath {
// For direct UFS setups where GenAlluxioUFSRootPath does not return a mount,
// derive the root shared setting from Dataset mounts.
if _, exists := value.Properties["alluxio.master.mount.table.root.shared"]; !exists {
for _, mount := range dataset.Spec.Mounts {
if mount.Shared {
value.Properties["alluxio.master.mount.table.root.shared"] = "true"
break
}
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +172 to +174
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

This new behavior (propagating Dataset shared into alluxio.master.mount.table.root.shared) isn’t covered by unit tests. Since this package already has transform_test.go, please add a focused test that builds a Dataset/Runtime with a shared mount and asserts the resulting value.Properties includes the expected root shared property (covering both the non-native root-mount case and the local-/pvc-backed /underFSStorage case if applicable).

Copilot uses AI. Check for mistakes.
}
e.Log.Info("output", "uRootPath", uRootPath, "m", m)
// set alluxio root ufs
Expand Down
Loading