feat(urunc-deploy): install artifacts under /opt/urunc #387
Conversation
✅ Deploy Preview for urunc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…runc config Signed-off-by: spline2hg <181270613+spline2hg@users.noreply.github.com>
spline2hg
force-pushed
the
urunc-deploy-config
branch
from
4b61cd6 to
ce21fa6
Compare
|
Hi @cmainas , can u please take a look and let me know if any more changes are required. Thanks! |
cmainas
left a comment
cmainas
left a comment
There was a problem hiding this comment.
Hello @spline2hg ,
thank you for this PR. Overall, it looks good, but there are a few points that we could improve:
- Could you also update this subsection in the documentation which is related to
urunc-deploy? - I think there is no reason for the install script to "generate" the config file, since most of the values are static. We can change that in the future, where we allow specific configuration options during deployment to pass to the configuration. However, for the time being (and as first iteration) we can simply use a static file that the script installs like any other artifact.
Signed-off-by: spline2hg <181270613+spline2hg@users.noreply.github.com>
Signed-off-by: spline2hg <181270613+spline2hg@users.noreply.github.com>
…emu data Signed-off-by: spline2hg <181270613+spline2hg@users.noreply.github.com>
cmainas
left a comment
cmainas
left a comment
There was a problem hiding this comment.
Hello @spline2hg ,
please do not perform changes that are not related to the respective PR/issue.
Also, I do not understand why urunc should be updated to search /opt/urunc; it should only check /usr/share. The monitor artifacts and files are not urunc specific ones. They can be system wide (e.g. installation through a package manager). This adds unnecessary overhead and users are already able to specify any directory through the config.
Signed-off-by: spline2hg <181270613+spline2hg@users.noreply.github.com>
|
Yes @cmainas, sorry about that, I got a bit confused there. I have removed the unrelated changes. |
cmainas
left a comment
cmainas
left a comment
There was a problem hiding this comment.
Hello @spline2hg ,
I have added two small comments that need to get address. Other than that please make sure to:
- Rebase your changes with the main branch of urunc
- Squash your commits in one
- Add yourself to https://github.com/spline2hg/urunc/blob/urunc-deploy-config/.github/contributors.yaml
so we can merge this PR.
| - Finally, `urunc` is added as a runtime class in k8s. | ||
|
|
||
| > Note: `urunc-deploy` will install a static version of QEMU in `/usr/local/bin/` along with the QEMU BIOS files in `/usr/local/share/`. Therefore, files with the same names under these directories will get overwritten. | ||
| > Note: `urunc-deploy` will install a static version of QEMU in `/opt/urunc/bin/` along with the QEMU BIOS files in `/opt/urunc/share/`. If QEMU is already installed system-wide, `urunc-deploy` will skip installation and use the existing QEMU binary. |
There was a problem hiding this comment.
We should remove this note, since we do not overwrite possible previous installations of Qemu.
| [extra_binaries.virtiofsd] | ||
| path = "/opt/urunc/bin/virtiofsd" | ||
| path = "/opt/urunc/libexec/virtiofsd" |
There was a problem hiding this comment.
This change is incorrect, because that part of installation guide assumes a different path for virtiofsd.
|
Also, make sure to fix your commit message. More information at https://urunc.io/developer-guide/contribute/ |
2 participants
Summary
Fixes #379
Changes urunc-deploy to install all artifacts under
/opt/uruncdirectory instead of scattering them across multiple directories. Also auto-generates/etc/urunc/config.tomlwith the correct paths.