Skip to content
This repository has been archived by the owner on Dec 16, 2020. It is now read-only.

Add annotation support #28

Merged
merged 1 commit into from
Aug 1, 2018
Merged

Add annotation support #28

merged 1 commit into from
Aug 1, 2018

Conversation

ewilde
Copy link
Contributor

@ewilde ewilde commented Jul 25, 2018

Tasks

  • Deploy handler
  • Read handler
  • Update handler
  • Manual testing
  • PR text

Store stack.yml annotation meta data in deploy, update and read
handlers

Relates to openfaas/faas#682

Signed-off-by: Edward Wilde ewilde@gmail.com

Motivation and Context

How Has This Been Tested?

Using the deploy.sh script from faas and changing faas-swarm to latest-dev in the docker-compose

  1. Deploy function without label or annotation

Test stack.yml

provider:
  name: faas
  gateway: http://127.0.0.1:8080  # can be a remote server
  network: "func_functions"       # this is optional and defaults to func_functions

functions:

  nodejs-echo:
    lang: node
    handler: ./sample/nodejs-echo
    image: alexellis/faas-nodejs-echo:0.1
$ docker service inspect nodejs-echo | jq '.[].Spec.Labels'

{
  "com.openfaas.function": "nodejs-echo",
  "com.openfaas.uid": "526388731",
  "function": "true"
}

http://localhost:8080/system/functions

[
  {
    "name": "nodejs-echo",
    "image": "alexellis/faas-nodejs-echo:0.1",
    "invocationCount": 0,
    "replicas": 1,
    "envProcess": "node index.js",
    "availableReplicas": 0,
    "labels": {
      "com.openfaas.function": "nodejs-echo",
      "com.openfaas.uid": "526388731",
      "function": "true"
    },
    "annotations": null
  }
]
  1. Deploy function with label and no annotation

Test stack.yml

provider:
  name: faas
  gateway: http://127.0.0.1:8080  # can be a remote server
  network: "func_functions"       # this is optional and defaults to func_functions

functions:

  nodejs-echo:
    lang: node
    handler: ./sample/nodejs-echo
    image: alexellis/faas-nodejs-echo:0.1
    labels:
      foo: bar

$ docker service inspect nodejs-echo | jq '.[].Spec.Labels'
{
  "com.openfaas.function": "nodejs-echo",
  "com.openfaas.uid": "889344122",
  "foo": "bar",
  "function": "true"
}

http://localhost:8080/system/functions

[
  {
    "name": "nodejs-echo",
    "image": "alexellis/faas-nodejs-echo:0.1",
    "invocationCount": 0,
    "replicas": 1,
    "envProcess": "node index.js",
    "availableReplicas": 0,
    "labels": {
      "com.openfaas.function": "nodejs-echo",
      "com.openfaas.uid": "889344122",
      "foo": "bar",
      "function": "true"
    },
    "annotations": null
  }
]
  1. Deploy function with label and with annotation

Test stack.yml

provider:
  name: faas
  gateway: http://127.0.0.1:8080  # can be a remote server
  network: "func_functions"       # this is optional and defaults to func_functions

functions:

  nodejs-echo:
    lang: node
    handler: ./sample/nodejs-echo
    image: alexellis/faas-nodejs-echo:0.1
    labels:
      foo: bar
    annotations:
      current-time: Fri 27 Jul 22:54:25 BST 2018
      id: fe4694c7-db78-4aa6-b157-ffbf0f475126
$ docker service inspect nodejs-echo | jq '.[].Spec.Labels'
{
  "annotation: current-time": "Fri 27 Jul 22:54:25 BST 2018",
  "annotation: id": "fe4694c7-db78-4aa6-b157-ffbf0f475126",
  "com.openfaas.function": "nodejs-echo",
  "com.openfaas.uid": "550627680",
  "foo": "bar",
  "function": "true"
}

http://localhost:8080/system/functions

[
  {
    "name": "nodejs-echo",
    "image": "alexellis/faas-nodejs-echo:0.1",
    "invocationCount": 0,
    "replicas": 1,
    "envProcess": "node index.js",
    "availableReplicas": 0,
    "labels": {
      "com.openfaas.function": "nodejs-echo",
      "com.openfaas.uid": "550627680",
      "foo": "bar",
      "function": "true"
    },
    "annotations": {
      "annotation: current-time": "Fri 27 Jul 22:54:25 BST 2018",
      "annotation: id": "fe4694c7-db78-4aa6-b157-ffbf0f475126"
    }
  }
]

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.


[[override]]
name = "github.com/moby/moby"
revision = "4a804016ab8b9fd55a45cff9687a1de12dee5eb7"
Copy link
Contributor Author

@ewilde ewilde Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removes the following warning when running dep:

Warning: the following project(s) have [[constraint]] stanzas in Gopkg.toml:

  ✗  github.com/moby/moby

However, these projects are not direct dependencies of the current project:
they are not imported in any .go files, nor are they in the 'required' list in
Gopkg.toml. Dep only applies [[constraint]] rules to direct dependencies, so
these rules will have no effect.

Either import/require packages from these projects so that they become direct
dependencies, or convert each [[constraint]] to an [[override]] to enforce rules
on these projects, if they happen to be transitive dependencies,

Warning: Gopkg.lock is out of sync with Gopkg.toml or the project's imports.

@ewilde ewilde force-pushed the annotations branch 2 times, most recently from 268b6da to 09b993a Compare July 27, 2018 21:42
func buildLabelsAndAnnotations(dockerLabels map[string]string) (labels map[string]string, annotations map[string]string) {
for k, v := range dockerLabels {
if strings.HasPrefix(k, annotationLabelPrefix) {
if annotations == nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only initialize map if we have at least one annotation, so nil is returned otherwise

@ewilde ewilde changed the title [WIP] Add annotation support Add annotation support Jul 27, 2018
@alexellis
Copy link
Member

@ewilde I think that annotations should not show up if it's null, that can be configured through omitEmpty in the struct representing the JSON.

The prefix of "annotation: " appears to work, but I had expected one single "annotation" label into which we would serialise all the annotations.

Something like:

_, labels["com.openfaas.annotations"] = json.Marshal(&userAnnotations)

Since this is just an internal representation we should be able to change it after the merge?

@alexellis
Copy link
Member

After merging #26 via @LucasRoesler there is a small conflict on Gopkg.lock. (should be quick to resolve)

@ewilde
Copy link
Contributor Author

ewilde commented Jul 31, 2018

@alexellis agree better label prefix would be com.openfaas.annotations. but probably better to leave the mapping 1-1 as is allows for filtering etc.. https://docs.docker.com/engine/reference/commandline/service_ls/#filtering which may be useful?

Since Docker does not deserialize the value, you cannot treat a JSON or XML document as a nested structure when querying or filtering by label value unless you build this functionality into third-party tooling
https://docs.docker.com/config/labels-custom-metadata/#label-keys-and-values

Store stack.yml annotation meta data in deploy, update and read
handlers

Relates to openfaas/faas#682

Signed-off-by: Edward Wilde <ewilde@gmail.com>
@@ -26,6 +26,8 @@ import (
"github.com/openfaas/faas/gateway/requests"
)

const annotationLabelPrefix = "com.openfaas.annotations."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using recommended key format as suggested

@alexellis alexellis merged commit cd4ff36 into openfaas:master Aug 1, 2018
@ewilde ewilde deleted the annotations branch August 1, 2018 15:20
@ewilde ewilde mentioned this pull request Aug 8, 2018
11 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants