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.

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

Jump to bottom

feat(spc): add feature to limit overprovisioning of cstor volumes #1577

Merged
merged 6 commits into from
Jan 22, 2020
Merged

feat(spc): add feature to limit overprovisioning of cstor volumes #1577

merged 6 commits into from
Jan 22, 2020

Conversation

sonasingh46
Copy link
Contributor

@sonasingh46 sonasingh46 commented Jan 7, 2020 •
edited
Loading

This PR adds a feature to limit the over-provisioning of a CStor volume on SPC pools and hence fixes the issue openebs/openebs#2855 for cStor volume provisioned on SPC.
A similar feature for CSPC would be introduced in upcoming PRs.

Refer to following doc for solution approach:
https://docs.google.com/document/d/1JFsrZsOiV9GLG4IJ6mDrbNf5flvS1BKExCzI8J3hX2I/edit
Example :

apiVersion: openebs.io/v1alpha1
kind: StoragePoolClaim
metadata:
  name: disk-claim-auto
spec:
  name: disk-claim-auto
  type: disk
  maxPools: 3
  minPools: 3
  poolSpec:
    poolType: striped
    cacheFile: /tmp/disk-claim-auto.cache
    # Over provisioning is disabled.
    overProvisioning: false

All the pools created under above SPC YAML will obey the over provisioning policy ( i.e. disabled).
Consider an example, if a CStor volume is created with 7G of capacity with n replicas then n pools should be available with availableSpaceOnPool>incomingVolumeCapacity(7G in this case) according to below formula:
availableSpaceOnPool:= (PoolCapacity*threshold/100 - Sum of all existing Volumes Capacities on the pool)
(Here threshold is by default 70% and can be overridden with OpenEBS annotations. A separate PR needs to be raised for this threshold overriding ability.)
If n such pools are not found -- the provisioning will fail.

One more point to note is that, if overprovisioning is disabled on SPC then all the pools(CSP) of that SPC will have overprovisioning disabled. There is no such case where one pool of SPC will have overprovisioning disabled and another one enabled.
In short, the overprovisioning policy specified on SPC applies homogenously to all the CSPs(pools) of the SPC.
Signed-off-by: Ashutosh Kumar ashutosh.kumar@openebs.io

Special notes for your reviewer:
Needs an e2e verification of the current feature.
Needs an e2e verification to check for regression(if any) of other existing policies of CVR placement.

ghost
ghost reviewed Jan 7, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Found some fixes!

P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).

pkg/cstor/pool/v1alpha2/cstorpool.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool.go Outdated Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool.go Outdated Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool.go Outdated Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool.go Outdated Show resolved Hide resolved
pkg/algorithm/cstorpoolselect/v1alpha1/select.go Outdated Show resolved Hide resolved
pkg/storagepoolclaim/v1alpha1/storagepoolclaim.go Show resolved Hide resolved
golangcibot
golangcibot reviewed Jan 7, 2020
View reviewed changes
pkg/algorithm/cstorpoolselect/v1alpha1/select.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool_test.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool_test.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool_test.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool_test.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool_test.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool_test.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool_test.go Show resolved Hide resolved
pkg/cstor/pool/v1alpha2/cstorpool_test.go Show resolved Hide resolved
pkg/algorithm/cstorpoolselect/v1alpha1/select.go Outdated Show resolved Hide resolved
feat(spc): add feature to limit overprovisioning of cstor volumes
2880fd8 
Signed-off-by: Ashutosh Kumar <ashutosh.kumar@openebs.io>
@sonasingh46 sonasingh46 self-assigned this Jan 7, 2020
@sonasingh46 sonasingh46 changed the title [WIP]feat(spc): add feature to limit overprovisioning of cstor volumes feat(spc): add feature to limit overprovisioning of cstor volumes Jan 7, 2020
@sonasingh46 sonasingh46 added needs-review pr/release-note PR should be included in release notes labels Jan 7, 2020
@sonasingh46 sonasingh46 mentioned this pull request Jan 7, 2020
Merged
@kmova kmova added this to the 1.7.0 milestone Jan 8, 2020
AmitKumarDas
AmitKumarDas approved these changes Jan 16, 2020
View reviewed changes
Copy link

@AmitKumarDas AmitKumarDas left a comment

Choose a reason for hiding this comment

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

/lgtm

Ashutosh Kumar added 2 commits January 17, 2020 13:01
incorporate review comments
26fbeed 
Signed-off-by: Ashutosh Kumar <ashutosh.kumar@mayadata.io>
incorporate review comments
08a1e9e 
Signed-off-by: Ashutosh Kumar <ashutosh.kumar@mayadata.io>
Ashutosh Kumar added 2 commits January 17, 2020 14:43
incorporate review comments
c7edd02 
Signed-off-by: Ashutosh Kumar <ashutosh.kumar@mayadata.io>
incorporate review comments
8ba92b6 
Signed-off-by: Ashutosh Kumar <ashutosh.kumar@mayadata.io>
mittachaitu
mittachaitu approved these changes Jan 20, 2020
View reviewed changes
Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

changes are good

vishnuitta
vishnuitta reviewed Jan 22, 2020
View reviewed changes
pkg/algorithm/cstorpoolselect/v1alpha1/select.go
var totalcapcity resource.Quantity
cstorVolumeMap := make(map[string]bool)
label := string(apis.CStorPoolKey) + "=" + csp.Name
cstorVolumeReplicaObjList, err := cstorvolumereplica.NewKubeclient().WithNamespace(p.openebsNamespace).List(metav1.ListOptions{LabelSelector: label})
Copy link
Contributor

Choose a reason for hiding this comment

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

does CVRs always stay in openebsNamespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

openebsNamespace is the namespace where openebs is installed. And CVRs always stay in a namespace where openebs is installed.

vishnuitta
vishnuitta reviewed Jan 22, 2020
View reviewed changes
pkg/algorithm/cstorpoolselect/v1alpha1/select.go

// getCStorVolumeCapacity returns the capacity present on a CStorVolume CR.
func (p *scheduleWithOverProvisioningAwareness) getCStorVolumeCapacity(name string) (resource.Quantity, error) {
cv, err := cstorvolume.NewKubeclient().WithNamespace(p.openebsNamespace).Get(name, metav1.GetOptions{})
Copy link
Contributor

Choose a reason for hiding this comment

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

does cv stays in openebsNamespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

incoporate review comments
353d4b6 
Signed-off-by: Ashutosh Kumar <ashutosh.kumar@mayadata.io>
vishnuitta
vishnuitta reviewed Jan 22, 2020
View reviewed changes
pkg/algorithm/cstorpoolselect/v1alpha1/select.go
func getVolumeCapacity(values ...string) (resource.Quantity, error) {
var capacity string
for _, val := range values {
if strings.Contains(val, string(volumeCapacityLabel)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

where are we filling this label with total capacity?

Copy link
Contributor

Choose a reason for hiding this comment

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

ok.. got it.. this is the capacity of the volume to be provisioned

vishnuitta
vishnuitta approved these changes Jan 22, 2020
View reviewed changes
Copy link
Contributor

@vishnuitta vishnuitta left a comment

Choose a reason for hiding this comment

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

changes are good

@vishnuitta vishnuitta merged commit dd5e82d into openebs-archive:master Jan 22, 2020
@chandankumar4 chandankumar4 mentioned this pull request Jan 28, 2020
Merged
@mittachaitu mittachaitu mentioned this pull request Oct 4, 2020
Closed
@mittachaitu mittachaitu mentioned this pull request Apr 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@golangcibot golangcibot golangcibot left review comments

@vishnuitta vishnuitta vishnuitta approved these changes

@mittachaitu mittachaitu mittachaitu approved these changes

@AmitKumarDas AmitKumarDas AmitKumarDas approved these changes

@kmova kmova Awaiting requested review from kmova

Assignees

@sonasingh46 sonasingh46

Labels
needs-review pr/release-note PR should be included in release notes
Projects
None yet
Milestone
1.7.0
Development

Successfully merging this pull request may close these issues.
6 participants
@sonasingh46 @AmitKumarDas @mittachaitu @vishnuitta @golangcibot @kmova