-
Notifications
You must be signed in to change notification settings - Fork 109
add scripts to install/uninstall kuasar-vmm #221
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
Conversation
Summary of ChangesHello @Tweakzx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive solution for managing the Kuasar VMM environment through two new shell scripts. The "install-kuasar-vmm.sh" script orchestrates the entire setup, from installing system dependencies to deploying Cloud-Hypervisor, Kuasar's VMM sandboxer, a specialized Containerd, crictl, and virtiofsd. Complementing this, the "uninstall-kuasar-vmm.sh" script provides a robust mechanism to cleanly remove these components and revert system configurations, enhancing the maintainability and ease of experimentation with Kuasar. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces installation and uninstallation scripts for kuasar-vmm, which is a great step towards simplifying the user experience. My review focuses on improving the robustness, security, and cleanliness of these scripts. For the installation script, I've suggested using a temporary directory for downloads to ensure proper cleanup, adding checksum verification for security, quoting variables for robustness, and I've pointed out a brittle URL. For the uninstallation script, I've identified a missing file in the cleanup process, unused variables, and a misleading comment. Addressing these points will make the scripts more reliable and maintainable for users.
2f8379c to
ff82db5
Compare
64d0767 to
f794124
Compare
f794124 to
276d5e8
Compare
|
@Tweakzx Do you think we should also update readme or add a quickstart here https://kuasar.io/docs/ |
276d5e8 to
a024ee1
Compare
These three scipts only install/configure/remove kuasar-vmm with cloud-hypervisor, not all runtimes and backends that kuasar supports. Maybe it's still in the early stages and not suitable to be seen as a quickstart guide yet. |
|
Otherwise It's a little bit hard to know how to run these script |
a024ee1 to
f5d3857
Compare
@hzxuzhonghu added a README file to describe how to run these scripts |
| sudo ctr version | ||
| sudo crictl version | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After Post-Installation Steps, there should be a guide to use vmm. Can you link it to the existing guide
Signed-off-by: lizhixuan <[email protected]>
f5d3857 to
3ecd91e
Compare
hzxuzhonghu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
kevin-wangzefeng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
CI failue is irrelevant, manually merging. |
add 3 scripts: