Skip to content

added support for manifest templating with environment variables and values key in skaffold.yaml#892

Closed
Place1 wants to merge 2 commits intoGoogleContainerTools:masterfrom
Place1:support-for-manifest-templating
Closed

added support for manifest templating with environment variables and values key in skaffold.yaml#892
Place1 wants to merge 2 commits intoGoogleContainerTools:masterfrom
Place1:support-for-manifest-templating

Conversation

@Place1
Copy link
Copy Markdown

@Place1 Place1 commented Aug 11, 2018

Sorry for not communicating at all about this feature idea, I've been thinking this could be a great addition to the tool for awhile but I just never got around to creating a feature request. Then today I read #543 and wrote a response for this idea, thinking it might be a good solution.

After checking every open/closed issue I couldn't find anyone else talking about this feature so I decided to jump in and implement it myself (i'm a golang newbie).

I think the comment I wrote on #543 sums up the feature best so i'll copy it here (with a few minor edits):


It'd be nice if skaffold would allow you to defined variables that are rendered into your manifests. These ideally could be global or profile specific and could be sourced from environment variables e.g.

apiVersion: skaffold/v1alpha2
kind: Config
profiles:
- name: prod
  # ...  
  deploy:
    kubectl:
      manifests: ./k8s/*
    values:
      namespace: "prod"
      domain: "myapp.com"
- name: feature-branch
  # ...  
  deploy:
    kubectl:
      manifests: ./k8s/*
    values:
      namespace: "{{.CI_ENVIRONMENT_SLUG}}"
      domain: "{{.CI_ENVIRONMENT_SLUG}}.myapp.com"

then your k8s manifests could have the following:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: myservice-ingress
  namespace: "{{.namespace}}"
spec:
  rules:
  - host: "myservice.{{.domain}}"
    http:
      paths:
      - path: /
        backend:
          serviceName: myservice
          servicePort: 80

I think this also allows skaffold to fill a nice middle ground between basic deployments with static manifests and helm deployments which adds a cluster component and multiple extra configuration files that feel like boilerplate when all you need is some basic template rendering.


@googlebot
Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@Place1
Copy link
Copy Markdown
Author

Place1 commented Aug 11, 2018

@googlebot I signed it!

@googlebot
Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 11, 2018

Codecov Report

Merging #892 into master will increase coverage by 0.06%.
The diff coverage is 45.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #892      +/-   ##
==========================================
+ Coverage   38.27%   38.33%   +0.06%     
==========================================
  Files          56       56              
  Lines        2576     2598      +22     
==========================================
+ Hits          986      996      +10     
- Misses       1476     1482       +6     
- Partials      114      120       +6
Impacted Files Coverage Δ
pkg/skaffold/deploy/kubectl.go 63.06% <45.45%> (-2.52%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f11067...68dca50. Read the comment docs.

@Place1
Copy link
Copy Markdown
Author

Place1 commented Aug 11, 2018

@googlebot I signed it!

@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

@Place1
Copy link
Copy Markdown
Author

Place1 commented Aug 11, 2018

I've added an example in examples/manifest-templating that outlines what this feature looks like.

@balopat
Copy link
Copy Markdown
Contributor

balopat commented Aug 13, 2018

Hi @Place1, thank you for the PR and for thinking about this, it is an important aspect of Skaffold that we are trying to figure out and it's great that you started to contribute in golang! However I don't think this aligns with our current thinking: manifest customization is something that we'd like to delegate to solutions downstream, for example kustomize or helm, skaffold itself is not aiming to implement another templating layer for the manifests for now.

@Place1
Copy link
Copy Markdown
Author

Place1 commented Aug 14, 2018

Hey @balopat cheers for the consideration and I understand your position on this.

But I'll add a few extra thoughts just incase 😛

The motivation to add templating support for the kubectl deployer is to help simplify the tooling requirements for deploying a service.

Here's my company's Gitlab CI use case:

  1. build multiple images and tag them with $CI_COMMIT_SHA
  2. deploy k8s manifests
    • using $CI_ENVIRONMENT_SLUG as namespace for all resources
    • using $CI_ENVIRONMENT_SLUG as a prefix on all ingress hostnames

We could use a tool like kustomize or helm but it's overkill for basic envsubst functionality.
In the case of helm, you're adding a server side component which developers then need to learn how to secure and operate. Alternatively Kustomize introduces a totally different approach with manifest patching which quite verbose and not as flexible as using template variables in my opinion, patching requires you to understand the structure of the resource thing you're patching, and while Kustomize does support variables, they support is not general see docs.

We were initially drawn to Skaffold because we saw the envTemplate feature and saw it solved our image tagging issue. Templating other values in the manifest feels like a natural extension to what skaffold already does - take a manifest and replace values (image names).

So, if adding templating to the kubectl deployer is not on the table, what about adding an envsubst or template deployer that allows Skaffold to fill this gap? I'd be more than happy to provide the leg work to build and document it 😄

@Place1
Copy link
Copy Markdown
Author

Place1 commented Aug 14, 2018

I've spent some time modelling this same use-case with helm and here's what i've ended up with:

apiVersion: skaffold/v1alpha2
kind: Config
build:
  tagPolicy:
    envTemplate:
      template: "{{.CI_REGISTRY_IMAGE}}/{{.IMAGE_NAME}}:{{.DIGEST_HEX}}"
  artifacts:
    - imageName: my-service
      workspace: ./my-service/
    - imageName: another-service
      workspace: ./another-service/
deploy:
  helm:
    releases:
      - name: my-service
        chartPath: ./my-service/k8s
        setValueTemplates:
          image: "{{.CI_REGISTRY_IMAGE}}/{{.IMAGE_NAME}}:{{.DIGEST_HEX}}"
          domain: "{{.CI_ENVIRONMENT_SLUG}}.mydomain.com"
      - name: another-service
        chartPath: ./another-service/k8s
        setValueTemplates:
          image: "{{.CI_REGISTRY_IMAGE}}/{{.IMAGE_NAME2}}:{{.DIGEST_HEX2}}"
          domain: "{{.CI_ENVIRONMENT_SLUG}}.mydomain.com"
      - name: ingress
        chartPath: ./k8s/ingress
        setValueTemplates:
          domain: "mydomain.com"
      - name: monitoring
        chartPath: ./k8s/monitoring
        setValueTemplates:
          domain: "mydomain.com"

This is for using skaffold to automate the build/deploy of:

  • 2 backend services
  • ingress setup (traefix + consul)
  • monitoring setup (prometheus + grafana)

The only reason to use helm here is to be able to render some variables in the k8s manifests, specifically for the ingress resource's host rules, to specify a domain name that is environment dependent.

As a result of using helm i've had to:

  • restructure all my k8s folder layout to follow what helm wants/suggests
  • add 8 new config files: Chart.yaml and values.yaml for 4 different apps
  • duplicate various variable definitions in my skaffold.yaml file because each helm release requires the same domain variable.

With the feature proposal in this PR it'd look like this:

apiVersion: skaffold/v1alpha2
kind: Config
build:
  tagPolicy:
    envTemplate:
      template: "{{.CI_REGISTRY_IMAGE}}/{{.IMAGE_NAME}}:{{.DIGEST_HEX}}"
  artifacts:
    - imageName: registry/user/my-service
      workspace: ./my-service/
    - imageName: registry/user/another-service
      workspace: ./another-service/
deploy:
  kubectl:
    manifests:
      - ./k8s/**
      - ./my-service/k8s/**
      - ./another-service/k8s/**
    values:
      domain: "mydomain.com"
      environment: "{{.CI_ENVIRONMENT_SLUG}}"

With this i wouldn't have to add any new configuration files or install a cluster side component (yay skaffold). Additionally, my manifests look the same as when I use helm because they both use golang templating.

Sorry to be writing so much on this 😬 just trying to explain where i'm coming from.

@dlorenc
Copy link
Copy Markdown
Contributor

dlorenc commented Aug 14, 2018

Have you tried kustomize or other templating systems? I don't doubt that there is room for improvement here, I just don't believe skaffold is the right place for these improvements.

@Place1
Copy link
Copy Markdown
Author

Place1 commented Aug 15, 2018

Alright no worries. I'll close this PR.

@Place1 Place1 closed this Aug 15, 2018
@balopat
Copy link
Copy Markdown
Contributor

balopat commented Aug 15, 2018

Thanks @Place1 for the detailed documentation of your use case - I'll get back to you about your idea of adding an envsubst deployer.

@Place1
Copy link
Copy Markdown
Author

Place1 commented Aug 16, 2018

@balopat thanks, i'd be happy to do the leg work building it. thanks for the consideration.

@Place1
Copy link
Copy Markdown
Author

Place1 commented Aug 24, 2018

@balopat any update on an envsubst deployer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants