Skip to content

Refactor worker to support different type of image to be mounted #677

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

Merged

Conversation

yevgeny-shnaidman
Copy link
Contributor

Currently worker only supports remote image mounting (from a repo). In the future we might want to support also mounting an image from a local tar file. This Pr lays a foundation for that. The basic idea is that all the code should stay the same, and only the image mounting implementation should change, as well as adding additional cobra commands. The sequence to add a new image mounter option is:

  1. Define a new cobra command and mount it under root command
  2. the PreRun command in the new command should initialize the worker with the new image mounter
  3. define a new load/unload cobra commands that should be exact replicas of the existing load/unload cobra command. The copy must be done due to the fact that in cobra a command cannot be a child of multiple commands Changes in the PR:
  4. kmod cobra command now initializes the worker. it will initiaze it with remote image mounter implementation. In case we need to add ad different mounter, we just define a different kmod cobra command and it will initialize the worker with the different image mounter implmentation
  5. Change ImagePuller interface to ImageMounter interface
  6. Move ImageMounter interface definition to a dedicated file
  7. Move the implementation of the RemoteImageMounter into the dedicated file 5 update unit-tests

Currently worker only supports remote image mounting (from a repo).
In the future we might want to support also mounting an image from
a local tar file. This Pr lays a foundation for that. The basic idea
is that all the code should stay the same, and only the image mounting
implementation should change, as well as adding additional cobra
commands. The sequence to add a new image mounter option is:
1. Define a new cobra command and mount it under root command
2. the PreRun command in the new command should initialize the worker
   with the new image mounter
3. define a new load/unload cobra commands that should be exact replicas
   of the existing load/unload cobra command. The copy must be done due
   to the fact that in cobra a command cannot be a child of multiple
   commands
Changes in the PR:
1. kmod cobra command now initializes the worker. it will initiaze it
   with remote image mounter implementation. In case we need to add ad
   different mounter, we just define a different kmod cobra command and
   it will initialize the worker with the different image mounter
   implmentation
2. Change ImagePuller interface to ImageMounter interface
3. Move ImageMounter interface definition to a dedicated file
4. Move the implementation of the RemoteImageMounter into the dedicated
   file
5 update unit-tests
@k8s-ci-robot k8s-ci-robot requested a review from ybettan December 24, 2023 13:28
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 24, 2023
@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 24, 2023
Copy link

netlify bot commented Dec 24, 2023

Deploy Preview for kubernetes-sigs-kmm ready!

Name Link
🔨 Latest commit f91d379
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kmm/deploys/65883212e45e530008d9986d
😎 Deploy Preview https://deploy-preview-677--kubernetes-sigs-kmm.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 24, 2023
@yevgeny-shnaidman
Copy link
Contributor Author

/assign @ybettan

@codecov-commenter
Copy link

Codecov Report

Attention: 36 lines in your changes are missing coverage. Please review.

Comparison is base (e85750b) 79.01% compared to head (f91d379) 78.91%.

Files Patch % Lines
cmd/worker/funcs_kmod.go 0.00% 27 Missing ⚠️
internal/worker/remoteimagemounter.go 50.00% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #677      +/-   ##
==========================================
- Coverage   79.01%   78.91%   -0.11%     
==========================================
  Files          50       50              
  Lines        5095     5093       -2     
==========================================
- Hits         4026     4019       -7     
- Misses        884      889       +5     
  Partials      185      185              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qbarrand
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 28, 2023
@k8s-ci-robot k8s-ci-robot merged commit a6444de into kubernetes-sigs:main Dec 28, 2023
qbarrand pushed a commit to qbarrand/kernel-module-management that referenced this pull request Jan 3, 2024
…ernetes-sigs#677)

Currently worker only supports remote image mounting (from a repo).
In the future we might want to support also mounting an image from
a local tar file. This Pr lays a foundation for that. The basic idea
is that all the code should stay the same, and only the image mounting
implementation should change, as well as adding additional cobra
commands. The sequence to add a new image mounter option is:
1. Define a new cobra command and mount it under root command
2. the PreRun command in the new command should initialize the worker
   with the new image mounter
3. define a new load/unload cobra commands that should be exact replicas
   of the existing load/unload cobra command. The copy must be done due
   to the fact that in cobra a command cannot be a child of multiple
   commands
Changes in the PR:
1. kmod cobra command now initializes the worker. it will initiaze it
   with remote image mounter implementation. In case we need to add ad
   different mounter, we just define a different kmod cobra command and
   it will initialize the worker with the different image mounter
   implmentation
2. Change ImagePuller interface to ImageMounter interface
3. Move ImageMounter interface definition to a dedicated file
4. Move the implementation of the RemoteImageMounter into the dedicated
   file
5 update unit-tests
k8s-ci-robot pushed a commit that referenced this pull request Jan 3, 2024
* Remove deprecated module ready labels on nodes (#668)

This change removes 'kmm.node.kubernetes.io/<module>.ready' labels on
all nodes, as it was deprecated in v1.1 and removed in v2.0.

* Fix handling of unknown kernels (#670)

Fix scheduling data generation for nodes that run an unconfigured
kernel and do not have the kmod loaded yet.

* Build / sign: handle all owner kinds (#673)

Make the build / sign events reconciler handle any owner for jobs, not
only Module.

* Refactor worker to support different type of image to be mounted (#677)

Currently worker only supports remote image mounting (from a repo).
In the future we might want to support also mounting an image from
a local tar file. This Pr lays a foundation for that. The basic idea
is that all the code should stay the same, and only the image mounting
implementation should change, as well as adding additional cobra
commands. The sequence to add a new image mounter option is:
1. Define a new cobra command and mount it under root command
2. the PreRun command in the new command should initialize the worker
   with the new image mounter
3. define a new load/unload cobra commands that should be exact replicas
   of the existing load/unload cobra command. The copy must be done due
   to the fact that in cobra a command cannot be a child of multiple
   commands
Changes in the PR:
1. kmod cobra command now initializes the worker. it will initiaze it
   with remote image mounter implementation. In case we need to add ad
   different mounter, we just define a different kmod cobra command and
   it will initialize the worker with the different image mounter
   implmentation
2. Change ImagePuller interface to ImageMounter interface
3. Move ImageMounter interface definition to a dedicated file
4. Move the implementation of the RemoteImageMounter into the dedicated
   file
5 update unit-tests

* Fixing DevicePlugin upgrade from v1.x to v2.x (#679)

In v1.x we had 2 type of Daemonsets: DevicePlugin and ModuleLoader,
which could be distingished by the DaemonsetRole label
In v2.x, there only 1 type of daemonset, DevicePlugin, so the
DaemonsetRole labels was removed.
During DevicePlugin reconciliation, reconcile get all the device plugin
Ds based only on module name label, which means that in case there is a
v1.1 ModuleLoader DS running it will also be chosen. Once reconciler
tries to reconcile ModuleLoader DS, it will fail
This PR, updates the function that chooses the DevicePlugin DS, by
removing old ModuleLoader Ds based on Role label

---------

Co-authored-by: Yevgeny Shnaidman <[email protected]>
qbarrand pushed a commit to qbarrand/kernel-module-management that referenced this pull request Jan 23, 2024
…ernetes-sigs#677) (kubernetes-sigs#946)

Currently worker only supports remote image mounting (from a repo).
In the future we might want to support also mounting an image from
a local tar file. This Pr lays a foundation for that. The basic idea
is that all the code should stay the same, and only the image mounting
implementation should change, as well as adding additional cobra
commands. The sequence to add a new image mounter option is:
1. Define a new cobra command and mount it under root command
2. the PreRun command in the new command should initialize the worker
   with the new image mounter
3. define a new load/unload cobra commands that should be exact replicas
   of the existing load/unload cobra command. The copy must be done due
   to the fact that in cobra a command cannot be a child of multiple
   commands
Changes in the PR:
1. kmod cobra command now initializes the worker. it will initiaze it
   with remote image mounter implementation. In case we need to add ad
   different mounter, we just define a different kmod cobra command and
   it will initialize the worker with the different image mounter
   implmentation
2. Change ImagePuller interface to ImageMounter interface
3. Move ImageMounter interface definition to a dedicated file
4. Move the implementation of the RemoteImageMounter into the dedicated
   file
5 update unit-tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants