networkservicemesh / cmd-admission-webhook-k8s

Apache License 2.0
0 stars 16 forks source link

Use NSM-envs from the client application #101

Open glazychev-art opened 2 years ago

glazychev-art commented 2 years ago

Description

We have a many NSM envs that allow us to configure the NSC in the best possible way: https://github.com/networkservicemesh/cmd-nsc/blob/main/internal/config/config.go#L31-L43

    Name             string        `default:"nsc" desc:"Name of Network Service Client"`
    ConnectTo        url.URL       `default:"unix:///var/lib/networkservicemesh/nsm.io.sock" desc:"url to connect to NSM" split_words:"true"`
    DialTimeout      time.Duration `default:"5s" desc:"timeout to dial NSMgr" split_words:"true"`
    RequestTimeout   time.Duration `default:"15s" desc:"timeout to request NSE" split_words:"true"`
    MaxTokenLifetime time.Duration `default:"10m" desc:"maximum lifetime of tokens" split_words:"true"`

    Labels    []string `default:"" desc:"A list of client labels with format key1=val1,key2=val2, will be used a primary list for network services" split_words:"true"`
    Mechanism string   `default:"kernel" desc:"Default Mechanism to use, supported values: kernel, vfio" split_words:"true"`

    NetworkServices       []url.URL               `default:"" desc:"A list of Network Service Requests" split_words:"true"`
    AwarenessGroups       awarenessgroups.Decoder `defailt:"" desc:"Awareness groups for mutually aware NSEs" split_words:"true"`
    LogLevel              string                  `default:"INFO" desc:"Log level" split_words:"true"`
    OpenTelemetryEndpoint string                  `default:"otel-collector.observability.svc.cluster.local:4317" desc:"OpenTelemetry Collector Endpoint"`

And we definitely can do it when we use a bare NSC, without any actual client application.

But this is not the main use case for NSM - users can add networkservicemesh annotations:

...
annotations:
    networkservicemesh.io: "kernel://my-networkservice-1/nsm-1"
...

NetworkServices or Mechanism - we can take it from the annotation. But what about others? What if the user wants to add AwarenessGroups and so on?

Possible solutions:

  1. Use NSM_ prefixed envs that are passed by user and inject them into NSM-containers
  2. Expand the current networkservicemesh.io annotation and add new parameters
glazychev-art commented 2 years ago

@denis-tingaikin @edwarnicke Any thoughts?

edwarnicke commented 2 years ago

@glazychev-art Could you provide concrete examples of things we might do with them?

denis-tingaikin commented 2 years ago

@edwarnicke Imagine the case when external client wants set Mutually Aware Group

edwarnicke commented 2 years ago

@LionelJouin Do you anticipate using the annotations for cases where you need to set a Mutually Aware Group?

LionelJouin commented 2 years ago

@edwarnicke no, I don't think so, we are not using the webhook.

edwarnicke commented 2 years ago

@glazychev-art @denis-tingaikin Do we have someone using Webhooks for more complex cases? Do we have a need ourselves there... or are we trying to get ahead of needs?

denis-tingaikin commented 2 years ago

@edwarnicke This could be useful for invariant testing. Moreover, I think this is actual question. What if external client wants to tune up nsc configuration?

denis-tingaikin commented 2 years ago

@edwarnicke

I think we might be inrested in support next syntax:

networkservicemesh.io: | {{. mechanism }}://{{ .serviceName}}/{{ .interfaceName }}?{{. labels }}
{{.envConfig}

Example:

networkservicemesh.io: | kernel://ns-1/nsm-1
NSM_DIAL_TIMEOUT=5s
NSM_MAX_TOKEN_LIFE_TIME=30s
...

Thoughts?

edwarnicke commented 2 years ago

What if we have multiple Network Services to which a Pod wishes to connect?

denis-tingaikin commented 2 years ago

What if we have multiple Network Services to which a Pod wishes to connect?

Thats an intresting question.

For this case we need change syntax to this

networkservicemesh.io: | 
  {{.envConfig}
  {{. mechanism1 }}://{{ .serviceName1}}/{{ .interfaceName1 }}?{{. labels1}}
  {{. mechanism2 }}://{{ .serviceName2}}/{{ .interfaceName2 }}?{{. labels2}}
  ...
  {{. mechanismN }}://{{ .serviceNameN}}/{{ .interfaceNameN }}?{{. labelsN}}

Or we could simply add a new annotation

networkservicemesh.io/env.config: |
   {{.envConfig}
edwarnicke commented 2 years ago

I am concerned here are that all the use cases we have so far involve us wanting to tweak our test cases. Introducing complex parameter tweaking into our test cases makes it less likely that things just 'work' out of the box with a simple single line annotation.

denis-tingaikin commented 2 years ago

We don't want to use this for testing.

We need this for users who want to enable Mutually Aware Group, or disable heal, or tune something for the specific cluster,.

@edwarnicke Makes sense?

edwarnicke commented 2 years ago

@denis-tingaikin Do we have users currently?

denis-tingaikin commented 2 years ago

At this moment we dont get any requests.

I also saw that, for local datapath debug could be super useful to disable data path to avoid ton of icmp requests in traces.