Skip to content

fix(quota): apply XFS/EXT4 project quota via host mount namespace - #357

Open
fourhu wants to merge 3 commits into
openebs:developfrom
fourhu:feature/issue-343
Open

fix(quota): apply XFS/EXT4 project quota via host mount namespace#357
fourhu wants to merge 3 commits into
openebs:developfrom
fourhu:feature/issue-343

Conversation

@fourhu

@fourhu fourhu commented Jul 27, 2026

Copy link
Copy Markdown

Summary

  • Fix XFS/EXT4 project quota not being applied from provisioner pods due to mount namespace mismatch (XFS project quota is not applied from provisioner pods due to mount namespace mismatch #343).
  • Run quota tools (xfs_quota, setquota, etc.) via nsenter into the host mount namespace and pass host paths (not /host/... or /data/... bind-mount paths).
  • Helper quota/cleanup pods mount host /proc at /host/proc so nsenter can reach the host mount namespace; flock uses the existing /data bind-mount of parentDir. Init pods are unchanged (/data only). DaemonSet mode already mounts the host root at /host.

Background

XFS/EXT4 project quota is resolved against the calling process mount table. Running xfs_quota inside the pod mount namespace against bind-mounted paths often fails or does not apply limits to the real host mount. Entering the host mount namespace with host-visible paths fixes this for both HelperPod and NodeDeployment modes.

Test plan

  • Unit tests for script generation (go test ./cmd/provisioner-localpv/app/) covering nsenter, host paths, container flock path, and no-prefix passthrough
  • Existing hostpath XFS/EXT4 quota BDD/e2e (tests/hostpath_quota_test.go) on a cluster with project-quota mounts and host xfsprogs/quota-tools installed
  • Manual smoke: enable XFSQuota SC, provision PVC, confirm xfs_quota report on the node shows the project limit

Notes

  • After nsenter, quota tools are resolved from the host PATH. Nodes must have xfsprogs / quota-tools installed (already documented for XFS quota).

Fixes #343

@fourhu
fourhu requested a review from a team as a code owner July 27, 2026 17:41
@niladrih
niladrih requested a review from Copilot August 4, 2026 00:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes XFS/EXT4 project quota application failures caused by running quota tools in a pod mount namespace by generating scripts that nsenter into the host mount namespace and consistently use host-visible paths, while keeping a container-visible path for lockfiles.

Changes:

  • Update quota script generation to run filesystem/quota tools via a host_exec() wrapper that enters the host mount namespace when HostPathPrefix is set.
  • Split host paths vs container-visible paths (PARENT_PATH/VOLUME_PATH vs CONTAINER_PARENT_PATH) so quota tools operate on host mounts while flock still uses a container-accessible lockfile path.
  • Update HelperPod behavior to mount the host root at /host and add unit tests covering nsenter usage and host-path semantics.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
cmd/provisioner-localpv/app/quota_scripts.go Adds host-namespace execution wrapper and separates host vs container-visible paths for quota ops and locking.
cmd/provisioner-localpv/app/quota_scripts_test.go Adds/updates tests to assert nsenter usage, host-path assignments, and correct lockfile path behavior.
cmd/provisioner-localpv/app/local_volume_manager.go Updates comments/usage to reflect host-path + nsenter quota execution model in node-deployment mode.
cmd/provisioner-localpv/app/helper_hostpath.go Switches helper pod quota scripts to host-path mode, and mounts host root at /host to support nsenter + lockfiles.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cmd/provisioner-localpv/app/quota_scripts.go
Comment thread cmd/provisioner-localpv/app/helper_hostpath.go Outdated
@niladrih

Copy link
Copy Markdown
Member

@fourhu we can't work with Merge commits from the update branch. This is because all of the commits need to carry the DCO signature. You'd want to rebase your PR (also an option on the drop-down in the update-branch GitHub button), instead.

If you could, please remove the Merge commits and rebase your PR.

@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.00%. Comparing base (fe2d77b) to head (78dc46f).
⚠️ Report is 10 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #357   +/-   ##
========================================
  Coverage    54.00%   54.00%           
========================================
  Files            1        1           
  Lines          474      474           
========================================
  Hits           256      256           
  Misses         209      209           
  Partials         9        9           
Flag Coverage Δ
integrationtests 54.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mergify

mergify Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@fourhu
fourhu force-pushed the feature/issue-343 branch from b9344e8 to 8a9fd0e Compare August 18, 2026 02:01
@fourhu

fourhu commented Aug 18, 2026

Copy link
Copy Markdown
Author

@niladrih thanks for the heads-up, my bad. I hit the default Update branch button and that pulled in the merge commits.

Rebased onto develop and dropped those. Both commits should have the DCO sign-off now. Thanks for walking me through it.

@niladrih

Copy link
Copy Markdown
Member

@fourhu There's a merge conflict on the cmd/provisioner-localpv/app/quota_scripts.go file. If you try to rebase again to the top of develop, you'll hit it. Could you resolve the conflict please?

fourhu added 3 commits August 18, 2026 13:37
Run quota tools through nsenter into the host mount namespace and pass
host paths so project quota is applied to the real host filesystem mount.
Helper pods now mount the host root at /host for nsenter and flock.

Fixes openebs#343

Signed-off-by: fourhu <fourhu@foxmail.com>
Init pods only mkdir under /data and do not need the host root.
Quota and cleanup helpers now mount host /proc at /host/proc for
nsenter, and flock on the existing /data bind-mount of parentDir.

Signed-off-by: fourhu <fourhu@foxmail.com>
Rebase onto develop kept host_exec/nsenter and the pipe-safe
chproj -D from openebs#361 instead of the old recursive -R.

Signed-off-by: fourhu <fourhu@foxmail.com>
@fourhu
fourhu force-pushed the feature/issue-343 branch from 8a9fd0e to 78dc46f Compare August 18, 2026 05:38
@fourhu

fourhu commented Aug 18, 2026

Copy link
Copy Markdown
Author

@niladrih done — rebased onto current develop.

The clash was with the XFS cleanup change in #361. Kept nsenter/host_exec, and switched to chproj -D 0 so we don't hang on pipes. DCO is on the new commits as well.

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.

XFS project quota is not applied from provisioner pods due to mount namespace mismatch

3 participants