search

networkservicemesh / cmd-cluster-info-k8s

Apache License 2.0
0 stars 7 forks source link

Inject CLUSTER_NAME env variable for each NSM appended client #2

Open denis-tingaikin opened 2 years ago

denis-tingaikin commented 2 years ago

Description

Currently clientinfo chain element can work with CLUSTER_NAME env https://github.com/networkservicemesh/sdk/blob/main/pkg/tools/clientinfo/clientinfo.go#L30

But we dont use it yet, because k8s at this moment doesn't have an option to get cluster name without any special complex steps.

Solution

  1. Wait when https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/2149-clusterid#property-idk8sio will be able for k8s users.
  2. Add check in cmd-admission-webhook-k8s on do we have a CRD related to k8s about api
  3. Add CLUSTER_NAME env variable into NSC with value from cluster property crd.
denis-tingaikin commented 2 years ago

Controller is in progress https://github.com/kubernetes-sigs/about-api

@edwarnicke This is blocked while about api in progress.

Should we just wait for the about-api release?

edwarnicke commented 2 years ago

@denis-tingaikin https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/2149-clusterid#property-idk8sio is a CRD. It is either already set when we run our admissions controller, in which case we simply use it's value, or it isn't in which case we should setup the CRD and set it to a value ourselves.

Its not at all clear to me what the https://github.com/kubernetes-sigs/about-api controller would do, or when it would expect to be in GA.

denis-tingaikin commented 2 years ago

@denis-tingaikin https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/2149-clusterid#property-idk8sio is a CRD. It is either already set when we run our admissions controller, in which case we simply use it's value, or it isn't in which case we should setup the CRD and set it to a value ourselves.

Yeah, that what we are planning to do when https://github.com/kubernetes-sigs/about-api will be ready.

Currently, as I can see CRD is not ready and is not using in k8s. Correct me if I wrong.

denis-tingaikin commented 2 years ago

See at cluster propery defenition in https://github.com/kubernetes-sigs/about-api/blob/master/clusterproperty/config/crd/bases/about.k8s.io_clusterproperties.yaml

edwarnicke commented 2 years ago

@denis-tingaikin Yes... but what is the controller doing other than adding the CRD? Is there any reason for us not to add the CRD if its not already present?