fix: failed to cancel volume expansion after engine is already expanded successfully #3165
Conversation
derekbit
force-pushed
the
issue-9466
branch
from
3f00d2e
to
ade76d6
Compare
…ed successfully
When the volume has already been successfully expanded, if the cancellation is issued after
completing the volume expansion, and the spec.size and status.currentSize of the volume and
engine are as follows:
- v.spec.size: 21474836480
- e.spec.size: 21474836480
- e.status.currentSize: 22548578304
The check
```
if e.Spec.VolumeSize == v.Spec.Size {
return nil
}
```
prevents v.Status.ExpansionRequired from becoming true, which hinders the cancellation
of the expansion.
Longhorn 9466
Signed-off-by: Derek Su <[email protected]>
derekbit
force-pushed
the
issue-9466
branch
from
ade76d6
to
bfc736b
Compare
derekbit
changed the title
| @@ -2026,7 +2026,7 @@ func (c *VolumeController) reconcileVolumeSize(v *longhorn.Volume, e *longhorn.E | |||
| if e == nil || rs == nil { | |||
| return nil | |||
| } | |||
| if e.Spec.VolumeSize == v.Spec.Size { | |||
| if e.Spec.VolumeSize == v.Spec.Size && e.Status.CurrentSize == v.Spec.Size { | |||
There was a problem hiding this comment.
This fix looks ok to me as far as resolving the issue. It seems this fix will allow v.Status.ExpansionRequired == true to be set again, which enables the user to cancel expansion, correcting the volume size to the post-expansion size v.Spec.Size = engine.Status.CurrentSize.
However, I am a bit confused about how v.Status.ExpansionRequired ended up being set to False during cancellation, as described in the issue.
Expanding volume through UI and then Cancelling expansion through UI results in volume stuck in expansion error:
IIRC, we only set v.Status.ExpansionRequired = False when e.Status.CurrentSize == v.Spec.Size.
But from the longhorn/longhorn#9466 (comment), there seems to be an inconsistency between the sizes in spec and status:
- v.spec.size: 21474836480
- e.spec.size: 21474836480
- e.status.currentSize: 22548578304
There was a problem hiding this comment.
While expanding the volume and before cancelling the expansion, ExpansionRequired is true and the sizes are:
- v.spec.size: 22548578304
- e.spec.size: 22548578304
- e.status.currentSize: 21474836480
After issuing the cancellation, v.spec.size and e.spec.size revert to 21474836480, and ExpansionRequired becomes false. Subsequently, the engine controller updates e.status.currentSize to the latest engine size of 22548578304. Therefore, the sizes become:
- v.spec.size: 21474836480
- e.spec.size: 21474836480
- e.status.currentSize: 22548578304
There was a problem hiding this comment.
I see, so this means the cancel volume expansion is ineffective once engineClientProxy.VolumeExpand() is invoked.
|
After discussing with @c3y1huang, the solution is unable to cancel the expansion. Need to figure out the new method. |
Which issue(s) this PR fixes:
Issue longhorn/longhorn#9466
What this PR does / why we need it:
When the volume has already been successfully expanded, if the cancellation is issued after completing the volume expansion, and the spec.size and status.currentSize of the volume and engine are as follows:
The check
prevents v.Status.ExpansionRequired from becoming true, which hinders the cancellation of the expansion.
Special notes for your reviewer:
Additional documentation or context