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

🐞 Fix Negative number for plan/migration status VMs count #1319

Conversation

sgratch
Copy link
Collaborator

@sgratch sgratch commented Sep 5, 2024

On both plan and migration status fields, the number of displayed migrated VMs within the status' progress bar label is sometimes negative, i.e. the value of X within "X of Y VMs migrated" is sometimes negative.

This is reproduced when at least one VM failed to migrate.

The reason is that failed VMs are often marked with an error but they are not marked as completed yet, so the calculation of migrated = completed - error < 0.

Before:

Screenshot from 2024-09-05 18-12-18

Screenshot from 2024-09-05 17-54-25

Screenshot from 2024-09-05 17-54-32

After:

Screenshot from 2024-09-05 18-14-25

Screenshot from 2024-09-05 18-14-41

Screenshot from 2024-09-05 18-15-00

On both plan and migration status fields, the number of displayed
migrated VMs within the status' progress bar label is sometimes negative,
i.e.  the value of X within "X out of Y VMs migrated" is sometimes
negative.

This is reproduced when at least one VM failed to migrated.

The reason is that failed VMs is often marked with an error but their completed
value is still set to false since VM's migration was not finished yet. So the
calculation of migrated = completed - error < 0.

Signed-off-by: Sharon Gratch <[email protected]>
@sgratch sgratch requested a review from yaacov September 5, 2024 15:21
Copy link

sonarcloud bot commented Sep 5, 2024

@sgratch sgratch changed the title Fix Negative number for plan/migration status VMs count 🐞 Fix Negative number for plan/migration status VMs count Sep 5, 2024
@yaacov
Copy link
Member

yaacov commented Sep 8, 2024

I'm ok with this fix as a temporary "hot fix" for the negative values, until we have time to fix #1284 - note that 1284 should address the correctness of all parameters including the success but not just it, the fix should also address the correctness during the time the user request a start of a migration and the time the values are updated in the plan CR ( see: #1283 )

@yaacov yaacov merged commit 91ce362 into kubev2v:main Sep 8, 2024
8 checks passed
@yaacov yaacov added the bug Categorizes issue or PR as related to a bug. label Sep 8, 2024
@yaacov yaacov added this to the 2.6.7 milestone Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants