Skip to content

Fix admin cutover#862

Merged
spai-p9 merged 10 commits intomainfrom
private/main/fix-admin-cutover
Sep 3, 2025
Merged

Fix admin cutover#862
spai-p9 merged 10 commits intomainfrom
private/main/fix-admin-cutover

Conversation

@spai-p9
Copy link
Copy Markdown
Collaborator

@spai-p9 spai-p9 commented Sep 2, 2025

What this PR does / why we need it

Which issue(s) this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged)

fixes #859

Special notes for your reviewer

Testing done

image image

All these were done using admin cutover.
please add testing details (logs, screenshots, etc.)

Summary by Bito

This pull request refines the migration process by addressing pod reference updates, admin cutover logic, and error reporting. It enhances the migration controller's logging and state handling, integrates a Reporter component for proper assessments, and prevents further disk copying when an admin cutover is initiated. The changes also include renaming functions, revising error conditions, and improving traceability for safer transitions. Additionally, it fixes a critical bug in guest network validation logic by updating the condition to check for network length rather than using a nil check.

Copy link
Copy Markdown
Contributor

@windsurf-bot windsurf-bot Bot left a comment

Choose a reason for hiding this comment

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

💡 To request another review, post a new comment with "/windsurf-review".

Comment thread v2v-helper/migrate/migrate.go Outdated
Comment thread k8s/migration/internal/controller/migration_controller.go
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Sep 2, 2025

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Bug Fix - Resolved admin cutover issues and migration inconsistencies

migration_controller.go - Enhanced logging and pod reference update for accurate migration state tracking.

migrationplan_controller.go - Removed redundant PodRef initialization to delegate pod reference handling to the migration controller.

credutils.go - Refactored guest network IP extraction with fallback logic to preserve existing IP addresses when VMware data is missing.

main.go - Integrated Reporter into the main migration flow to support enhanced event logging.

migrate.go - Implemented admin cutover checks to halt disk copying and manage VM power operations appropriately.

reporter.go - Added GetPodLabels method to improve the retrieval of pod metadata for migration reporting.

Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot left a comment

Choose a reason for hiding this comment

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

Code Review Agent Run #2758bc

Actionable Suggestions - 2
  • v2v-helper/migrate/migrate.go - 1
    • Non-blocking select causes missed channel messages · Line 246-255
  • k8s/migration/internal/controller/migrationplan_controller.go - 1
    • Removed required field initialization without replacement · Line 481-486
Review Details
  • Files reviewed - 3 · Commit Range: 6d7edb4..52a39fa
    • k8s/migration/internal/controller/migration_controller.go
    • k8s/migration/internal/controller/migrationplan_controller.go
    • v2v-helper/migrate/migrate.go
  • Files skipped - 0
  • Tools
    • Golangci-lint (Linter) - ✖︎ Failed
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at mithil@platform9.com.

Documentation & Help

AI Code Review powered by Bito Logo

Comment thread v2v-helper/migrate/migrate.go Outdated
Comment thread k8s/migration/internal/controller/migrationplan_controller.go Outdated
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Sep 2, 2025

Code Review Agent Run #8cf910

Actionable Suggestions - 0
Additional Suggestions - 1
  • v2v-helper/main.go - 1
    • Redundant Reporter field in Migrate struct · Line 146-146
      The `Reporter` field is added to the `Migrate` struct but not used in the `MigrateVM` method. This creates a redundant field since the same functionality is already handled by `EventReporter` and `PodLabelWatcher` channels.
      Code suggestion
       @@ -145,7 +145,6 @@
        		SecurityGroups:         utils.RemoveEmptyStrings(strings.Split(migrationparams.SecurityGroups, ",")),
        		UseFlavorless:          os.Getenv("USE_FLAVORLESS") == "true",
        		TenantName:             openstackProjectName,
      -		Reporter:               eventReporter,
        	}
       
        	if err := migrationobj.MigrateVM(ctx); err != nil {
Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

  • v2v-helper/migrate/migrate.go - 1
    • Contradictory function name and implementation logic · Line 254-256
Review Details
  • Files reviewed - 3 · Commit Range: 52a39fa..7c17c85
    • v2v-helper/main.go
    • v2v-helper/migrate/migrate.go
    • v2v-helper/reporter/reporter.go
  • Files skipped - 0
  • Tools
    • Golangci-lint (Linter) - ✖︎ Failed
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at mithil@platform9.com.

Documentation & Help

AI Code Review powered by Bito Logo

Copy link
Copy Markdown
Collaborator

@OmkarDeshpande7 OmkarDeshpande7 left a comment

Choose a reason for hiding this comment

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

Please add testing details

Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot left a comment

Choose a reason for hiding this comment

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

Code Review Agent Run #aca27f

Actionable Suggestions - 1
  • v2v-helper/migrate/migrate.go - 1
    • Duplicate admin cutover logic causes redundant operations · Line 335-346
Review Details
  • Files reviewed - 1 · Commit Range: 7c17c85..0c5ed16
    • v2v-helper/migrate/migrate.go
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Golangci-lint (Linter) - ✖︎ Failed

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at mithil@platform9.com.

Documentation & Help

AI Code Review powered by Bito Logo

Comment on lines +335 to +346
if adminInitiatedCutover {
utils.PrintLog("Admin initiated cutover detected, skipping changed blocks copy")
if err := migobj.WaitforAdminCutover(); err != nil {
return vminfo, errors.Wrap(err, "failed to start VM Cutover")
}
utils.PrintLog("Shutting down source VM and performing final copy")
err = vmops.VMPowerOff()
if err != nil {
return vminfo, errors.Wrap(err, "failed to power off VM")
}
final = true
}
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.

Duplicate admin cutover logic causes redundant operations

The admin-initiated cutover logic is duplicated in two places. The same check and actions are performed here and later at lines 413-426, causing redundant VM power off operations.

Code suggestion
Check the AI-generated fix before applying
Suggested change
if adminInitiatedCutover {
utils.PrintLog("Admin initiated cutover detected, skipping changed blocks copy")
if err := migobj.WaitforAdminCutover(); err != nil {
return vminfo, errors.Wrap(err, "failed to start VM Cutover")
}
utils.PrintLog("Shutting down source VM and performing final copy")
err = vmops.VMPowerOff()
if err != nil {
return vminfo, errors.Wrap(err, "failed to power off VM")
}
final = true
}

Code Review Run #aca27f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Sep 2, 2025

Bito Review Skipped - No Changes Detected

Bito didn't review this pull request because we did not detect any changes in the pull request to review.

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Sep 2, 2025

Code Review Agent Run #9600b7

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 0c5ed16..fa800a5
    • k8s/migration/pkg/utils/credutils.go
  • Files skipped - 0
  • Tools
    • Golangci-lint (Linter) - ✖︎ Failed
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at mithil@platform9.com.

Documentation & Help

AI Code Review powered by Bito Logo

Copy link
Copy Markdown
Collaborator

@OmkarDeshpande7 OmkarDeshpande7 left a comment

Choose a reason for hiding this comment

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

lgtm

@spai-p9 spai-p9 enabled auto-merge (squash) September 3, 2025 09:25
@spai-p9 spai-p9 merged commit e66512e into main Sep 3, 2025
12 checks passed
@spai-p9 spai-p9 deleted the private/main/fix-admin-cutover branch September 3, 2025 09:34
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Sep 3, 2025

Bito Automatic Review Failed - Technical Failure

Bito encountered technical difficulties while generating code feedback . To retry, type /review in a comment and save. If the issue persists, contact support@bito.ai and provide the following details:

Agent Run ID: 3bd4b6fe-3dd6-47b5-a372-c260f193a4f3

rishabh625 pushed a commit to rishabh625/vjailbreak that referenced this pull request Sep 5, 2025
@DazGaming
Copy link
Copy Markdown

Hi Team,
This admin cutover option fails after 1 hr or for longer duration can you check and update on it

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.

Admin cutover via UI sometimes fails intermittently

3 participants