Day1 KMM solution should support V2 with scratch images #939
Conversation
✅ Deploy Preview for openshift-kmm ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yevgeny-shnaidman The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
yevgeny-shnaidman
force-pushed
the
yevgeny/day1-scratch
branch
from
7c7b620 to
092f7b9
Compare
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #939 +/- ##
=======================================
Coverage 76.13% 76.14%
=======================================
Files 63 63
Lines 5728 5730 +2
=======================================
+ Hits 4361 4363 +2
Misses 1145 1145
Partials 222 222 ☔ View full report in Codecov by Sentry. |
| @@ -2,14 +2,14 @@ | |||
| echo "before checking podman images" | |||
| if podman image exists {{.Image}}; then | |||
| echo "Image {{.Image}} found in the local registry, removing in-tree kernel module" | |||
| podman run --privileged --entrypoint modprobe {{.Image}} -rd /opt {{.KernelModule}} | |||
| podman run --user=root --privileged -v /etc/kmm-worker-day1/config.yaml:/etc/kmm-worker/config.yaml {{.Image}} kmod unload /etc/kmm-worker/config.yaml | |||
There was a problem hiding this comment.
What is the benefits of using kmod rather than modprobe? Looks like modprobe is just a link to kmod
There was a problem hiding this comment.
this code actually runs worker pod, and kmod is part d the arguments to the worker app in the pod. modporbe will be called by the worker pod
There was a problem hiding this comment.
Ohh OK. I was confused since there is a kmod binary loading kernel modules in Linux.
|
/hold |
yevgeny-shnaidman
force-pushed
the
yevgeny/day1-scratch
branch
2 times, most recently
from
cc7b81b to
217eba0
Compare
| } | ||
| return fmt.Errorf("invalid image %s, input should be either in tag or digest format. Digest error %v, Tag error %v", image, digestErr, tagErr) | ||
|
|
||
| return "/var/lib/image_file_day1.tar", nil |
There was a problem hiding this comment.
This restricts the day1 solution to one image only. Should we make that path unique? See #920.
There was a problem hiding this comment.
I think that a better solution would be to use soft dependencies: saves loading different MCOs, creating new additional services and pulling different images
| if string(expectedRes) != res { | ||
| fmt.Printf("<%s>\n", res) | ||
| fmt.Printf("<%s>\n", expectedRes) | ||
| } |
There was a problem hiding this comment.
This looks like debugging to me 🙂
There was a problem hiding this comment.
It was done intentionally: in case test fails, it would be easier to debug seeing the whole ouput of both MCO
| @@ -1,13 +1,22 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
I believe it is systemd calling those scripts, right? If yes, I think it would be cleaner if the variables were either environment variables or arguments configured in the unit file.
There was a problem hiding this comment.
Not sure i agree: those are one-shot services. We don not expect to configure/change them once they have been run. In addition having all the parameters defined in the scripts allows for easier testing of scripts as standalone
Currently day1 solution relies on the fact that images kernel module images used are DTK-based image, and therefore contain modprobe command. Since KMM V2 we now support basic scratch images. This PR adds support for scratch images by running worker image with supplied config
yevgeny-shnaidman
force-pushed
the
yevgeny/day1-scratch
branch
from
217eba0 to
77e4566
Compare
|
/lgtm |
|
/unhold |
|
/test lint |
openshift-merge-bot
bot
merged commit bdb58f8
into
rh-ecosystem-edge:main
Currently day1 solution relies on the fact that images kernel module images used are DTK-based image, and therefore contain modprobe command. Since KMM V2 we now support basic scratch images. This PR adds support for scratch images by running worker image with supplied config