Skip to content
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

dataset support pvc storageSize #4178

Merged

Conversation

wangshulei098
Copy link
Contributor

@wangshulei098 wangshulei098 commented Jun 24, 2024

Ⅰ. Describe what this PR does

The generated PVC and PV have a fixed size of 100 Pi and cannot be changed. In a namespace with a Resourcequota set for storage, it is not possible to create such large PV and PVC. Therefore, the size needs to be configurable.

Ⅱ. Does this pull request fix one issue?

fixes #4177

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

“pvc.fluid.io/resources.requests.storage” as the annotation key on Dataset to specify the desired storage size for the generated PVC. If not set, or if the storage size format is incorrect, it will be set to the default value of 100Pi.

Ⅴ. Special notes for reviews

Copy link

fluid-e2e-bot bot commented Jun 24, 2024

Hi @wangshulei098. Thanks for your PR.

I'm waiting for a fluid-cloudnative 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.

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.

api/v1alpha1/dataset_types.go Outdated Show resolved Hide resolved
pkg/utils/dataset/volume/create.go Outdated Show resolved Hide resolved
@wangshulei098 wangshulei098 changed the title dataset add storageSize [WIP]dataset add storageSize Jun 25, 2024
@wangshulei098 wangshulei098 force-pushed the feat/storagesize branch 6 times, most recently from 650163e to 23e0877 Compare June 25, 2024 07:05
@wangshulei098 wangshulei098 changed the title [WIP]dataset add storageSize dataset add storageSize Jun 25, 2024

// StorageCapacity is the storage size of generated PVC and PV
// +optional
StorageCapacity resource.Quantity `json:"storageCapacity,omitempty"`
Copy link
Member

@TrafalgarZZZ TrafalgarZZZ Jun 27, 2024

Choose a reason for hiding this comment

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

Do we have to add a new field to the Dataset CRD? StorageCapacity is just informational in Fluid, might be confusing with some other concept like tieredstore.quota. How about following PR#3146? So we can override storage capacity with a given annotation on Dataset? WDYT @wangshulei098 @cheyang

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point. Agree. I think we can use the annotation key pvc.fluid.io/resources.request.storageSize to specify the desired storage size for the PVC. This key is descriptive and aligns with the Kubernetes convention for resource requests.

It can provide flexibility to extend the customization of PVC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,I will change it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have used “pvc.fluid.io/resources.requests.storage” as the annotation key on Dataset to specify the desired storage size for the generated PVC. If not set, or if the storage size format is incorrect, it will be set to the default value of 100Pi.

@wangshulei098 wangshulei098 changed the title dataset add storageSize [WIP] dataset support pvc storageSize Jun 27, 2024
@wangshulei098 wangshulei098 changed the title [WIP] dataset support pvc storageSize dataset support pvc storageSize Jun 27, 2024
pkg/utils/dataset.go Outdated Show resolved Hide resolved
pkg/utils/dataset.go Show resolved Hide resolved
pkg/utils/dataset.go Outdated Show resolved Hide resolved
pkg/utils/dataset.go Show resolved Hide resolved
pkg/utils/dataset.go Outdated Show resolved Hide resolved
pkg/utils/dataset.go Show resolved Hide resolved
pkg/utils/dataset.go Show resolved Hide resolved
Signed-off-by: wangshulei098 <[email protected]>
@cheyang
Copy link
Collaborator

cheyang commented Jun 29, 2024

/test fluid-e2e

@cheyang
Copy link
Collaborator

cheyang commented Jun 30, 2024

@wangshulei098 Thanks for your contributions! @TrafalgarZZZ please also take a look. Thanks.

Copy link
Member

@TrafalgarZZZ TrafalgarZZZ left a comment

Choose a reason for hiding this comment

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

Thanks @wangshulei098 for supporting this! LGTM
/lgtm

@cheyang
Copy link
Collaborator

cheyang commented Jul 1, 2024

@wangshulei098 Would you mind developing document for this? Thanks. It can be next PR.

Copy link
Collaborator

@cheyang cheyang left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Copy link

fluid-e2e-bot bot commented Jul 1, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheyang, TrafalgarZZZ

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [TrafalgarZZZ,cheyang]

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

@fluid-e2e-bot fluid-e2e-bot bot merged commit 2680812 into fluid-cloudnative:master Jul 1, 2024
8 checks passed
cheyang pushed a commit to Ametsuji-akiya/fluid that referenced this pull request Jul 16, 2024
Signed-off-by: wangshulei098 <[email protected]>
Signed-off-by: cheyang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURES] Support PVC and PV stroage size configuration
3 participants