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

PV Resizing: Give precedance to usage percentage #1112

Closed
wants to merge 1 commit into from
Closed

PV Resizing: Give precedance to usage percentage #1112

Show file tree
Hide file tree
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
5 changes: 3 additions & 2 deletions pkg/controller/miganalytic/volume_adjustment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,13 +265,14 @@ func (pva *PersistentVolumeAdjuster) calculateProposedVolumeSize(usagePercentage
reason = migapi.VolumeAdjustmentNoOp

if actualCapacity.Cmp(maxSize) == 1 {
maxSize = actualCapacity
reason = migapi.VolumeAdjustmentCapacityMismatch
}

if volumeSizeWithThreshold.Cmp(maxSize) == 1 {
maxSize = volumeSizeWithThreshold
reason = migapi.VolumeAdjustmentUsageExceeded
if usagePercentage+int64(pva.getVolumeUsagePercentageThreshold()) > 100 {
reason = migapi.VolumeAdjustmentUsageExceeded
Copy link
Contributor

@djwhatle djwhatle May 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Question: do we let users know that we are planning to downsize their volumes from the provisioned capacity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@djwhatle Good point. No we do not.

}
}

proposedSize = maxSize
Expand Down
13 changes: 6 additions & 7 deletions pkg/controller/miganalytic/volume_adjustment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func TestPersistentVolumeAdjuster_calculateProposedVolumeSize(t *testing.T) {
wantReason string
}{
{
name: "Given values of usagePercentage, actualCapacity and requestedCapacity, appropriate volume size is returned, here proposed size = actual capacity",
name: "Given requested capacity < actual capacity, should return 103% of used capacity of the volume",
fields: fields{
Client: fake.NewFakeClient(),
},
Expand All @@ -270,11 +270,11 @@ func TestPersistentVolumeAdjuster_calculateProposedVolumeSize(t *testing.T) {
actualCapacity: resource.MustParse("200"),
requestedCapacity: resource.MustParse("20"),
},
wantProposedSize: resource.MustParse("200"),
wantProposedSize: resource.MustParse("106"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that 103% is desired but wantProposedSize: "106", is this a typo?

Copy link
Contributor Author

@pranavgaikwad pranavgaikwad May 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, a typo and a logic flaw :) Good catch

Copy link
Contributor Author

@pranavgaikwad pranavgaikwad May 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Ohh now that I realized, this is not a logic issue. 106 is the correct value, the reference for usage percentage is the actual volume size which is 200 in this case, which makes the 3% = 6Gi buffer. We cannot use requested size as a reference point as it could differ from actual capacity.

wantReason: migapi.VolumeAdjustmentCapacityMismatch,
},
{
name: "Given values of usagePercentage, actualCapacity and requestedCapacity, appropriate proposed volume size is returned, here proposed size = volume with threshold size",
name: "Given volume usage close to 100%, should return 103% of original capacity",
fields: fields{
Client: fake.NewFakeClient(),
},
Expand All @@ -287,7 +287,7 @@ func TestPersistentVolumeAdjuster_calculateProposedVolumeSize(t *testing.T) {
wantReason: migapi.VolumeAdjustmentUsageExceeded,
},
{
name: "Given values of usagePercentage, actualCapacity and requestedCapacity, appropriate proposed volume size is returned, here proposed size = requested capacity",
name: "Given requested capacity > actual capacity, should return requested capacity",
fields: fields{
Client: fake.NewFakeClient(),
},
Expand All @@ -300,8 +300,7 @@ func TestPersistentVolumeAdjuster_calculateProposedVolumeSize(t *testing.T) {
wantReason: migapi.VolumeAdjustmentNoOp,
},
{
name: "Given values of usagePercentage, actualCapacity and requestedCapacity, appropriate volume size is returned, here volumeS size with threshold" +
"is greater than requested capacity and actual capacity is greater than requested capacity",
name: "Given requested capacity < actual capacity and usage not close to 100%, should return requested capacity",
fields: fields{
Client: fake.NewFakeClient(),
},
Expand All @@ -310,7 +309,7 @@ func TestPersistentVolumeAdjuster_calculateProposedVolumeSize(t *testing.T) {
actualCapacity: resource.MustParse("200"),
requestedCapacity: resource.MustParse("150"),
},
wantProposedSize: resource.MustParse("200"),
wantProposedSize: resource.MustParse("150"),
wantReason: migapi.VolumeAdjustmentCapacityMismatch,
},
}
Expand Down