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

Improve interaction between volume attachment controller and migrations #2894

Merged

Conversation

ejweber
Copy link
Collaborator

@ejweber ejweber commented Jun 19, 2024

Which issue(s) this PR fixes:

longhorn/longhorn#8735

What this PR does / why we need it:

See context at longhorn/longhorn#8735 (comment) and implementation plan at longhorn/longhorn#8735 (comment).

This is a draft for now as I work on testing it in all situations.

@ejweber ejweber marked this pull request as draft June 19, 2024 00:40
@ejweber ejweber force-pushed the 3401-cancel-migration-node-down-agreed-idea branch from 4fa50e7 to fff650e Compare June 19, 2024 00:49
@ejweber ejweber changed the title 3401 cancel migration node down agreed idea Improve interaction between volume attachment controller and migrations Jun 19, 2024
@ejweber ejweber force-pushed the 3401-cancel-migration-node-down-agreed-idea branch 6 times, most recently from 4cb0462 to f267d5b Compare June 24, 2024 14:06
@ejweber
Copy link
Collaborator Author

ejweber commented Jun 24, 2024

The behaviors in this PR now match the test cases from longhorn/longhorn-tests#1948. It is ready for review.

@ejweber ejweber marked this pull request as ready for review June 24, 2024 20:33
@ejweber ejweber force-pushed the 3401-cancel-migration-node-down-agreed-idea branch from 80c4f7f to ffe4a02 Compare June 28, 2024 15:32
@PhanLe1010
Copy link
Contributor

PhanLe1010 commented Jun 28, 2024

I will review this today on Monday

Copy link
Contributor

@PhanLe1010 PhanLe1010 left a comment

Choose a reason for hiding this comment

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

In general LGTM

controller/volume_controller.go Outdated Show resolved Hide resolved
controller/volume_controller.go Show resolved Hide resolved
datastore/longhorn.go Outdated Show resolved Hide resolved
@ejweber ejweber force-pushed the 3401-cancel-migration-node-down-agreed-idea branch from ffe4a02 to e570152 Compare July 2, 2024 16:19
Copy link
Contributor

@PhanLe1010 PhanLe1010 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@shuo-wu shuo-wu left a comment

Choose a reason for hiding this comment

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

In general LGTM

controller/volume_attachment_controller.go Show resolved Hide resolved
datastore/longhorn.go Show resolved Hide resolved
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3041

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
…node

Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
Longhorn 3401

Signed-off-by: Eric Weber <eric.weber@suse.com>
@ejweber ejweber force-pushed the 3401-cancel-migration-node-down-agreed-idea branch from e570152 to b5e93c1 Compare July 8, 2024 20:37
@mergify mergify bot merged commit 8e37595 into longhorn:master Jul 8, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants