Skip to content

Add coreutils package to support sparse file#291

Merged
k8s-ci-robot merged 1 commit into
kubernetes-csi:masterfrom
stoneshi-yunify:master
May 19, 2021
Merged

Add coreutils package to support sparse file#291
k8s-ci-robot merged 1 commit into
kubernetes-csi:masterfrom
stoneshi-yunify:master

Conversation

@stoneshi-yunify

@stoneshi-yunify stoneshi-yunify commented May 13, 2021

Copy link
Copy Markdown
Contributor

Signed-off-by: stoneshi-yunify stoneshi@yunify.com

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change

/kind bug

/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
Without this PR, the cloning of a hostpath volume containing linux spare file could be very slow and the resulting file can be very large.

The cp from coreutils will optimize the cloning dramatically.

Which issue(s) this PR fixes:

Fixes #279

Special notes for your reviewer:

  • Adding coreutils does not require any code change of hostpath controller/node.
  • After adding the coreutils, the hostpath image is now 30.8MB VS 28.5MB before (v1.6.2).

Does this PR introduce a user-facing change?:

Optimize sparse file cloning

@stoneshi-yunify
add coreutils package
311b0e5 
Signed-off-by: stoneshi-yunify <stoneshi@yunify.com>
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 13, 2021
@k8s-ci-robot k8s-ci-robot requested review from pohly and saad-ali May 13, 2021 02:48
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 13, 2021
@k8s-ci-robot

k8s-ci-robot commented May 13, 2021

Copy link
Copy Markdown
Contributor

Hi @stoneshi-yunify. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 13, 2021
@stoneshi-yunify

stoneshi-yunify commented May 13, 2021

Copy link
Copy Markdown
Contributor Author

/ok-to-test

@k8s-ci-robot

k8s-ci-robot commented May 13, 2021

Copy link
Copy Markdown
Contributor

@stoneshi-yunify: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/ok-to-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@pohly

pohly commented May 14, 2021

Copy link
Copy Markdown
Contributor

Have you tested that this works?

@stoneshi-yunify

stoneshi-yunify commented May 14, 2021

Copy link
Copy Markdown
Contributor Author

I made a docker image based on my change, loaded it, and cloned a volume containing sparse file (a virtual machine image), then checked the cloned volume, everything was fine.

@pohly

pohly commented May 14, 2021

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@k8s-ci-robot

k8s-ci-robot commented May 14, 2021

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pohly, stoneshi-yunify

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 14, 2021
@k8s-ci-robot

k8s-ci-robot commented May 14, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

@stoneshi-yunify: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubernetes-csi-csi-driver-host-path-1-20-test-on-kubernetes-1-20 311b0e5 link /test pull-kubernetes-csi-csi-driver-host-path-1-20-test-on-kubernetes-1-20
pull-kubernetes-csi-csi-driver-host-path-1-21-on-kubernetes-1-21 311b0e5 link /test pull-kubernetes-csi-csi-driver-host-path-1-21-on-kubernetes-1-21

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@pohly

pohly commented May 19, 2021

Copy link
Copy Markdown
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 19, 2021
@k8s-ci-robot k8s-ci-robot merged commit 597da4a into kubernetes-csi:master May 19, 2021
dfajmon added a commit to dfajmon/csi-driver-host-path that referenced this pull request Jan 13, 2026
@dfajmon
Squashed 'release-tools/' changes from b12e407c..707a99ec
6eb72ed 
707a99ec Merge pull request kubernetes-csi#291 from dfajmon/logcheck
a9d2b0fb Bump logcheck to v0.10.0
d6846630 Merge pull request kubernetes-csi#290 from dfajmon/go-1.25.5
55e527c4 Bump golang to 1.25.5

git-subtree-dir: release-tools
git-subtree-split: 707a99ecae026a8faa36d60aa9ea88505b5865df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pohly pohly Awaiting requested review from pohly

@saad-ali saad-ali Awaiting requested review from saad-ali

Assignees

@pohly pohly

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

clone volume: cp doesn't support sparse file

3 participants

@stoneshi-yunify @k8s-ci-robot @pohly