-
Notifications
You must be signed in to change notification settings - Fork 316
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
Allow extending volume size with inUse status #1174
base: development
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## development #1174 +/- ##
===============================================
- Coverage 34.82% 34.61% -0.21%
===============================================
Files 97 97
Lines 17622 17656 +34
===============================================
- Hits 6137 6112 -25
- Misses 10614 10672 +58
- Partials 871 872 +1
|
|
return | ||
} | ||
if pool.Status != model.PoolAvailable { | ||
errMsg := fmt.Sprintf("pool:%s with status:%s can not support extending volume", pool.Name, pool.Status) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the message be
errMsg := fmt.Sprintf("pool:%s with status:%s can not support extending volume", pool.Name, pool.Status) | |
errMsg := fmt.Sprintf("pool:%s has status:%s. Can not extend volume", pool.Name, pool.Status) |
if vol.Status != model.VolumeAvailable && vol.Status != model.VolumeInUse { | ||
errMsg := "only the status of volume is available or in-use, the snapshot can be created" | ||
if vol.Status != model.VolumeAvailable { | ||
errMsg := "only the status of volume is available, the snapshot can be created" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is these messages correct?
Fix has two parts, one for extending the size of volume and other to remove the attached/detached state from volume list. For the second part, don't we need the attached/detached state on volume also. Someone can go and check the volume list to see if that volume is attached or detached. Not from host list ofcourse |
volume still has the property named `attached' to display whether the volume is attached or not |
What this PR does / why we need it:
This PR will enable the feature of extending volume size with in use status. There is an impact on volume status. Before this PR, the volume status includes:
Since opensds already has attachment api, the attach status is redundant in volume. Therefore, I plan to remove them. After this PR, the status of volume will only include the following status. It will become more clear for volume.
Which issue this PR fixes: fixes #1126
Special notes for your reviewer:
Release note: