search

hashicorp / faas-nomad

OpenFaaS plugin for Nomad
https://www.openfaas.com
MIT License
254 stars 46 forks source link

allowing consul service tags to be set #87

Closed attachmentgenie closed 4 years ago

attachmentgenie commented 4 years ago

i have a need to setu consul service tag for the openfaas functions, i implemented this similar to https://github.com/hashicorp/faas-nomad/pull/65

attachmentgenie commented 4 years ago

The code looks ok, just wondering if env vars is the right place for this? (good addition btw) There are also the label and annotation collections.

Forcing more and more into envars felt a bit dirty to me too, i looked into labels and annotations as well. Which take KV pairs [1] which tags can be, be mostly aren't.

Therefor i stopped my efforts into creating a pull request for implementing tags as one of those. for future reference i had selected annotations as the route to go down, although those also get used to set metadata at the job level. I choose annotations over labels since labels are implemented at driver level and annotations in the main dsl.

if you believe going down that path is more appropriate then i ll create a PR using those instead [1] https://docs.openfaas.com/reference/yaml/#function-labels

acornies commented 4 years ago

Based on The OpenFaaS docs we probably should store these using annotations. Env vars (in my mind) are usually for setting config for the function/app/microservice code.

I recognize that we've already broken this rule in the provider, so I actually think this is OK to merge and provide users a way to set consul service tags.

I have an ongoing task/repo with a focus on a rewrite of the provider using up-to-date deps. Perhaps we can address it differently there: https://github.com/acornies/faas-nomad-x