Adding an enhancement for integrating day0 kmods with KMM. #957
Conversation
openshift-ci
bot
requested review from
mresvanis,
qbarrand and
yevgeny-shnaidman
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ybettan 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 |
✅ Deploy Preview for openshift-kmm ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Here is a link describing the full process on how to build the first custom ISO and container image to build the cluster using assisted-installer: https://github.com/ybettan/image-composer/blob/main/CUSTOMIZE_RHCOS.md |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #957 +/- ##
==========================================
- Coverage 76.49% 76.47% -0.02%
==========================================
Files 61 62 +1
Lines 5675 5684 +9
==========================================
+ Hits 4341 4347 +6
- Misses 1115 1117 +2
- Partials 219 220 +1 ☔ View full report in Codecov by Sentry. |
ybettan
force-pushed
the
day0
branch
3 times, most recently
from
3b62ced to
80b41c1
Compare
|
General remark: I still think that KMM should not be the one responsible for updating the MachineConfig |
|
I'd suggest the RHCOS version string (ie. I don't know if the RHCOS version string exactly identifies the kernel version (which is what matters to kmods). For example, if realtime kernel support were completed, it would presumably have the same RHCOS version string due to the packages having been included in the same image. Of course, the ostree/container image could include kmods compiled for multiple kernel versions, and there is value in that, but that somewhat deviates from design assumptions inherent in KMM's use of Presumably there is no way to label the autogenerated |
ybettan
force-pushed
the
day0
branch
5 times, most recently
from
d0f742d to
51f3c33
Compare
Signed-off-by: Yoni Bettan <[email protected]>
The `MachineConfigPool`s are used by MCO to tie between a MC and the nodes it should be applied to. KMM should only care about which MC he should act upon. Signed-off-by: Yoni Bettan <[email protected]>
@yevgeny-shnaidman If we put it in KMM then we have 2 options:
Do you still think it shouldn't be in KMM even if it is in a separate image? |
`/var/lib/firmware` isn't in the default lookup path for firmware. Signed-off-by: Yoni Bettan <[email protected]>
Signed-off-by: Yoni Bettan <[email protected]>
| When a `MachineConfig` uses `osImageURL`, it is called | ||
| [image-layering](https://docs.openshift.com/container-platform/4.14/post_installation_configuration/coreos-layering.html). | ||
|
|
||
| When image-layering is used, the OCP layer is being "detached" from the underlying |
There was a problem hiding this comment.
is it still the case? I was under the assumption that it should be addressed in future releases
There was a problem hiding this comment.
Me too. I haven't checked its status recently. Will ask.
There was a problem hiding this comment.
It is still the same behavior indeed.
Looks like the proposed-expriance may duplicate some effort planned in https://issues.redhat.com/browse/MCO-665.
@romfreiman Do you think we should avoid upgrading the nodes after a cluster upgrade (as proposed in this PR) due to the recent info about planned action in the MCO space?
There was a problem hiding this comment.
i rather not have 2 mechanisms to achieve the same. And, it will introduce more reboots - 10m pare baremetal server.
There was a problem hiding this comment.
Then I will drop this "improvement" from the proposal.
FYI @qbarrand
|
@ybettan: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
* Removed the `osMapping` and the `literal` from the API. * Described how the `ModuleDay0` is constructed by KMM. * Added a build example as Yevgeni's request Signed-off-by: Yoni Bettan <[email protected]>
|
@pcolledg-amd Thank you for your review.
Are you referring to the container image ( In KMM, we use those templates in case a user is targeting multiple nodes with different kernel using a regexp, therefore we need to template the kernel image to adjust it to each node. In the new day0 flow, a machineConfig will target nodes with the exact same OS on them so I am not sure I get the point of templating it. |
|
After some discussions, we won't introduce an MCO-KMM integration for now, therefore, closing in favor of #1004 /close |
|
@ybettan: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cc @yevgeny-shnaidman @qbarrand @mresvanis