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

Remove the longhorn-upgrade attacher type and refactor the upgrade path #2006

Merged
merged 3 commits into from Jun 21, 2023
Merged

Remove the longhorn-upgrade attacher type and refactor the upgrade path #2006

merged 3 commits into from Jun 21, 2023

Conversation

PhanLe1010
Copy link
Contributor

@PhanLe1010
Copy link
Contributor Author

PhanLe1010 commented Jun 19, 2023 •
edited

Need to wait for e2e test run before merging this PR

https://ci.longhorn.io/job/private/job/longhorn-tests-regression/4256/

@PhanLe1010 PhanLe1010 marked this pull request as ready for review June 19, 2023 21:30
@PhanLe1010 PhanLe1010 requested a review from a team as a code owner June 19, 2023 21:30
innobead
innobead previously approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@innobead innobead left a comment

Choose a reason for hiding this comment

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

LGTM

upgrade/v14xto150/upgrade.go Outdated Show resolved Hide resolved
@innobead
Copy link
Member

@mergify backport v1.5.x

@mergify
Copy link

mergify bot commented Jun 20, 2023

backport v1.5.x

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]

derekbit
derekbit previously approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@derekbit derekbit left a comment

Choose a reason for hiding this comment

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

LGTM

@PhanLe1010
Remove the longhorn-upgrade attacher type and refactor the upgrade path
aa24311 
longhorn-6139

Signed-off-by: Phan Le <phan.le@suse.com>
innobead
innobead approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@innobead innobead left a comment

Choose a reason for hiding this comment

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

@PhanLe1010 it's ready to merge as per the e2e testing results?

@PhanLe1010
Copy link
Contributor Author

@innobead The test is still running here https://ci.longhorn.io/job/private/job/longhorn-tests-regression/4256/

@PhanLe1010 PhanLe1010 mentioned this pull request Jun 20, 2023
Closed
@PhanLe1010
Copy link
Contributor Author

PhanLe1010 commented Jun 20, 2023 •
edited

The test result looks good. There some failing tests but they don't seem related to the PR. Except for the one test_migration_with_unscheduled_replica. I am retesting test_migration_with_unscheduled_replica manually

@PhanLe1010
Copy link
Contributor Author

Update: test_migration_with_unscheduled_replica passed when testing manually. Maybe the test case was a bit flaky.

Suggesting merging the PR. Thank you very much @innobead

@innobead
Copy link
Member

cc @longhorn/qa for the flaky test case

@innobead innobead merged commit 4438165 into longhorn:master Jun 21, 2023
5 checks passed
@innobead
Copy link
Member

@mergify backport v1.5.x

@mergify
Copy link

mergify bot commented Jun 21, 2023 •
edited

backport v1.5.x

✅ Backports have been created

@mergify mergify bot mentioned this pull request Jun 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shuo-wu shuo-wu shuo-wu approved these changes

@innobead innobead innobead approved these changes

@derekbit derekbit derekbit left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@PhanLe1010 @innobead @derekbit @shuo-wu