feat: #18 implement vm migrating feature #24
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a live migration feature for KubeVirt virtual machines (issue #18). The migration capability allows VMs to be moved between nodes while running, enhancing operational flexibility and enabling maintenance workflows.
Key changes include:
- Added
migrate()method to VirtualMachine class to create and submit VirtualMachineInstanceMigration resources - Enhanced the VM details page with real-time pod/node tracking via watch streams and a new "Live Migrate" action button
- Improved status display using color-coded chips and added Node information to the details view
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| kubevirt/src/kubevirt/VirtualMachines/VirtualMachine.tsx | Adds the core migrate() method to create migration resources via KubeVirt API |
| kubevirt/src/kubevirt/VirtualMachines/Details.tsx | Implements UI for migration action, adds real-time pod/node tracking with watch streams, and enhances status visualization |
| kubevirt/.eslintcache | Updated ESLint cache reflecting the code changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| await ApiProxy.request( | ||
| `/apis/kubevirt.io/v1/namespaces/${this.getNamespace()}/virtualmachineinstancemigrations`, | ||
| { | ||
| method: 'POST', | ||
| body: JSON.stringify(migration), | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| } | ||
| ); | ||
| return migrationName; | ||
| } catch (error) { | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
The catch block re-throws the error without adding any context or handling. Consider either removing the try-catch block entirely or adding meaningful error context before re-throwing.
| try { | |
| await ApiProxy.request( | |
| `/apis/kubevirt.io/v1/namespaces/${this.getNamespace()}/virtualmachineinstancemigrations`, | |
| { | |
| method: 'POST', | |
| body: JSON.stringify(migration), | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| return migrationName; | |
| } catch (error) { | |
| throw error; | |
| } | |
| await ApiProxy.request( | |
| `/apis/kubevirt.io/v1/namespaces/${this.getNamespace()}/virtualmachineinstancemigrations`, | |
| { | |
| method: 'POST', | |
| body: JSON.stringify(migration), | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| return migrationName; |
| queryParams.append('watch', 'true'); | ||
| const url = `/api/v1/namespaces/${namespace}/pods?${queryParams.toString()}`; | ||
|
|
||
| const onStream = (result, disconnect, error) => { |
There was a problem hiding this comment.
The stream callback parameters lack type annotations. Add proper TypeScript types for the parameters: result: any, disconnect: () => void, error?: Error.
| ); | ||
| const eventUrl = `/api/v1/namespaces/${namespace}/events?${eventQueryParams.toString()}`; | ||
|
|
||
| const onEventStream = (result, disconnect, error) => { |
There was a problem hiding this comment.
The event stream callback parameters lack type annotations. Add proper TypeScript types for the parameters: result: any, disconnect: () => void, error?: Error.
| description={t('Live Migrate')} | ||
| icon="mdi:swap-horizontal" | ||
| onClick={() => { | ||
| console.log('Starting live migration for ' + item.getName()); |
There was a problem hiding this comment.
Console.log statement should be removed from production code or replaced with proper logging. Consider using a logging library or removing this debug statement.
| console.log('Starting live migration for ' + item.getName()); |
chifu1234
force-pushed
the
feat/18
branch
2 times, most recently
from
2a9b607 to
8ce73bc
Compare
No description provided.