hashicorp / faas-nomad

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

issue number 61. com.openfaas.scale.max and com.openfaas.scale.min ha… #62

Closed hxalid closed 5 years ago

hxalid commented 5 years ago

https://github.com/hashicorp/faas-nomad/issues/61

acornies commented 5 years ago

Hi @hxalid - this looks like a good change, tried it out on the vagrant env:

curl -i http://192.168.50.2:8080/system/function/figlet
HTTP/1.1 200 OK
Content-Length: 181
Content-Type: application/json
Date: Wed, 28 Nov 2018 14:32:42 GMT

{"name":"figlet","image":"functions/figlet:0.9.6","invocationCount":0,"replicas":1,"envProcess":"","availableReplicas":1,"labels":{"com.openfaas.scale.min":"2"},"annotations":null}

In this example I didn't deploy with annotations, but it's fine because I see you're using the same function(s) and members as in reader.go

For future, it's best if you create a feature branch on your fork so that you can keep the default branch clean for syncing upstream.

@nicholasjackson looks like there's something up with the CI. Do you mind taking a look?

acornies commented 5 years ago

Although, now I see that there might be an issue with propagating the invocation count in this call, but that's technically a separate issue.

nicholasjackson commented 5 years ago

Looks like Go-Releaser has updated the config format, let me pin this to a branch for an older version, once we merge this I will upgrade it.

nicholasjackson commented 5 years ago

@hxalid I have fixed the config and your PR is now building, had to move it to here: https://github.com/hashicorp/faas-nomad/pull/63

However if you are happy I can merge this and release an update later today.

Nic

hxalid commented 5 years ago

@nicholasjackson thank you for fixing the config issue.

Sure, please feel free to merge it.

Regards, Khalid