nri icon indicating copy to clipboard operation
nri copied to clipboard

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

Open Apokleos opened this issue 2 years ago • 3 comments

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
 }

Apokleos avatar Sep 01 '23 12:09 Apokleos

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 ?

klihub avatar Sep 13 '23 07:09 klihub

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

Apokleos avatar Sep 18 '23 02:09 Apokleos