openconfig / gnmic

gNMIc is a gNMI CLI client and collector
https://gnmic.openconfig.net
Apache License 2.0
190 stars 57 forks source link

Consul Loader ignoring tags #272

Closed lucabrasi83 closed 12 months ago

lucabrasi83 commented 1 year ago

Hi!

I'm using gnmic v0.33 and have setup the Consul loader as below with multiple entries using the same name but different tags in order to apply different subscriptions:


loader:
  type: consul
  address: consul:8500
  datacenter: dc1
  enable-metrics: true
  services:
    - name: networks-gnmic-targets
      tags:
        - "vendor=arista"
        - "platform=eos"
      config:
        subscriptions:
          - netgenie_telemetry_proto
        outputs:
          - prom-output-proto
        insecure: true
    - name: networks-gnmic-targets
      tags:
        - "vendor=cisco"
        - "platform=iosxe"
      config:
        subscriptions:
          - netgenie_telemetry_json
        outputs:
          - prom-output-cisco-xe
        insecure: true

The tags are correctly assigned to the target service ID's in Consul:


{

  "ID": "test-cat9k",
  "Service": "networks-gnmic-targets",
  "Tags": [
    "vendor=cisco",
    "platform=iosxe"

  ],

  "Meta": {},
  "Port": 50052,
  "Address": "10.0.46.11",
  "Weights": {
    "Passing": 1,
    "Warning": 1
  },

  "EnableTagOverride": false,
  "ContentHash": "fe321e05900eb3a8"

}

However, this seems to be ignored by gnmic as it just applies the config from the first service name:

2023/11/01 23:20:58.784037 [gnmic] adding target {"name":"test-cat9k","address":"[10.0.46.11:50052](http://10.0.46.11:50052/)","username":"****","password":"****","timeout":10000000000,"insecure":true,"skip-verify":false,"subscriptions":["netgenie_telemetry_proto"],"outputs":["prom-output-proto"],"buffer-size":100,"retry-timer":10000000000,"log-tls-secret":false,"gzip":true,"token":""}

Is it the correct way I'm configuring it and it's supposed to work ?

karimra commented 1 year ago

gNMIc applies the config from the first service with a matching name. This was not designed to support services with the same name. It should work if you create 2 services called:networks-gnmic-targets-cisco and networks-gnmic-targets-arista.

If you really need to create just one service I will need to write a PR to match against both the service name and the tags.

lucabrasi83 commented 1 year ago

Thanks @karimra using different services indeed work. So I guess I was getting confused by https://github.com/openconfig/gnmic/issues/106#issuecomment-1512021956

karimra commented 1 year ago

You are right, that has to be fixed. Will find some time to work on it.

lucabrasi83 commented 1 year ago

Hi @karimra I believe the update in #281 is causing a panic as we always return tc as nil when we found a matching service in Consul.

Shouldn't we have something like this instead in function serviceEntryToTargetConfig:

              // match service name
        if se.Service.Service != sd.Name {
            continue
        }
...
    return nil, errors.New("unable to find a match in Consul service(s)")

I can submit a PR if that looks ok to you.

Thanks

karimra commented 1 year ago

Hi @lucabrasi83, I agree it would be a good idea to return a error there. I don't that it would cause a panic because gNMIc will add the target name as an address, but you will end up with a target you didn't want. Thanks for catching this, feel free to create a PR.