Skip to content
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

device-injector for handling Mount annotations in parseMount needs do support parts of the Mount info #49

Open
Apokleos opened this issue Sep 1, 2023 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Apokleos
Copy link

Apokleos commented Sep 1, 2023

In my scenario, containers use PV/PVC to set up Volume/VolumeMount. In this case, the Mount.Destinationand Mount.Type in annotations. mounts.nri.io/container.container001is explicitly specified in the YAML configuration file without Mount.Source. I only want to specify the destination/type in the mount annotation, and then the NRI can automatically complete the other fields of Mount from the Container.Mount[i] by comparing the destination.

I understand that the current processing logic does not automatically complete the omitted Source/Options. This is not user-friendly for users who use CSI/PV/PVC.

The yaml as below looks like:

apiVersion: v1
kind: Pod
metadata:
  name: kata-nri-mutlvolx03
  annotations:
    mounts.nri.io/container.nri-vol002: |+
      - destination: /vol_x0000000002
        type: directvol
        options:
        - rbind
        - rw
spec:
  containers:
  - name: nri-vol002
    image: nginx
    volumeMounts:
      - name: vol-x00002
        mountPath: /vol_x0000000002
      - name: test-volx03
        mountPath: /data_vol03
  volumes:
  - name: vol-x00002
    emptyDir:
      sizeLimit: 500Mi
  - name: test-volx03
    emptyDir:
      sizeLimit: 500Mi

And just with the destination /vol_x0000000002 in the annotation

  annotations:
    mounts.nri.io/container.nri-vol002: |+
      - destination: /vol_x0000000002
        type: directvol
        options:
        - rbind
        - rw

I have a solution to make it work well:

(1) pareMount in device-injector needs have an argument container *api.Container not container Name.
(2) Unmarshal(annotation, &mounts)
(3) look up the containerMounts, with the Destination specified in annotation, if the annotion.Source is None, Just fill mounts with container.Mounts[i],Source;

Here is a rough code implementation:

diff --git a/plugins/device-injector/device-injector.go b/plugins/device-injector/device-injector.go
index 3c19cd2..7f2ad7f 100644
--- a/plugins/device-injector/device-injector.go
+++ b/plugins/device-injector/device-injector.go
@@ -105,7 +105,7 @@ func (p *plugin) CreateContainer(_ context.Context, pod *api.PodSandbox, contain
        }
 
        // inject mounts to container
-       mounts, err = parseMounts(container.Name, pod.Annotations)
+       mounts, err = parseMounts(container, pod.Annotations)
        if err != nil {
                return nil, nil, err
        }
@@ -162,7 +162,7 @@ func parseDevices(ctr string, annotations map[string]string) ([]device, error) {
        return devices, nil
 }
 
-func parseMounts(ctr string, annotations map[string]string) ([]mount, error) {
+func parseMounts(container *api.Container, annotations map[string]string) ([]mount, error) {
        var (
                key        string
                annotation []byte
@@ -171,7 +171,7 @@ func parseMounts(ctr string, annotations map[string]string) ([]mount, error) {
 
        // look up effective device annotation and unmarshal devices
        for _, key = range []string{
-               mountKey + "/container." + ctr,
+               mountKey + "/container." + container.Name,
                mountKey + "/pod",
                mountKey,
        } {
@@ -189,6 +189,12 @@ func parseMounts(ctr string, annotations map[string]string) ([]mount, error) {
                return nil, fmt.Errorf("invalid mount annotation %q: %w", key, err)
        }
 
+       for _, m := range container.Mounts {
+               if m.Destination == mounts[0].Destination {
+                       mounts[0].Source = m.Source
+               }
+       }
+
        return mounts, nil
 }

@klihub
Copy link
Member

klihub commented Sep 13, 2023

Okay, so basically instead of injecting a new mount into a container you would like to adjusts the mount type (?) and options of an existing mount which would be matched/identified by its destination path ?

@Apokleos
Copy link
Author

Thx @klihub
Yeah, in my case, I just want to modify the type of the matched Mount with the destination specified.

@mikebrow mikebrow added help wanted Extra attention is needed enhancement New feature or request labels May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants