Git Product home page Git Product logo

network-resources-injector's People

Contributors

adrianchiris avatar ahalimx86 avatar alinasecret avatar billy99 avatar cgoncalves avatar cyclinder avatar dependabot[bot] avatar jcaamano avatar lgtm-migrator avatar manuelbuil avatar martinkennelly avatar michalguzieniuk avatar nhennigan avatar pperiyasamy avatar przemeklal avatar rthakur-est avatar schseba avatar vivekthrivikraman-est avatar vpickard avatar zeeke avatar zshi-redhat avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

network-resources-injector's Issues

User Defined Injections - documentation is misleading

The ConfigMap given as an example at https://github.com/k8snetworkplumbingwg/network-resources-injector#user-defined-injections is wrong, and in result it is not possible to take example as it is and verify if User Defined Injections feature of NRI is working correctly.

ConfigMap does not support '/' character. There is an issue described at kubernetes/kubernetes#87119. In result the key of data section feature.pod.kubernetes.io/sriov-network: is invalid and not accepted by kubernetes.

It is described within documentation https://pkg.go.dev/k8s.io/api/core/v1#ConfigMap that

Each key must consist of alphanumeric characters, '-', '_' or '.'.

create new release

Hi, Can we have a new release on the NRI ? My project team would like to have a new release with this fix.

node selector injection feature doesn't work when ResourceName is not defined in NAD

Hey,
During e2e testing, we noticed that node selector injection doesn't work when resourceName is not defined in the NAD.
Works fine when there is an additional network defined.
I believe a use case was when no additional network was defined. i.e. macvlan
Documentation example for readme: #50
Original PR: #21
@pperiyasamy

Node selector injection feature works:

apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  name: foo-network
  annotations:
    k8s.v1.cni.cncf.io/nodeSelector: kubernetes.io/hostname=kind-worker2
    k8s.v1.cni.cncf.io/resourceName: example.com/foo
spec:
...

Node selector injection feature doesnt work:

apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  name: foo-network
  annotations:
    k8s.v1.cni.cncf.io/nodeSelector: kubernetes.io/hostname=kind-worker2
spec:
...

Sample Macvlan NAD with no resourceName defined:

apiVersion: "k8s.cni.cncf.io/v1"
kind: NetworkAttachmentDefinition
metadata:
  name: macvlan-conf
spec: 
  config: '{
      "cniVersion": "0.3.1",
      "plugins": [
        {
          "type": "macvlan",
          "capabilities": { "ips": true },
          "master": "eth0",
          "mode": "bridge",
          "ipam": {
            "type": "static",
            "routes": [
              {
                "dst": "0.0.0.0/0",
                "gw": "10.1.1.1"
              }
            ] 
          }
        }, {
          "capabilities": { "mac": true },
          "type": "tuning"
        }
      ]
    }'

Add Helm Chart

to facilitate easy deployment and upgrades we should have a helm chart to deploy NRI in cases where it is used without sriov-network-operator.

in addition sriov-network-operator could then deploy network-resource-injector via helm if deemed needed.
(i.e have a dependent chart in sriov-network-operator helm chart)

npwg helm charts are currently hosted in:
https://github.com/k8snetworkplumbingwg/helm-charts

Remove .idea & .gopath

What happened?

Both directories .idea and .gopath should not be in the code base. .idea is specific to a jetbrains product and .gopath is no longer needed due to go mod support

What did you expect to happen?

NA

What are the minimal steps needed to reproduce the bug?

NA

Anything else we need to know?

Will be corrected in PR.

Add stable tag for network resource injector container

What would you like to be added?

Similarly to sriov-network-device-plugin PR#367 it is desirable to have a stable tag for network-resource-injector container artifacts.

What is the use case for this feature / enhancement?

Allow consumers to consume latest major version updates without updating their yamls. May aid automated testing infra.

User Defined Injections - does not take into account json path operation

While defining custom data in ConfigMap it is possible to define JSON path operation (add, remove, replace, copy, move). Those operation are not taken into account by NRI.

For instance for ConfigMap

apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "remove", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "foo-network"}}'

Original POD specification is not modified

apiVersion: v1
kind: Pod
metadata:
  name: testpod
  labels:
    customInjection: "true"
  annotations:
    k8s.v1.cni.cncf.io/networks: foo-network
spec:
  containers:
  - name: app
    image: alpine
    command: [ "/bin/sh", "-c", "sleep INF" ]

Expected to remove foo-network from POD specification.

Second use case, for ConfigMap, operation add

apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "add", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "sriov-net-attach-def"}}'

and above POD definition, I would expect for operation add to have after modification two networks

 k8s.v1.cni.cncf.io/networks: foo-network, sriov-net-attach-def

instead for given key, values are replaced. Current state:

 k8s.v1.cni.cncf.io/networks: sriov-net-attach-def

make test fails

make test on NRI fails with following error.

# make test
export t="/tmp/go-cover.$.tmp" && go test -coverprofile= ./... && go tool cover -html=
?       github.com/intel/network-resources-injector/cmd/installer       [no test files]
?       github.com/intel/network-resources-injector/cmd/webhook [no test files]
?       github.com/intel/network-resources-injector/pkg/installer       [no test files]
?       github.com/intel/network-resources-injector/pkg/types   [no test files]
ok      github.com/intel/network-resources-injector/pkg/webhook 0.016s  coverage: 23.6% of statements
too many options
For usage information, run "go tool cover -help"
Makefile:22: recipe for target 'test' failed
make: *** [test] Error 1

Can Network-Resources-Injector(NRI) to be deployed as a deployment

This is more or less a question toward to NRI:

Currently, according to example from https://github.com/k8snetworkplumbingwg/network-resources-injector/blob/master/deployments/server.yaml, the NRI was deployed as a Pod in K8S cluster.
But in case the node which holding NRI pod got broken or run out of resource, then NIR pod will be deleted/evicted and won't be re-created.

So we would like to know whether NRI can be deployed as a deployment?
If so, is there any requirement for NRI release version?

@zshi-redhat

resource not injected in a large K8s cluster

When we bringup a deployment which has 100 pod replicas, then some pods are not created because NRI doesn't inject required SRIOV VF resources into the pod.
From troubleshooting NRI (of course logging has to be improved by writing pod info as well in the logs), it looks NRI takes around 15 secs (because of GC ?) to respond to admission request for the failed pod whereas it just look around 5 secs for the working pods. is admission response ignored due to timeout ? it happens though webhook timeout set to 30 sec (default value?).
After changing failurePolicy from Ignore to Fail, then all pods are injected with resources. Does Fail policy make any difference ? As per the documentation, it says it would cause pod admission to fail. isn't it ?
If Fail policy fixes the issue, then we would need to make failurePolicy as configurable parameter, currently it's hardcoded to use Ignore policy.

Support customized injection

Sometimes, user wants to inject customized content in pod manifest on the fly. For example, adding an additional network by injecting net-attach-defs in pod annotation.

This can be achieved by defining a configMap which contains key:value in its data entry, key is a user defined k8s label, value is the content to be injected in pod manifest. NRI inspects the pod label and inject the corresponding data value in pod manifest if the pre-defined label is found. For example:

apiVersion: v1
data:
  network-resource-injector-pod-annotation: '{"op": "add", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "sriov-net-attach-def"} }'
kind: ConfigMap
metadata:
  name: network-resource-injector-customized-injection-config
  namespace: network-resource-injector-namespace

In above example, network-resource-injector-pod-annotation is the k8s label defined by user, followed by the value (json string) defining customized content to be injected in pod spec.

NRI would listen to the configMap change and record the configmap data entry in its internal data structure.
During pod creation, NRI receives the pod manifest and inspects the pod labels, if any of pod label is found to match with the key of recorded data entry, NRI would inject the corresponding data entry value to pod manifest.
If configMap is not provided or data is empty in configMap, NRI would skip the customized injection.

Allow control of per-pod injection for a given resource

In a large cluster, user may want to apply a given NRI injection for certain pods, but not the others.
For example, NRI would inject resource requests/limits to first container in the pod, but this doesn't work for pod with multiple containers (with each requesting a userspace device). So user would like to disable resource injection for muiltiple userspace containers and keep using it for the rest of pods.

A possible solution for this issue:

  1. Define a k8s label for each NRI supported injection

Such as:
network-resources-injector-downward-api-volume: true
network-resources-injector-k8s-extended-resource: false

These labels will be recognized by NRI and has default value being true.

  1. User add the labels in pod spec indicating whether or not a certain resource injection should be enabled/disabled. Default is true (enabled) in the cases that label is not added or label value is not set.
  2. Based on the label key and/or value, NRI would enable or disable resource injection for that particular pod.

network-resources-injector doesn't work in multinode cluster

I've created 2 node k8s cluster (v1.16.2) and installed sriov network device plugin, sriov cni plugin and network-resource-injector with required cpu and topology manager settings.
Now when dpdk pod is created with k8s.v1.cni.cncf.io/networks annotation for injecting the VF device, it works on master (also acts as worker) and not on the another worker node (pod is actually running, but without VF device attached to it).

I just ran the following to deploy resource injector on the master.

make image
kubectl apply -f deployments/auth.yaml \
              -f deployments/server.yaml

Conceptual problem: Pods still need to know device pool names

In our understanding the main purpose of the Network Resources Injector is to hide the complexity of PCI network device resource management from application pods. Pods just refer to the NADs in their pod annotations and leave the resource handling to the network resources injector, network device plugin, device manager and scheduler.

However, this abstraction breaks because the only way for a pod to know which PCI devices it has been assigned as network attachments is to check for environment variables like the following
PCIDEVICE_INTEL_COM_MELLANOX_SNIC0=0000:18:00.4,0000:18:00.3

These are inserted into the pod by the SRIOV network device plugin and refer to resource pool name (mellanox_snic0 in the above example). So the application anyway needs to know the resource names and the only simplification that remains is that the resources do not have to be listed in the pod's resource section.

We would very much like make the network annotation abstraction complete. This would require providing the pod with a "mapping" between NAD names and PCI addresses.

The network device plugin doesn't know about network annotations, so it can't easily create corresponding environment variables. The Multus CNI seems to be the right entity to provide that mapping as it already has all data at hand, but it seems difficult, if possible at all, for the Multus CNI to add additional environment variables into the pod when it is invoked.

Any opinions and suggestions?

'User defined injections' feature does not react on ConfigMap remove

Problem:
After ConfigMap removal labels defined in the POD specification are read by NRI and still processed. In result, annotations are updated even if I as a user does not expect such behaviour.

Test steps:
As a precondition NRI is setup, and all networks are available.

  1. Add network attachment definitions
  2. Add ConfigMap
  3. Wait 1 minute - time needed to update POD and NRI
  4. Create POD - verify that annotations were changed according to labels (k8s.v1.cni.cncf.io/networks: sriov-net-attach-def)
  5. Delete POD
  6. Delete ConfigMap
  7. Wait 1 minute - time needed to update POD and NRI
  8. Create POD - verify that annotation were not changed, even if POD specification defines label.
    Expected annotation k8s.v1.cni.cncf.io/networks: foo-network
    Actual annotation k8s.v1.cni.cncf.io/networks: sriov-net-attach-def
apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "add", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "sriov-net-attach-def"}}'
apiVersion: v1
kind: Pod
metadata:
  name: testpod
  labels:
    customInjection: "true"
  annotations:
    k8s.v1.cni.cncf.io/networks: foo-network
spec:
  containers:
  - name: app
    image: alpine
    command: [ "/bin/sh", "-c", "sleep INF" ]
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  name: foo-network
  annotations:
    k8s.v1.cni.cncf.io/resourceName: example.com/foo
spec:
  config: |
    {
      "cniVersion": "0.3.0",
      "name": "foo-network",
      "type": "loopback"
    }
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  name: sriov-net-attach-def
  # namespace: kube-system
  annotations:
    k8s.v1.cni.cncf.io/resourceName: example.com/boo
spec:
  config: |
    {
      "cniVersion": "0.3.0",
      "name": "sriov-net-attach-def",
      "type": "loopback"
    }

URLs need to be updated

What went wrong?
Imports and documentation within the repository are referencing the old Intel repository URL.
Will be corrected in following PR.

Help understanding webhook client certificates

Hello,

I'm trying to set up a NRI based on the quick-start, and I get a TLS error when I try to create a Pod:

Error from server (InternalError): error when creating "/tmp/testpod.yaml": Internal error occurred: failed calling webhook "network-resources-injector-mutating-config.k8s.cni.cncf.io": failed to call webhook: Post "https://network-resources-injector-service.kube-system.svc:443/mutate?timeout=10s": remote error: tls: bad certificate

In the NRI logs, a corresponding message saying the client didn't provide a cert:

2023/06/23 01:33:07 http: TLS handshake error from 10.244.0.1:38392: tls: client didn't provide a certificate

If I run with --insecure, it works.

I see that the NRI server loads the service account client cert on startup, and without --insecure, expects it to be sent by the apiserver with each request. But from my understanding of the relevant Kubernetes docs, getting the apiserver to send a client cert involves starting the API server with --admission-control-config-file, populating a kubeConfig file somewhere, none of which is mentioned in the NRI readme. So I feel like I'm missing something. Is it possible to use NRI with client cert auth, without touching config files on the node?

Sidebar question - does NRI have side-effects? If not --insecure seems OK in that I'm not too worried who the client is if all the controller does is return a patch response.

Many thanks for this handy tool!

Honor the existing resources

There are use cases where same resource pool name is consumed by different co existing network plugins (example: using both multus and networkservicemesh). In case of multus specific network attachment, network-resources-injector can be used to add resources request/limit depending on the k8s.v1.cni.cncf.io/resourceName annotation in the NetworkAttachmentDefinition object.
But in case of networkservicemesh, resources requests/limits are directly specified in the pod spec and later networkservicemesh injects the appropriate interface into the container.

But currently network-resources-injector ignores the existing resources requests/limits when device requested from the same pool for networkservicemesh network and adds only the quantities that are required for multus network.

network resources injector cannot handle pods from deployment properly

network resources injector inspects the net-attach-def and injects the sriov resource request to pod resource request/limits. It works perfectly with a pod, but has issues dealing with a deployment pod in a non-default namespace.

The workflow of network resources injector is:

  1. watch the creation/update of pod
  2. receive AdmissionReview request
  3. deserialize pod object from AdmissionReview request
  4. set pod namespace to 'default', If pod object doesn't contain valid namespace (e.g. empty namespace for a deployment pod)
  5. call API to retrieve net-attach-def with above namespace + net-attach-def name (from pod object annotation)
  6. If resource is found in net-attach-def, inject the resource name in pod object. else do nothing.

The problem for a deployment pod happens in step 5), the deserialized deployment pod object contains an empty namespace field, so it is set to default namespace, which in turn cause the actual namespace of deployment not be respected. So when network-resources-injector tries to get the net-attach-def with default namespace, it fails.

It looks like we have two possible ways to fix this:

  1. find a way to get pod namespace from deployment
  2. watch the creation/update of deployment object and mutate the deployment spec (this implies that we should ignore the pod creation/update event that's from the same deployment)

resource injector doesn't honor the capacity for resources

I've set capacity for a resource called intel_sriov_netdevice as 4 (like below), but when pods are created with same network more than 5, it doesn't throw any error.
It just goes on and boot the pod with remaining available devices which matches selection criteria.

curl -s --header "Content-Type: application/json-patch+json" \
     --request PATCH \
     --data '[{"op": "add", "path": "/status/capacity/intel.com~1intel_sriov_netdevice", "value": "4"}]' \
     http://localhost:8001/api/v1/nodes/dl380-006-eccd-sut/status >/dev/null

Set http(s) proxy and/or no_proxy appropriately in build-image.sh

The build-image.sh sets docker build arguments with http(s) proxy and no proxy though these are not set in a test environment which makes make image step fail as part of network-resources-injector installation.

Please set those parameters only if these are defined in system env variables.

Resources limit and requests are added only to first container inside POD

NRI adds limits and requests to resources section only to first container within POD. Second container resources are not affected.

Test case:

  1. Run NRI with flag -honor-resources
  2. Create foo and boo CRD networks
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  annotations:
    k8s.v1.cni.cncf.io/resourceName: example.com/boo
  name: boo-network
  namespace: default
spec:
  config: '{"cniVersion": "0.3.0", "name": "boo-network", "type":"loopback"}'
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  annotations:
    k8s.v1.cni.cncf.io/resourceName: example.com/foo
  name: foo-network
  namespace: default
spec:
  config: '{"cniVersion": "0.3.0", "name": "foo-network", "type":"loopback"}'
  1. Create POD according to below specification
apiVersion: v1
kind: Pod
metadata:
  annotations:
    k8s.v1.cni.cncf.io/networks: foo-network,boo-network
  name: nri-e2e-test
  namespace: default
spec:
  containers:
  - command:
    - /bin/sh
    - -c
    - sleep INF
    image: docker.io/alpine
    imagePullPolicy: Always
    name: test
  - command:
    - /bin/sh
    - -c
    - sleep INF
    image: docker.io/alpine
    imagePullPolicy: Always
    name: second-name
    resources:
      limits:
        example.com/foo: "9"
      requests:
        example.com/foo: "9"
  1. Verify that resources in both containers are updated. Expected result
    First container
    resources:
      limits:
        example.com/boo: "1"
        example.com/foo: "1"
      requests:
        example.com/boo: "1"
        example.com/foo: "1"

Second container

    resources:
      limits:
        example.com/boo: "1"
        example.com/foo: "10"
      requests:
        example.com/boo: "1"
        example.com/foo: "10"

Current result:

apiVersion: v1
kind: Pod
metadata:
  annotations:
    k8s.v1.cni.cncf.io/networks: foo-network,boo-network
  name: nri-e2e-test
  namespace: default
spec:
  containers:
  - command:
    - /bin/sh
    - -c
    - sleep INF
    image: docker.io/alpine
    imagePullPolicy: Always
    name: test
    resources:
      limits:
        example.com/boo: "1"
        example.com/foo: "1"
      requests:
        example.com/boo: "1"
        example.com/foo: "1"
  - command:
    - /bin/sh
    - -c
    - sleep INF
    image: docker.io/alpine
    imagePullPolicy: Always
    name: second-name
    resources:
      limits:
        example.com/foo: "9"
      requests:
        example.com/foo: "9"

NRI logs

# kubectl logs -n kube-system network-resources-injector 
I0512 08:43:09.189322       1 main.go:69] starting mutating admission controller for network resources injection
I0512 08:43:09.190627       1 tlsutils.go:120] added client CA to cert pool from path '/var/run/secrets/kubernetes.io/serviceaccount/ca.crt'
I0512 08:43:09.190651       1 tlsutils.go:122] added '1' client CA(s) to cert pool

[root@silpixa00397909 network-resources-injector]# kubectl apply -f pod.yaml 
pod/nri-e2e-test created

[root@silpixa00397909 network-resources-injector]# kubectl logs -n kube-system network-resources-injector 
I0512 09:02:57.435382       1 webhook.go:725] Received mutation request
I0512 09:02:57.441010       1 webhook.go:698] search v1.multus-cni.io/default-network in original pod annotations
I0512 09:02:57.441039       1 webhook.go:705] search v1.multus-cni.io/default-network in user-defined injections
I0512 09:02:57.441047       1 webhook.go:719] v1.multus-cni.io/default-network is not found in either pod annotations or user-defined injections
I0512 09:02:57.441059       1 webhook.go:698] search k8s.v1.cni.cncf.io/networks in original pod annotations
I0512 09:02:57.441069       1 webhook.go:701] k8s.v1.cni.cncf.io/networks is defined in original pod annotations
I0512 09:02:57.441096       1 webhook.go:257] 'foo-network,boo-network' is not in JSON format: invalid character 'o' in literal false (expecting 'a')... trying to parse as comma separated network selections list
I0512 09:02:57.473200       1 webhook.go:355] network attachment definition 'default/foo-network' found
I0512 09:02:57.473242       1 webhook.go:362] resource 'example.com/foo' needs to be requested for network 'default/foo-network'
I0512 09:02:57.481258       1 webhook.go:355] network attachment definition 'default/boo-network' found
I0512 09:02:57.481289       1 webhook.go:362] resource 'example.com/boo' needs to be requested for network 'default/boo-network'
I0512 09:02:57.481302       1 webhook.go:811] honor-resources=true
I0512 09:02:57.481345       1 webhook.go:821] injectHugepageDownApi=false
I0512 09:02:57.481377       1 webhook.go:879] patch after all mutations: [{add /spec/containers/0/resources/requests map[]} {add /spec/containers/0/resources/limits map[]} {add /spec/containers/0/resources/requests/example.com~1boo {{1 0} {<nil>}  DecimalSI}} {add /spec/containers/0/resources/limits/example.com~1boo {{1 0} {<nil>}  DecimalSI}} {add /spec/containers/0/resources/requests/example.com~1foo {{1 0} {<nil>}  DecimalSI}} {add /spec/containers/0/resources/limits/example.com~1foo {{1 0} {<nil>}  DecimalSI}} {add /spec/containers/0/volumeMounts/- {podnetinfo true /etc/podnetinfo  <nil> }} {add /spec/containers/1/volumeMounts/- {podnetinfo true /etc/podnetinfo  <nil> }} {add /spec/volumes/- {podnetinfo {nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil &DownwardAPIVolumeSource{Items:[]DownwardAPIVolumeFile{DownwardAPIVolumeFile{Path:annotations,FieldRef:&ObjectFieldSelector{APIVersion:,FieldPath:metadata.annotations,},ResourceFieldRef:nil,Mode:nil,},},DefaultMode:nil,} nil nil nil nil nil nil nil nil nil nil nil nil}}}]
I0512 09:02:57.481776       1 webhook.go:397] sending response to the Kubernetes API server

Add GitHub actions to improve code reviews

Hey all,
Following last weeks community meeting, I have investigated how we can add static analysis, building, testing for each PR or push to this repository.
Heavy inspiration from Multus GAs (Github Actions). Thanks @adrianchiris for letting me know.
Hoping to add e2e tests to this repository as well like what Multus has done with kind.

WDYT?

'User defined injections' feature removes defined network annotations

Problem:
NRI removes k8s.v1.cni.cncf.io/networks: foo-network label from annotations even if ConfigMap does not defines any network. In result, POD does not have additional network information.

Test steps:
As a precondition NRI is setup, and all networks are available.

  1. Add network attachment definition
  2. Add ConfigMap
  3. Wait 1 minute - time needed to update POD with NRI
  4. Create POD
    Expected:
    k8s.v1.cni.cncf.io/networks: foo-network annotation is available together with second entry in annotations list top-secret: password
    Current result:
    Network annotation is removed. Only new label from ConfigMap is visible.

Part of POD spec after modification

Annotations:  k8s.v1.cni.cncf.io/network-status:
                [{
                    "name": "",
                    "interface": "eth0",
                    "ips": [
                        "10.244.1.2"
                    ],
                    "mac": "82:74:b7:10:75:94",
                    "default": true,
                    "dns": {}
                }]
              k8s.v1.cni.cncf.io/networks-status:
                [{
                    "name": "",
                    "interface": "eth0",
                    "ips": [
                        "10.244.1.2"
                    ],
                    "mac": "82:74:b7:10:75:94",
                    "default": true,
                    "dns": {}
                }]
              top-secret: password

Expected POD spec

Annotations:  k8s.v1.cni.cncf.io/network-status:
                [{
                    "name": "",
                    "interface": "eth0",
                    "ips": [
                        "10.244.2.3"
                    ],
                    "mac": "aa:9e:8a:a1:42:b6",
                    "default": true,
                    "dns": {}
                },{
                    "name": "default/foo-network",
                    "interface": "lo",
                    "ips": [
                        "127.0.0.1",
                        "::1"
                    ],
                    "mac": "00:00:00:00:00:00",
                    "dns": {}
                }]
              k8s.v1.cni.cncf.io/networks: foo-network
              k8s.v1.cni.cncf.io/networks-status:
                [{
                    "name": "",
                    "interface": "eth0",
                    "ips": [
                        "10.244.2.3"
                    ],
                    "mac": "aa:9e:8a:a1:42:b6",
                    "default": true,
                    "dns": {}
                },{
                    "name": "default/foo-network",
                    "interface": "lo",
                    "ips": [
                        "127.0.0.1",
                        "::1"
                    ],
                    "mac": "00:00:00:00:00:00",
                    "dns": {}
                }]
              top-secret: password

POD, NAD, ConfigMap specs used in tests

apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "add", "path": "/metadata/annotations", "value": {"top-secret": "password"}}'
apiVersion: v1
kind: Pod
metadata:
  name: testpod
  labels:
    customInjection: "true"
  annotations:
    k8s.v1.cni.cncf.io/networks: foo-network
spec:
  containers:
  - name: app
    image: alpine
    command: [ "/bin/sh", "-c", "sleep INF" ]
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  name: foo-network
  annotations:
    k8s.v1.cni.cncf.io/resourceName: example.com/foo
spec:
  config: |
    {
      "cniVersion": "0.3.0",
      "name": "foo-network",
      "type": "loopback"
    }

Create a Deployment yaml for network-resource-injector

currently our Deployment example cosists of:

  • permissions (auth.yaml): service account, roles, clusterRoles, roleBindings, clusterRoleBindings
  • pod(server.yaml):
  • service (service.yaml): k8s service exposing NRI
  • webhook config (webhook.yaml): MutatingWebhookConfiguration

we should have a deployment instead of a pod so that there may be multiple backends to the service as well as have NRI pods lifecycle managed by a controller to handle placement and restarts.

while at it, its worth doing some cleanup in auth.yaml to remove certificate signing request as they are no longer used

Build error due to incompatible client-go version

Run make against latest master branch (1ea450d)
$ make

scripts/build.sh
# github.com/intel/network-resources-injector/vendor/k8s.io/client-go/rest
.gopath/src/github.com/intel/network-resources-injector/vendor/k8s.io/client-go/rest/request.go:598:31: not enough arguments in call to watch.NewStreamWatcher
	have (*versioned.Decoder)
	want (watch.Decoder, watch.Reporter)
make: *** [Makefile:16: default] Error 2

resource injector overwrites user specified resource limit/request

When a pod spec already has resource limit and request configured for cpu or memory, network-resources-injector will overwrites cpu/memory resources in the pod spec and only add device plugin resourceName in pod resource field.

For example, when creating a pod with below spec file using network-resources-injector:

apiVersion: v1
kind: Pod
metadata:
  name: testpod4
  annotations:
    k8s.v1.cni.cncf.io/networks: sriov-net1
spec:
  containers:
  - name: appcntr4
    image: centos/tools 
    imagePullPolicy: IfNotPresent
    command: [ "/bin/bash", "-c", "--" ]
    args: [ "while true; do sleep 300000; done;" ]
    resources:
      requests:
        cpu: 2
        memory: 100Mi
      limits:
        cpu: 2
        memory: 100Mi

The deployed pod spec will be:

apiVersion: v1
kind: Pod
metadata:
  annotations:
    k8s.v1.cni.cncf.io/networks: sriov-net1
    k8s.v1.cni.cncf.io/networks-status: |-
      [{
          "name": "",
          "ips": [
              "10.96.1.139"
          ],
          "default": true,
          "dns": {}
      },{
          "name": "sriov-network",
          "interface": "net1",
          "ips": [
              "10.56.217.63"
          ],
          "mac": "fa:32:38:0e:b5:94",
          "dns": {}
      }]
  creationTimestamp: "2019-06-18T14:10:41Z"
  name: testpod4
  namespace: default
  resourceVersion: "21891"
  selfLink: /api/v1/namespaces/default/pods/testpod4
  uid: daea75c4-91d2-11e9-8c67-3cfdfeb5188c
spec:
  containers:
  - args:
    - while true; do sleep 300000; done;
    command:
    - /bin/bash
    - -c
    - --
    image: centos/tools
    imagePullPolicy: IfNotPresent
    name: appcntr4
    resources:
      limits:
        intel.com/sriov: "1"
      requests:
        intel.com/sriov: "1"

User Defined Injections - injects only one property

For below ConfigMap, where there is more than one key/value pair NRI injects only one pair into POD specification. I observed that this injection is random. It means that only one of the pair: "top-secret" or "k8s.v1.cni.cncf.io/network" is added to the POD.

apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "add", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "sriov-net-attach-def"}}'
  "secondInjection": '{"op": "add", "path": "/metadata/annotations", "value": {"top-secret": "password"}}'

POD specification

apiVersion: v1
kind: Pod
metadata:
  name: testpod
  labels:
    customInjection: "true"
    secondInjection: "true"
  annotations:
    k8s.v1.cni.cncf.io/networks: foo-network
spec:
  containers:
  - name: app
    image: alpine
    command: [ "/bin/sh", "-c", "sleep INF" ]

Logs:

I0413 10:45:43.523737       1 main.go:69] starting mutating admission controller for network resources injection
I0413 10:45:43.524389       1 tlsutils.go:120] added client CA to cert pool from path '/var/run/secrets/kubernetes.io/serviceaccount/ca.crt'
I0413 10:45:43.524405       1 tlsutils.go:122] added '1' client CA(s) to cert pool
I0413 10:46:13.550537       1 webhook.go:943] Initializing user-defined injections with key: customInjection, value: {"op": "add", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "sriov-net-attach-def"}}
I0413 10:46:13.550584       1 webhook.go:943] Initializing user-defined injections with key: secondInjection, value: {"op": "add", "path": "/metadata/annotations", "value": {"top-secret": "password"}}
I0413 10:46:58.393901       1 webhook.go:704] Received mutation request
I0413 10:46:58.438718       1 webhook.go:677] search v1.multus-cni.io/default-network in original pod annotations
I0413 10:46:58.438742       1 webhook.go:684] search v1.multus-cni.io/default-network in user-defined injections
I0413 10:46:58.438751       1 webhook.go:698] v1.multus-cni.io/default-network is not found in either pod annotations or user-defined injections
I0413 10:46:58.438761       1 webhook.go:677] search k8s.v1.cni.cncf.io/networks in original pod annotations
I0413 10:46:58.438769       1 webhook.go:680] k8s.v1.cni.cncf.io/networks is defined in original pod annotations
I0413 10:46:58.438795       1 webhook.go:256] 'foo-network' is not in JSON format: invalid character 'o' in literal false (expecting 'a')... trying to parse as comma separated network selections list
I0413 10:46:58.450875       1 webhook.go:354] network attachment definition 'default/foo-network' found
I0413 10:46:58.450915       1 webhook.go:361] resource 'example.com/foo' needs to be requested for network 'default/foo-network'
I0413 10:46:58.450928       1 webhook.go:790] honor-resources=false
I0413 10:46:58.450955       1 webhook.go:800] injectHugepageDownApi=true
I0413 10:46:58.450981       1 webhook.go:858] patch after all mutations: [{add /spec/containers/0/resources/requests map[]} {add /spec/containers/0/resources/limits map[]} {add /spec/containers/0/resources/requests/example.com~1foo {{1 0} {<nil>}  DecimalSI}} {add /spec/containers/0/resources/limits/example.com~1foo {{1 0} {<nil>}  DecimalSI}} {add /spec/containers/0/volumeMounts/- {podnetinfo false /etc/podnetinfo  <nil> }} {add /spec/volumes/- {podnetinfo {nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil &DownwardAPIVolumeSource{Items:[]DownwardAPIVolumeFile{DownwardAPIVolumeFile{Path:labels,FieldRef:&ObjectFieldSelector{APIVersion:,FieldPath:metadata.labels,},ResourceFieldRef:nil,Mode:nil,},DownwardAPIVolumeFile{Path:annotations,FieldRef:&ObjectFieldSelector{APIVersion:,FieldPath:metadata.annotations,},ResourceFieldRef:nil,Mode:nil,},},DefaultMode:nil,} nil nil nil nil nil nil nil nil nil nil nil nil}}} {add /metadata/annotations map[k8s.v1.cni.cncf.io/networks:sriov-net-attach-def]} {add /metadata/annotations map[k8s.v1.cni.cncf.io/networks:foo-network top-secret:password]}]
I0413 10:46:58.451457       1 webhook.go:396] sending response to the Kubernetes API server
I0413 10:47:13.565255       1 webhook.go:952] Removing stale entry: customInjection from user-defined injections
I0413 10:47:13.565289       1 webhook.go:952] Removing stale entry: secondInjection from user-defined injections

Downward API Injection: Hugepage injection doesn't work with mult container

On top of Issue #44 (Downward API Injection: Multiple containers in pod don't work), when manually adding hugepage fields to the Downward API for multiple containers, the same value is reported in each container, even if the requests are different. This is most likely a Kubernetes issue (will update with issue link when issue is found), but tracking here for documentation of issue.

NRI pod restart not handled

NRI doesn't currently handle nri pod restarts, if for some reason nri pod gets killed and there are no other nri pods in the cluster, then nri pod(and other pods) would not be able to start as the service serving the nri webhook(which is the nri pod itself) is down(as webhook configuration is still present).

'User Defined Injections' feature is missing access rights to configmap

What happened?
NRI is not able to read configmaps, and in result is not able to inject user defined resources into POD specification.

In logs I see such warning
W0407 09:50:20.910817 1 main.go:189] Failed to get configmap for user-defined injections: configmaps "nri-user-defined-injections" is forbidden: User "system:serviceaccount:kube-system:network-resources-injector-sa" cannot get resource "configmaps" in API group "" in the namespace "kube-system"

What did you expect to happen?
I expect that NRI is able to get configuration from configmaps

Note
I did small research on this topic and I think that the problem are permissions defined at

when I add there configmaps to the resources list, injection feature starts to work as expected.

SR-IOV network resources injector cannot parse multiple ip addresses in ipam config

When configuring ips via pod annotation, SR-IOV injector cannot parse net-attach-def with an array of ip list, which result in SR-IOV resource request/limit not be injected in pod spec. The pod can still be created successfully, but no SR-IOV interface is attached.

  1. create sriov-net1 net-attach-def

apiVersion: "k8s.cni.cncf.io/v1"
kind: NetworkAttachmentDefinition
metadata:
name: sriov-net1
annotations:
k8s.v1.cni.cncf.io/resourceName: intel.com/intel_sriov_netdevice
spec:
config: '{
"type": "sriov",
"cniVersion": "0.3.1",
"name": "sriov-network",
"ipam": {
"type": "static"
}
}'

  1. use pod spec below:

apiVersion: v1
kind: Pod
metadata:
name: pod-sriov-vf
annotations:
k8s.v1.cni.cncf.io/networks: |
[
{
"name": "sriov-net1",
"ips": ["100.100.100.100/24", "2001::2/64"]
}
]
spec:
containers:

  • name: appcntr3
    image: centos/tools
    imagePullPolicy: IfNotPresent
    command: [ "/bin/bash", "-c", "--" ]
    args: [ "while true; do sleep 300000; done;" ]
    resources:
    requests:
    memory: 100Mi
    cpu: '2'
    intel.com/intel_sriov_netdevice: '1'
    limits:
    memory: 100Mi
    cpu: '2'
    intel.com/intel_sriov_netdevice: '1'

Network resources injector logs

I1108 01:56:43.472065 1 webhook.go:332] Received mutation request
I1108 01:56:43.475727 1 webhook.go:371] network attachment definition 'default/sriov-net1' found
I1108 01:56:43.475765 1 webhook.go:377] resource 'intel.com/intel_sriov_netdevice' needs to be requested for network 'default/sriov-net1'
I1108 01:56:43.475850 1 webhook.go:422] patch after all mutations%!(EXTRA []webhook.jsonPatchOperation=[{add /spec/containers/0/resources/requests/intel.com1intel_sriov_netdevice {{1 0} {} DecimalSI}} {add /spec/containers/0/resources/limits/intel.com1intel_sriov_netdevice {{1 0} {} DecimalSI}} {add /spec/containers/0/volumeMounts/- {podnetinfo false /etc/podnetinfo }} {add /spec/volumes/- {podnetinfo {nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil &DownwardAPIVolumeSource{Items:[{labels ObjectFieldSelector{APIVersion:,FieldPath:metadata.labels,} nil } {annotations &ObjectFieldSelector{APIVersion:,FieldPath:metadata.annotations,} nil }],DefaultMode:nil,} nil nil nil nil nil nil nil nil nil nil nil nil}}}])
I1108 01:56:43.476046 1 webhook.go:257] sending response to the Kubernetes API server
I1108 01:59:45.137375 1 webhook.go:332] Received mutation request
I1108 01:59:45.138197 1 webhook.go:157] '[
{
"name": "sriov-net1",
"ips": "100.100.100.100/24"
}
]
' is not in JSON format: json: cannot unmarshal string into Go struct field NetworkSelectionElement.ips of type []string... trying to parse as comma separated network selections list
I1108 01:59:45.138347 1 webhook.go:217] at least one of the network selection units is invalid: error found at '[
{
"name": "sriov-net1"'
E1108 01:59:45.138388 1 webhook.go:163] error parsing network selection element: at least one of the network selection units is invalid: error found at '[
{
"name": "sriov-net1"'
I1108 01:59:45.138421 1 webhook.go:391] pod doesn't need any custom network resources
I1108 01:59:45.138432 1 webhook.go:257] sending response to the Kubernetes API server

Support for user configurable resource name keys

The network-resources-injector currently supports only one resource (per network) defined with resource name key k8s.v1.cni.cncf.io/resourceName which makes difficult to provide two distinct resources in the same network attachment definition object. Please refer to ovs-cni issue for more details.
Hence I would to enhance network-resources-injector with user configurable network-resource-name-keys (comma separated strings) configuration parameter.
By default, it supports existing k8s.v1.cni.cncf.io/resourceName when no values are provided.

How can we better handle feature control switches in NRI?

Currently in pkg/webhook we have defined variables to control whether certain functionality is enabled/disabled:

var (
	clientset              kubernetes.Interface
	injectHugepageDownApi  bool
	resourceNameKeys       []string
	honorExistingResources bool
)

As NRI feature-set grows, this approach is unworkable. How can we prevent the growth of variable feature control switches in NRI?

I think it would be better to define a struct or interface and pass it to functions.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.