Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Add custom annotation for service #376

Merged
merged 1 commit into from
Mar 9, 2020

Conversation

jb-abbadie
Copy link
Contributor

This will allow users of External DNS to create a record with the Pod IP of the servers.

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 6, 2020

CLA assistant check
All committers have signed the CLA.

@jb-abbadie jb-abbadie force-pushed the add_service_annotations branch from 615c331 to a24f568 Compare March 6, 2020 10:13
values.yaml Outdated
# Extra annotations to attach to the server service
# This should be a multi-line string mapping directly to the a map of
# the annotations to apply to the server services
serviceAnnotations: null
Copy link
Member

Choose a reason for hiding this comment

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

Can you make it like our other service annotation keys.

service:
  # Optional YAML string for additional annotations.
   # This should be a multi-line string of
  # the annotations to apply to the server service.
  annotations: null

@@ -15,6 +15,9 @@ metadata:
heritage: {{ .Release.Service }}
release: {{ .Release.Name }}
annotations:
{{- if .Values.server.serviceAnnotations }}
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test to test/unit/server-service.bats similar to the mesh gateway test:

#--------------------------------------------------------------------
# annotations

@test "meshGateway/Service: no annotations by default" {
  cd `chart_dir`
  local actual=$(helm template \
      -x templates/mesh-gateway-service.yaml  \
      --set 'meshGateway.enabled=true' \
      --set 'connectInject.enabled=true' \
      --set 'client.grpc=true' \
      --set 'meshGateway.service.enabled=true' \
      . | tee /dev/stderr |
      yq -r '.metadata.annotations' | tee /dev/stderr)
  [ "${actual}" = "null" ]
}

@test "meshGateway/Service: can set annotations" {
  cd `chart_dir`
  local actual=$(helm template \
      -x templates/mesh-gateway-service.yaml  \
      --set 'meshGateway.enabled=true' \
      --set 'connectInject.enabled=true' \
      --set 'client.grpc=true' \
      --set 'meshGateway.service.enabled=true' \
      --set 'meshGateway.service.annotations=key: value' \
      . | tee /dev/stderr |
      yq -r '.metadata.annotations.key' | tee /dev/stderr)
  [ "${actual}" = "value" ]
}

@@ -15,6 +15,9 @@ metadata:
heritage: {{ .Release.Service }}
release: {{ .Release.Name }}
annotations:
{{- if .Values.server.serviceAnnotations }}
{{- tpl .Values.server.serviceAnnotations . | nindent 4 }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{- tpl .Values.server.serviceAnnotations . | nindent 4 }}
{{- tpl .Values.server.serviceAnnotations . | nindent 4 | trim }}

@lkysow
Copy link
Member

lkysow commented Mar 6, 2020

Thanks for the PR! Just a few comments.

@lkysow lkysow added area/chart-only Related to changes that simply require yaml chart changes, e.g. exposing a new field enhancement New feature or request waiting-on-response Waiting on the issue creator for a response before taking further action labels Mar 6, 2020
@jb-abbadie jb-abbadie force-pushed the add_service_annotations branch from a24f568 to 8d2dac5 Compare March 9, 2020 10:45
@jb-abbadie jb-abbadie force-pushed the add_service_annotations branch from 8d2dac5 to f9a536d Compare March 9, 2020 10:45
@jb-abbadie
Copy link
Contributor Author

Fixed + Rebased based on the comments.

Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Thanks!

@lkysow lkysow merged commit 5596244 into hashicorp:master Mar 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/chart-only Related to changes that simply require yaml chart changes, e.g. exposing a new field enhancement New feature or request waiting-on-response Waiting on the issue creator for a response before taking further action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants