Add meta on ConsulSidecarService

[horaciomonsalvo]

Description

Changes

• Added support for Sidecar Service meta configuration:
  • Added Meta field to ConsulSidecarService struct and updated its receiver functions.
• Updated jobspec/parse_service.go
• Update related tests

Testing Changes

To test these changes you need to build a Nomad binary and deploy a cluster with both Nomad and Consul agents and clients.
When you have your cluster running you can run the following job-spec:

job "countlinux" {
   datacenters = ["dc1"]
   group "api" {
    constraint {
        attribute = "${attr.kernel.name}"
        value = "linux"
     }
    //  Required to enable the connect feature.
     network {
       mode = "bridge"
     }

     service {
       name = "count-api"
       port = "9001"

       connect {
         sidecar_service {
           # New Field
           meta {
            testkey = "testValue"
            testkey1 = "testValue1"
            testkey2 = "testValue2"
           }   
         }
       }
     }

     task "web" {
       driver = "docker"
       config {
         image = "hashicorpnomad/counter-api:v1"
       }
     }
   }
 }
 

Once the job is running and healthy, use Consul's API to get the terminating gateway's configuration:

curl --request GET http://127.0.0.1:8500/v1/catalog/service/count-api-sidecar-service | json_pp

You will get the following JSON output:

[
    {
        "ID": "8ab7d765-a8ad-ca56-4934-cc69b17cae0f",
        "Node": "ip-172-31-5-89",
        "Address": "172.31.5.89",
        "Datacenter": "dc1",
        "TaggedAddresses": {
            "lan": "172.31.5.89",
            "lan_ipv4": "172.31.5.89",
            "wan": "172.31.5.89",
            "wan_ipv4": "172.31.5.89"
        },
        "NodeMeta": {
            "consul-network-segment": ""
        },
        "ServiceKind": "connect-proxy",
        "ServiceID": "_nomad-task-e94b575b-4d22-f8fa-9092-05e06dbeaa51-group-api-count-api-9001-sidecar-proxy",
        "ServiceName": "count-api-sidecar-proxy",
        "ServiceTags": [],
        "ServiceAddress": "172.31.5.89",
        "ServiceTaggedAddresses": {
            "consul-virtual": {
                "Address": "240.0.0.1",
                "Port": 25574
            },
            "lan_ipv4": {
                "Address": "172.31.5.89",
                "Port": 25574
            },
            "wan_ipv4": {
                "Address": "172.31.5.89",
                "Port": 25574
            }
        },
        "ServiceWeights": {
            "Passing": 1,
            "Warning": 1
        },
        "ServiceMeta": {
            "testkey": "testValue",
            "testkey1": "testValue1",
            "testkey2": "testValue2"
        },
        "ServicePort": 25574,
        "ServiceSocketPath": "",
        "ServiceEnableTagOverride": false,
        "ServiceProxy": {
            "DestinationServiceName": "count-api",
            "DestinationServiceID": "_nomad-task-e94b575b-4d22-f8fa-9092-05e06dbeaa51-group-api-count-api-9001",
            "LocalServiceAddress": "127.0.0.1",
            "LocalServicePort": 9001,
            "Mode": "",
            "Config": {
                "bind_address": "0.0.0.0",
                "bind_port": 25574,
                "envoy_stats_tags": [
                    "nomad.alloc_id=e94b575b-4d22-f8fa-9092-05e06dbeaa51",
                    "nomad.group=api",
                    "nomad.job=countlinux",
                    "nomad.namespace=default"
                ]
            },
            "MeshGateway": {},
            "Expose": {}
        },
        "ServiceConnect": {},
        "CreateIndex": 581,
        "ModifyIndex": 581
    }
]

Demo Video

ConsulSidecarService.mp4

[hashicorp-cla]

All committers have signed the CLA.

[tgross]

Hi @horaciomonsalvo !

This is missing the implementation for Diff from nomad/structs/diff.go. If you take a look at the jobspec checklist it's the 2nd-to-the-last item in the main checklist. The easiest way to manually test this is to run a job with the new field, change the field, and then nomad plan to verify the change is detected.

[tgross]

Can we move meta up above port so they're in alphabetical order?

[horaciomonsalvo]

Hi @tgross , thanks for the feedback!

I moved meta to match alphabetical order. 👍

Regarding Diff updates, I think there is no need to apply any updates for this PR, since the only field added is the meta field, which is a map[string]string. Anyway, I have added a test case for the scenario you mentioned.
Also, I tested this implementation as you commented, and it seems to be working as expected.

[tgross]

Regarding Diff updates, I think there is no need to apply any updates for this PR, since the only field added is the meta field, which is a map[string]string. Anyway, I have added a test case for the scenario you mentioned.
Also, I tested this implementation as you commented, and it seems to be working as expected.

Ah, looks like the diff for this field passes thru the flatmap.Flatten, which I didn't realize actually works for maps of primitives (ref flatmap.go#L55). But thanks for testing!

[tgross]

LGTM

[tgross]

@horaciomonsalvo I missed that we need a changelog entry for this. I tried pushing a commit onto the PR but something about the way you've set this up is preventing me from doing so. If you have disabled "maintainers can make edits" on your PRs, please don't for this body of work.

In the meantime, to add a changelog file, run make cl and it'll run you thru questions:

• this is PR 16705
• it's an "improvement" (option 2)
• the body should read connect: Added support for meta field on sidecar service block

[horaciomonsalvo]

@horaciomonsalvo I missed that we need a changelog entry for this. I tried pushing a commit onto the PR but something about the way you've set this up is preventing me from doing so. If you have disabled "maintainers can make edits" on your PRs, please don't for this body of work.

In the meantime, to add a changelog file, run make cl and it'll run you thru questions:

• this is PR 16705
• it's an "improvement" (option 2)
• the body should read connect: Added support for meta field on sidecar service block

Thanks @tgross ! I added the changelog. 👍

Regarding "maintainers can make edits" I don't see this options. I will continue looking at it, but I think it could be related to this issue.

[tgross]

Regarding "maintainers can make edits" I don't see this options. I will continue looking at it, but I think it could be related to this issue.

Yeah that looks like it. No worries.

[tgross]

LGTM. Just going to wait for CI to be green again and then I'll merge. Thanks @horaciomonsalvo!