Skip to content

Commit

Permalink
fix(core): override Build version with module
Browse files Browse the repository at this point in the history
For `BuildAction`s generated by converting modules, we now override
the `versionString` of the full version of `Build` actions with the
`versionString` of their parent module.

This fixes an issue where e.g. `helm` modules that referred to
`${modules.some-container-module.version}` would fail to deploy, since
the version of the Build action that `container-module` would get
converted to wouldn't match that of the `container` module that it was
converted from—this resulted in a different version being built than the
Helm chart expected, leading to image pull errors.

The same pattern is also standard when using `kubernetes` and
`container` modules in conjunction.

This should ease the transition from 0.12 to 0.13 for many projects.
  • Loading branch information
thsig committed Mar 8, 2023
1 parent 7f7490b commit 565ce12
Show file tree
Hide file tree
Showing 13 changed files with 399 additions and 6 deletions.
20 changes: 20 additions & 0 deletions core/src/actions/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import {
ExecutedActionExtension,
} from "./base"
import { ResolvedConfigGraph } from "../graph/config-graph"
import { ActionVersion } from "../vcs/vcs"
import { Memoize } from "typescript-memoize"

export interface BuildCopyFrom {
build: string
Expand Down Expand Up @@ -142,6 +144,24 @@ export class BuildAction<
> extends BaseAction<C, O> {
kind: "Build"

/**
* Builds from module conversions inherit their version from their parent module. This is done for compatibility
* reasons, so that e.g. the module version hash that appears in `${modules.*.outputs.deployment-image-id}` in
* a runtime step in a module config is consistent with the version hash in the image tag pushed by the `container`
* build. Otherwise, this would fail, since the Build version would differ from the module version.
*
* Semantically, this should be irrelevant to the user, since build cache hits or misses should be triggered for
* similar changes to the underlying build-relevant parts of the module config, or to the included sources.
*/
@Memoize()
getFullVersion(): ActionVersion {
const actionVersion = super.getFullVersion()
if (this._moduleVersion) {
actionVersion.versionString = this.moduleVersion().versionString
}
return actionVersion
}

/**
* Returns the build path for the action. The path is generally `<project root>/.garden/build/<action name>`.
* If `buildAtSource: true` is set on the config, the path is the base path of the action.
Expand Down
22 changes: 22 additions & 0 deletions core/test/data/test-projects/helm/api copy/.helmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Patterns to ignore when building packages.
# This supports shell glob matching, relative path matching, and
# negation (prefixed with !). Only one pattern per line.
.DS_Store
# Common VCS dirs
.git/
.gitignore
.bzr/
.bzrignore
.hg/
.hgignore
.svn/
# Common backup files
*.swp
*.bak
*.tmp
*~
# Various IDEs
.project
.idea/
*.tmproj
.vscode/
9 changes: 9 additions & 0 deletions core/test/data/test-projects/helm/api copy/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: v1
appVersion: "1.0"
description: A Helm chart for Kubernetes
name: api-module
version: 0.1.0
image:
repository: busybox
tag: latest
pullPolicy: IfNotPresent
19 changes: 19 additions & 0 deletions core/test/data/test-projects/helm/api copy/garden.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
kind: Module
description: The API backend for the voting UI
type: helm
name: api-helm-module
releaseName: api-module-release
devMode:
sync:
- target: /app
mode: two-way
serviceResource:
kind: Deployment
containerModule: api-image
values:
image:
tag: ${modules.api-image.version}
ingress:
enabled: true
paths: [/]
hosts: [api-module.local.app.garden]
21 changes: 21 additions & 0 deletions core/test/data/test-projects/helm/api copy/templates/NOTES.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
1. Get the application URL by running these commands:
{{- if .Values.ingress.enabled }}
{{- range $host := .Values.ingress.hosts }}
{{- range $.Values.ingress.paths }}
http{{ if $.Values.ingress.tls }}s{{ end }}://{{ $host }}{{ . }}
{{- end }}
{{- end }}
{{- else if contains "NodePort" .Values.service.type }}
export NODE_PORT=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.ports[0].nodePort}" services {{ include "api-module.fullname" . }})
export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}")
echo http://$NODE_IP:$NODE_PORT
{{- else if contains "LoadBalancer" .Values.service.type }}
NOTE: It may take a few minutes for the LoadBalancer IP to be available.
You can watch the status of by running 'kubectl get svc -w {{ include "api-module.fullname" . }}'
export SERVICE_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "api-module.fullname" . }} -o jsonpath='{.status.loadBalancer.ingress[0].ip}')
echo http://$SERVICE_IP:{{ .Values.service.port }}
{{- else if contains "ClusterIP" .Values.service.type }}
export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app.kubernetes.io/name={{ include "api-module.name" . }},app.kubernetes.io/instance={{ .Release.Name }}" -o jsonpath="{.items[0].metadata.name}")
echo "Visit http://127.0.0.1:8080 to use your application"
kubectl port-forward $POD_NAME 8080:80
{{- end }}
32 changes: 32 additions & 0 deletions core/test/data/test-projects/helm/api copy/templates/_helpers.tpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{{/* vim: set filetype=mustache: */}}
{{/*
Expand the name of the chart.
*/}}
{{- define "api-module.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
{{- end -}}

{{/*
Create a default fully qualified app name.
We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
If release name contains chart name it will be used as a full name.
*/}}
{{- define "api-module.fullname" -}}
{{- if .Values.fullnameOverride -}}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" -}}
{{- else -}}
{{- $name := default .Chart.Name .Values.nameOverride -}}
{{- if contains $name .Release.Name -}}
{{- .Release.Name | trunc 63 | trimSuffix "-" -}}
{{- else -}}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
{{- end -}}
{{- end -}}
{{- end -}}

{{/*
Create chart name and version as used by the chart label.
*/}}
{{- define "api-module.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}}
{{- end -}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ include "api-module.fullname" . }}
labels:
app.kubernetes.io/name: {{ include "api-module.name" . }}
helm.sh/chart: {{ include "api-module.chart" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
spec:
replicas: {{ .Values.replicaCount }}
selector:
matchLabels:
app.kubernetes.io/name: {{ include "api-module.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
template:
metadata:
labels:
app.kubernetes.io/name: {{ include "api-module.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
spec:
shareProcessNamespace: {{ .Values.shareProcessNamespace }}
containers:
- name: {{ .Chart.Name }}
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
imagePullPolicy: {{ .Values.image.pullPolicy }}
args: [python, app.py]
ports:
- name: http
containerPort: 80
protocol: TCP
resources:
{{- toYaml .Values.resources | nindent 12 }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.affinity }}
affinity:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.tolerations }}
tolerations:
{{- toYaml . | nindent 8 }}
{{- end }}
90 changes: 90 additions & 0 deletions core/test/data/test-projects/helm/api copy/templates/ingress.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
{{- if .Values.ingress.enabled -}}
{{- $fullName := include "api-module.fullname" . -}}
{{- $ingressPaths := .Values.ingress.paths -}}

{{- if .Capabilities.APIVersions.Has "networking.k8s.io/v1/Ingress" -}}

# Use the new Ingress manifest structure
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: {{ $fullName }}
labels:
app.kubernetes.io/name: {{ include "api-module.name" . }}
helm.sh/chart: {{ include "api-module.chart" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- with .Values.ingress.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
{{- if .Values.ingress.tls }}
tls:
{{- range .Values.ingress.tls }}
- hosts:
{{- range .hosts }}
- {{ . | quote }}
{{- end }}
secretName: {{ .secretName }}
{{- end }}
{{- end }}
rules:
{{- range .Values.ingress.hosts }}
- host: {{ . | quote }}
http:
paths:
{{- range $ingressPaths }}
- path: {{ . }}
pathType: Prefix
backend:
service:
name: {{ $fullName }}
port:
number: 80
{{- end }}
{{- end }}

{{- else -}}

# Use the old Ingress manifest structure
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
name: {{ $fullName }}
labels:
app.kubernetes.io/name: {{ include "api-module.name" . }}
helm.sh/chart: {{ include "api-module.chart" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- with .Values.ingress.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
{{- if .Values.ingress.tls }}
tls:
{{- range .Values.ingress.tls }}
- hosts:
{{- range .hosts }}
- {{ . | quote }}
{{- end }}
secretName: {{ .secretName }}
{{- end }}
{{- end }}
rules:
{{- range .Values.ingress.hosts }}
- host: {{ . | quote }}
http:
paths:
{{- range $ingressPaths }}
- path: {{ . }}
backend:
serviceName: {{ $fullName }}
servicePort: http
{{- end }}
{{- end }}


{{- end }}
{{- end }}
19 changes: 19 additions & 0 deletions core/test/data/test-projects/helm/api copy/templates/service.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
apiVersion: v1
kind: Service
metadata:
name: {{ include "api-module.fullname" . }}
labels:
app.kubernetes.io/name: {{ include "api-module.name" . }}
helm.sh/chart: {{ include "api-module.chart" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
spec:
type: {{ .Values.service.type }}
ports:
- port: {{ .Values.service.port }}
targetPort: http
protocol: TCP
name: http
selector:
app.kubernetes.io/name: {{ include "api-module.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
51 changes: 51 additions & 0 deletions core/test/data/test-projects/helm/api copy/values.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Default values for api.
# This is a YAML-formatted file.
# Declare variables to be passed into your templates.

replicaCount: 1

image:
repository: api-image
tag: stable
pullPolicy: IfNotPresent

nameOverride: ""
fullnameOverride: ""

service:
type: ClusterIP
port: 80

ingress:
enabled: false
annotations: {}
# kubernetes.io/ingress.class: nginx
# kubernetes.io/tls-acme: "true"
paths: []
hosts:
- chart-example.local
tls: []
# - secretName: chart-example-tls
# hosts:
# - chart-example.local

resources: {}
# We usually recommend not to specify default resources and to leave this as a conscious
# choice for the user. This also increases chances charts run on environments with little
# resources, such as Minikube. If you do want to specify resources, uncomment the following
# lines, adjust them as necessary, and remove the curly braces after 'resources:'.
# limits:
# cpu: 100m
# memory: 128Mi
# requests:
# cpu: 100m
# memory: 128Mi

# This field is whitelisted in `runPodSpecWhitelist`, so it should be included when running tests/tasks
shareProcessNamespace: true

nodeSelector: {}

tolerations: []

affinity: {}
42 changes: 42 additions & 0 deletions core/test/integ/src/plugins/kubernetes/helm/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,48 @@ describe("helmDeploy", () => {
])
})

it("should deploy a chart from a converted Helm module referencing a container module version in its image tag", async () => {
graph = await garden.getConfigGraph({ log: garden.log, emit: false })
const action = await garden.resolveAction<HelmDeployAction>({
action: graph.getDeploy("api-helm-module"),
log: garden.log,
graph,
})

const status = await helmDeploy({
ctx,
log: garden.log,
action,
force: false,
syncMode: false,
localMode: false,
})

const releaseName = getReleaseName(action)
const releaseStatus = await getReleaseStatus({
ctx,
action,
releaseName,
log: garden.log,
syncMode: false,
localMode: false,
})

expect(releaseStatus.state).to.equal("ready")
expect(releaseStatus.detail["values"][".garden"]).to.eql({
moduleName: "api-helm-module",
projectName: garden.projectName,
version: action.versionString(),
})
expect(status.detail?.namespaceStatuses).to.eql([
{
pluginName: "local-kubernetes",
namespaceName: "helm-test-default",
state: "ready",
},
])
})

it("should deploy a chart with sync enabled", async () => {
graph = await garden.getConfigGraph({ log: garden.log, emit: false })
const action = await garden.resolveAction<HelmDeployAction>({
Expand Down
Loading

0 comments on commit 565ce12

Please sign in to comment.