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

[TASK] Remove a deprecated feature disable-replica-rebuild from longhorn-manager #4997

Closed
innobead opened this issue Dec 5, 2022 · 7 comments
Assignees
Labels
area/setting Global setting or volume setting component/longhorn-manager Longhorn manager (control plane) kind/task General task request to fulfill another primary request priority/0 Must be fixed in this release (managed by PO) release/obsolete-compatibility If the fix introduces the code for backward compatibility: Has a separate issue been filed with the require/doc Require updating the longhorn.io documentation
Milestone

Comments

@innobead
Copy link
Member

innobead commented Dec 5, 2022

What's the task? Please describe

disable-replica-rebuild has been deprecated for several feature releases. For any deprecated feature, it will only exist for one feature release, so it should be removed from the following feature release.

Describe the items of the task (DoD, definition of done) you'd like

Additional context

N/A

cc @PhanLe1010

@innobead innobead added component/longhorn-manager Longhorn manager (control plane) release/obsolete-compatibility If the fix introduces the code for backward compatibility: Has a separate issue been filed with the kind/task General task request to fulfill another primary request labels Dec 5, 2022
@innobead innobead added this to the v1.4.0 milestone Dec 5, 2022
@innobead innobead added the area/setting Global setting or volume setting label Dec 5, 2022
@innobead innobead modified the milestones: v1.4.0, v1.5.0 Dec 20, 2022
@weizhe0422
Copy link
Contributor

weizhe0422 commented Dec 22, 2022

I found that this setting is still be used here when upgrading from 1.2.0 to 1.2.1, and its default setting value is false.

We may need to wait for this issue to be completed to enforce a user upgrade path before we can safely remove this setting.

@innobead
Copy link
Member Author

ref: #5131

@ejweber
Copy link
Contributor

ejweber commented May 11, 2023

  • Remove all traces from longhorn repo
  • Regenerate longhorn.yaml
  • Remove all traces from longhorn-manager repo (left local version in upgrade/v120to121 for now)
  • Check for traces in longhorn-tests repo (none found)
  • Run standard integration tests (used "-m coretest")
  • Run upgrade integration tests (upgraded from v1.4.1 and used "-m coretest")
  • Make changes to website

@ejweber
Copy link
Contributor

ejweber commented May 11, 2023

@longhorn-io-github-bot
Copy link

longhorn-io-github-bot commented May 11, 2023

Pre Ready-For-Testing Checklist

  • Where is the reproduce steps/test steps documented?
  • All existing integration and upgrade integration tests should pass.
  • The "Disable Replica Rebuild" setting should no longer appear in the UI. (This is actually not a good check, as the Type for this setting was previously set to SettingTypeDeprecated, meaning the UI was already hiding it.
  • On a clean install, kubectl get -n longhorn-system setting shows no disable-replica-rebuild setting.
  • On upgrade, kubectl get -n longhorn-system setting shows no disable-replica-rebuild setting.
  • Is there a workaround for the issue? If so, where is it documented?

  • Does the PR include the explanation for the fix or the feature?

  • Does the PR include deployment change (YAML/Chart)? If so, where are the PRs for both YAML file and Chart?
    The PR for the YAML change is at: Remove disable-replica-rebuild setting #5919.
    The PR for the chart change is at: Remove disable-replica-rebuild setting #5919.

  • Have the backend code been merged (Manager, Engine, Instance Manager, BackupStore etc) (including backport-needed/*)?
    The PR is at: Remove disable-replica-rebuild setting longhorn-manager#1899.

  • Which areas/issues this PR might have potential impacts on?

  • If labeled: require/LEP Has the Longhorn Enhancement Proposal PR submitted?

  • If labeled: area/ui Has the UI issue filed or ready to be merged (including backport-needed/*)?

  • If labeled: require/doc Has the necessary document PR submitted or merged (including backport-needed/*)?
    The documentation issue/PR is at: Remove disable-replica-rebuild setting website#698.

  • If labeled: require/automation-e2e Has the end-to-end test plan been merged? Have QAs agreed on the automation test case? If only test case skeleton w/o implementation, have you created an implementation issue (including backport-needed/*)?

  • If labeled: require/automation-engine Has the engine integration test been merged (including backport-needed/*)?

  • If labeled: require/manual-test-plan Has the manual test plan been documented?

  • If the fix introduces the code for backward compatibility Has a separate issue been filed with the label release/obsolete-compatibility?

@ejweber ejweber added the require/doc Require updating the longhorn.io documentation label May 12, 2023
@yangchiu
Copy link
Member

  • All existing integration and upgrade integration tests should pass.

This item is pending by longhorn/longhorn-tests#1357. Currently upgrade test always fail on master-head.

@innobead innobead assigned chriscchien and unassigned yangchiu May 25, 2023
@chriscchien
Copy link
Contributor

Verified pass in longhon master(longhorn-manager eb26ae)

Setting grep disable-replica-rebuild no longer exist in below install scenario

  • Fresh install longhorn master-head
  • Upgrade longhorn from v1.4.2 to master-head
  • Upgrade longhorn from v1.3.3 to v1.4.2 to master-head

Daily regression did not see cascading failure related to this deprecated setting value

  • master-head build 497
  • upgrade test pipeline build 478
  • 2 stage upgrade build 187

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/setting Global setting or volume setting component/longhorn-manager Longhorn manager (control plane) kind/task General task request to fulfill another primary request priority/0 Must be fixed in this release (managed by PO) release/obsolete-compatibility If the fix introduces the code for backward compatibility: Has a separate issue been filed with the require/doc Require updating the longhorn.io documentation
Projects
None yet
Development

No branches or pull requests

7 participants