Skip to content

plugins: Add rollback support for failed plugin updates - #6179

Open
skools-here wants to merge 17 commits into
kubernetes-sigs:mainfrom
skools-here:fix-plugin-update-rollback
Open

plugins: Add rollback support for failed plugin updates#6179
skools-here wants to merge 17 commits into
kubernetes-sigs:mainfrom
skools-here:fix-plugin-update-rollback

Conversation

@skools-here

Copy link
Copy Markdown
Contributor

Summary

This PR makes plugin updates transactional by introducing a backup-and-rollback mechanism in the plugin update flow.

Previously, PluginManager.update() removed the existing plugin before moving the newly downloaded version into place. If the move operation failed due to filesystem errors, insufficient permissions, or disk space issues, the original plugin was permanently lost. This change ensures that the existing plugin is preserved and automatically restored if the update process fails.

Related Issue

Fixes #6174

Changes

  • Added backup-and-rollback logic to the plugin update workflow.
  • Created a temporary backup of the existing plugin before applying updates.
  • Restored the original plugin automatically when update operations fail.
  • Added unit tests covering successful updates, rollback behavior, and failure scenarios.
  • Improved update reliability by preventing plugin loss during failed updates.

@kubernetes-prow
kubernetes-prow Bot requested a review from kahirokunn June 24, 2026 13:13
@kubernetes-prow kubernetes-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 24, 2026
@kubernetes-prow
kubernetes-prow Bot requested a review from yolossn June 24, 2026 13:13
@kubernetes-prow kubernetes-prow Bot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 24, 2026
@skools-here

Copy link
Copy Markdown
Contributor Author

ptal @illume

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR makes plugin updates safer by adding a backup-and-rollback mechanism so an existing plugin isn’t lost if installing the new version fails mid-update.

Changes:

  • Update flow now renames the existing plugin directory to a .backup directory before moving the new plugin into place, and restores it on failure.
  • Adds cleanup logic for temporary extraction directories on failure.
  • Adds Electron unit tests covering successful updates, rollback on move failure, and failures before backup creation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
plugins/pluginctl/src/plugin-management.js Adds backup/rollback behavior to transactionalize CLI plugin updates.
app/electron/plugin-management.ts Adds backup/rollback behavior to transactionalize Electron plugin updates.
app/electron/plugin-management.test.ts Adds unit tests for update success, rollback, and pre-backup failure scenarios.

Comment thread plugins/pluginctl/src/plugin-management.js
Comment thread app/electron/plugin-management.ts
@skools-here

Copy link
Copy Markdown
Contributor Author

ptal @illume

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread plugins/pluginctl/src/plugin-management.js
Comment thread app/electron/plugin-management.test.ts Outdated
Comment thread app/electron/plugin-management.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comment thread app/electron/plugin-management.ts
Comment thread app/electron/plugin-management.ts
Comment thread plugins/pluginctl/src/plugin-management.js
Comment thread plugins/pluginctl/src/plugin-management.js

@illume illume left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this PR.

The open review comments from Copilot still need attention — can you have a look? Once addressed, please mark them as resolved.

@kubernetes-prow kubernetes-prow Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 26, 2026
@illume
illume requested a review from Copilot June 26, 2026 16:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread plugins/pluginctl/src/plugin-management.js Outdated
Comment thread plugins/pluginctl/src/plugin-management.js
@skools-here
skools-here requested a review from illume June 26, 2026 18:55
@skools-here

Copy link
Copy Markdown
Contributor Author

ptal @illume

@illume
illume requested a review from Copilot June 28, 2026 10:10
@skools-here
skools-here force-pushed the fix-plugin-update-rollback branch from 72955b1 to 5cd4999 Compare July 5, 2026 11:38
@illume
illume requested a review from Copilot July 5, 2026 12:12

@illume illume left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this.

Looks like there are some lint errors in the frontend — cd frontend && npm run lint will show them.

How to fix lint errors

Run cd frontend && npm run lint to see all ESLint errors. Many can be fixed automatically with cd frontend && npm run lint -- --fix. Remaining errors need manual attention.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread plugins/pluginctl/src/plugin-management.test.js

@illume illume left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this PR.

The open review comments from Copilot still need attention — can you have a look? Once addressed, please mark them as resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread plugins/pluginctl/src/plugin-management.js
Comment thread app/electron/plugin-management.ts
@kubernetes-prow kubernetes-prow Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 6, 2026
@illume
illume requested a review from Copilot July 7, 2026 09:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment on lines +164 to +183
test('Error propagation when progressCallback is provided', async () => {
const errorCallback = jest.fn();

// install should resolve to undefined when progressCallback is provided, as it catches the error and reports it via callback
await expect(
PluginManager.install(
'https://artifacthub.io/packages/headlamp/headlamp-plugins/non-existent-plugin',
tempDir,
'',
errorCallback
)
).resolves.toBeUndefined();

expect(errorCallback).toHaveBeenCalledWith(
expect.objectContaining({
type: 'error',
message: expect.stringContaining('HTTP error! status: 404'),
})
);
});
Comment on lines +101 to +105
await PluginManager.install(
'https://artifacthub.io/packages/headlamp/headlamp-plugins/headlamp_flux',
tempDir,
'',
mockProgressCallback
Comment on lines +239 to +245
if (progressCallback) {
try {
progressCallback({ type: 'success', message: 'Plugin Updated' });
} catch (callbackErr) {
console.error('Progress callback failed:', callbackErr);
}
}

@illume illume left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this.

Would you mind addressing the open Copilot review comments? Please mark each comment as resolved after addressing it.

@illume illume added app kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. plugins testing labels Jul 25, 2026

@illume illume left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the contribution.

This PR has git conflicts — could you resolve them?

How to resolve conflicts

Rebase or merge the latest main into your branch, resolve the conflicts, and push the updated branch.

@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: skools-here
Once this PR has been reviewed and has the lgtm label, please assign yolossn for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 5, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. plugins size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugin update is not atomic and can leave plugin unusable if moveDirs fails

4 participants