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

Detect already running Persistent VMs #541

Merged
merged 7 commits into from
Feb 20, 2024

Conversation

nesitor
Copy link
Member

@nesitor nesitor commented Feb 14, 2024

Problem: Persistent VMs running were not detected after the orchestrator reboot.

Solution: Don't delete the entire table on the start process. Also added small fixes about guest_api and solved code quality issue.

@nesitor nesitor requested a review from hoh February 14, 2024 12:05
@nesitor nesitor self-assigned this Feb 14, 2024
@github-actions github-actions bot added the BLUE This PR is simple and straightforward. label Feb 14, 2024
Copy link

  • Refactored start_guest_api method in executable.py by terminating process instead of joining it. This is more efficient as we don't need to wait for the process to finish before continuing with other tasks.
  • Simplified stop_guest_api method in executable.py by replacing direct process termination with a check if the process is still running, which makes the code safer and easier to understand.
  • Removed unnecessary drop of all tables from database in metrics.py. This change does not introduce any bugs but simplifies the code for future maintenance.
  • Refactored operate_reboot function in views/operator.py by removing async await, which makes the code cleaner and easier to understand.
  • Removed unnecessary VM forgetting from operate_erase function in views/operator.py. This change does not introduce any bugs but simplifies the code for future maintenance.
  • Simplified stop_persistent_execution method in pool.py by removing async await, which makes the code cleaner and easier to understand.
  • Removed unnecessary VM forgetting from forget_vm function in pool.py. This change does not introduce any bugs but simplifies the code for future maintenance.

RED: The PR includes larger changes that may have moderate risks or require deeper understanding of the project architecture. These changes are likely to introduce minor bugs and should be reviewed by experienced developers.

  • Changed allocate_vm_ipv6_subnet method in IPv6Allocator protocol class from a coroutine to a synchronous function, which may cause issues if not handled correctly. This change is likely to introduce minor bugs and should be reviewed by experienced developers.

BLACK: The PR includes extensive changes that have a high potential for introducing bugs or require deep understanding of the project architecture. These changes are likely to introduce significant bugs and should only be reviewed by experienced developers.

  • Removed async await from create_tables function in metrics.py, which may cause issues if not handled correctly. This change is likely to introduce significant bugs and should be reviewed by experienced developers.

Please note that the categorization of PRs can vary depending on specific rules or guidelines set by the team. The above analysis is based on general principles and might not fully capture all potential risks associated with each category. It's recommended to review these changes thoroughly before merging them into the main codebase.

src/aleph/vm/pool.py Outdated Show resolved Hide resolved
src/aleph/vm/utils.py Show resolved Hide resolved
src/aleph/vm/orchestrator/views/operator.py Show resolved Hide resolved
src/aleph/vm/controllers/firecracker/executable.py Outdated Show resolved Hide resolved
src/aleph/vm/orchestrator/views/operator.py Show resolved Hide resolved
@nesitor nesitor requested a review from hoh February 15, 2024 14:09
@hoh hoh merged commit a470f4e into main Feb 20, 2024
18 checks passed
hoh pushed a commit that referenced this pull request Mar 12, 2024
Problem: Persistent VMs running were not detected after the orchestrator reboot.

Solution: Don't delete the entire table on the start process.

* Fix: Avoid to wait so long time to stop guest_api process. Put a timeout of 10 seconds.
* Fix: Avoid final `cannot unpack non-iterable VmExecution object` errors giving an empty list instead None value.
* Fix: If the execution already exist, only continue, not break the loop.

---------

Co-authored-by: Andres D. Molins <[email protected]>
@Psycojoker Psycojoker deleted the andres-fix_detect_already_running_persistent_vms branch July 24, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLUE This PR is simple and straightforward.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants