Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chart: Use stable API versions where available #17211

Merged
merged 10 commits into from
Sep 16, 2021
10 changes: 10 additions & 0 deletions chart/UPDATING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ Default Airflow version is updated to ``2.1.2``

The default Airflow version that is installed with the Chart is now ``2.1.3``, previously it was ``2.1.2``.

Removed ``ingress.flower.precedingPaths`` and ``ingress.flower.succeedingPaths`` parameters
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""

``ingress.flower.precedingPaths`` and ``ingress.flower.succeedingPaths`` parameters have been removed as they had previously had no effect on rendered YAML output.

Change of default ``path`` on Ingress
"""""""""""""""""""""""""""""""""""

With the move to support the stable Kubernetes Ingress API the default path has been changed from being unset to ``/``. For most Ingress controllers this should not change the behavior of the resulting Ingress resource.

Airflow Helm Chart 1.1.0
------------------------

Expand Down
10 changes: 10 additions & 0 deletions chart/templates/_helpers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -592,3 +592,13 @@ Create the name of the cleanup service account to use
{{- $_ := set $config "auths" $auth -}}
{{ $config | toJson | print }}
{{- end }}

{{/* Allow Kubernetes Version to be overridden. Credit to https://github.com/prometheus-community/helm-charts for Regex. */}}
{{- define "kubeVersion" -}}
{{- $kubeVersion := default .Capabilities.KubeVersion.Version .Values.kubeVersionOverride -}}
{{/* Special use case for Amazon EKS, Google GKE */}}
{{- if and (regexMatch "\\d+\\.\\d+\\.\\d+-(?:eks|gke).+" $kubeVersion) (not .Values.kubeVersionOverride) -}}
{{- $kubeVersion = regexFind "\\d+\\.\\d+\\.\\d+" $kubeVersion -}}
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
4 changes: 4 additions & 0 deletions chart/templates/cleanup/cleanup-cronjob.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
{{- $nodeSelector := or .Values.cleanup.nodeSelector .Values.nodeSelector }}
{{- $affinity := or .Values.cleanup.affinity .Values.affinity }}
{{- $tolerations := or .Values.cleanup.tolerations .Values.tolerations }}
{{- if semverCompare ">= 1.21.x" (include "kubeVersion" .) }}
apiVersion: batch/v1
{{- else }}
apiVersion: batch/v1beta1
{{- end }}
kind: CronJob
metadata:
name: {{ .Release.Name }}-cleanup
Expand Down
23 changes: 21 additions & 2 deletions chart/templates/flower/flower-ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@
#################################
{{- if .Values.flower.enabled }}
{{- if and .Values.ingress.enabled (or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor")) }}
{{- $apiIsStable := semverCompare ">= 1.19.x" (include "kubeVersion" .) -}}
{{- if $apiIsStable }}
apiVersion: networking.k8s.io/v1
{{- else }}
apiVersion: networking.k8s.io/v1beta1
{{- end }}
kind: Ingress
metadata:
name: {{ .Release.Name }}-flower-ingress
Expand All @@ -30,8 +35,9 @@ metadata:
release: {{ .Release.Name }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: {{ .Release.Service }}
{{- if .Values.ingress.flower.annotations }}
annotations: {{ toYaml .Values.ingress.flower.annotations | nindent 4 }}
{{- with .Values.ingress.flower.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
{{- if .Values.ingress.flower.tls.enabled }}
Expand All @@ -44,13 +50,26 @@ spec:
- http:
paths:
- backend:
{{- if $apiIsStable }}
service:
name: {{ .Release.Name }}-flower
port:
name: flower-ui
{{- else }}
serviceName: {{ .Release.Name }}-flower
servicePort: flower-ui
{{- end }}
{{- if .Values.ingress.flower.path }}
path: {{ .Values.ingress.flower.path }}
{{- if $apiIsStable }}
pathType: {{ .Values.ingress.flower.pathType }}
{{- end }}
{{- end }}
{{- if .Values.ingress.flower.host }}
host: {{ .Values.ingress.flower.host }}
{{- end }}
{{- if and .Values.ingress.flower.ingressClassName $apiIsStable }}
ingressClassName: {{ .Values.ingress.flower.ingressClassName }}
{{- end }}
{{- end }}
{{- end }}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
#################################
{{- if and .Values.pgbouncer.enabled .Values.pgbouncer.podDisruptionBudget.enabled }}
kind: PodDisruptionBudget
{{- if semverCompare ">= 1.21.x" (include "kubeVersion" .) }}
apiVersion: policy/v1
{{- else }}
apiVersion: policy/v1beta1
{{- end }}
metadata:
name: {{ .Release.Name }}-pgbouncer-pdb
labels:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
#################################
{{- if .Values.scheduler.podDisruptionBudget.enabled }}
kind: PodDisruptionBudget
{{- if semverCompare ">= 1.21.x" (include "kubeVersion" .) }}
apiVersion: policy/v1
{{- else }}
apiVersion: policy/v1beta1
{{- end }}
metadata:
name: {{ .Release.Name }}-scheduler-pdb
labels:
Expand Down
54 changes: 45 additions & 9 deletions chart/templates/webserver/webserver-ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
## Airflow Webserver Ingress
#################################
{{- if .Values.ingress.enabled }}
{{- $apiIsStable := semverCompare ">= 1.19.x" (include "kubeVersion" .) -}}
{{- if $apiIsStable }}
apiVersion: networking.k8s.io/v1
{{- else }}
apiVersion: networking.k8s.io/v1beta1
{{- end }}
kind: Ingress
metadata:
name: {{ .Release.Name }}-airflow-ingress
Expand All @@ -29,12 +34,10 @@ metadata:
release: {{ .Release.Name }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: {{ .Release.Service }}
{{- if .Values.ingress.web.annotations }}
{{- with .Values.ingress.web.annotations }}
annotations:
{{- with .Values.ingress.web.annotations }}
{{ toYaml . | indent 4 }}
{{- end }}
{{- end}}
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
{{- if .Values.ingress.web.tls.enabled }}
tls:
Expand All @@ -47,23 +50,56 @@ spec:
paths:
{{- range .Values.ingress.web.precedingPaths }}
- path: {{ .path }}
{{- if $apiIsStable }}
pathType: {{ .pathType }}
{{- end }}
backend:
{{- if $apiIsStable }}
service:
name: {{ .serviceName }}
port:
name: {{ .servicePort }}
{{- else }}
serviceName: {{ .serviceName }}
servicePort: {{ .servicePort }}
{{- end }}
{{- end }}
- backend:
{{- if $apiIsStable }}
service:
name: {{ .Release.Name }}-webserver
port:
name: airflow-ui
{{- else }}
serviceName: {{ .Release.Name }}-webserver
servicePort: airflow-ui
{{- if .Values.ingress.web.path }}
{{- end }}
{{- if .Values.ingress.web.path }}
path: {{ .Values.ingress.web.path }}
{{- end }}
{{- if $apiIsStable }}
pathType: {{ .Values.ingress.web.pathType }}
{{- end }}
{{- end }}
{{- range .Values.ingress.web.succeedingPaths }}
- path: {{ .path }}
{{- if $apiIsStable }}
pathType: {{ .pathType }}
{{- end }}
backend:
{{- if $apiIsStable }}
service:
name: {{ .serviceName }}
port:
name: {{ .servicePort }}
{{- else }}
serviceName: {{ .serviceName }}
servicePort: {{ .servicePort }}
{{- end }}
{{- end }}
{{- if .Values.ingress.web.host }}
{{- if .Values.ingress.web.host }}
host: {{ .Values.ingress.web.host }}
{{- end }}
{{- end }}
{{- if and .Values.ingress.web.ingressClassName $apiIsStable }}
ingressClassName: {{ .Values.ingress.web.ingressClassName }}
{{- end }}
{{- end }}
45 changes: 33 additions & 12 deletions chart/tests/helm_template_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.

import json
import subprocess
import sys
from functools import lru_cache
Expand All @@ -30,26 +31,31 @@

api_client = ApiClient()

BASE_URL_SPEC = "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.15.0"
BASE_URL_SPEC = "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v"
DEFAULT_KUBERNETES_VERSION = "1.22.0"

crd_lookup = {
'keda.sh/v1alpha1::ScaledObject': 'https://raw.githubusercontent.com/kedacore/keda/v2.0.0/config/crd/bases/keda.sh_scaledobjects.yaml', # noqa: E501
}


def get_schema_k8s(api_version, kind):
def get_schema_k8s(api_version, kind, kubernetes_version):
api_version = api_version.lower()
kind = kind.lower()

if '/' in api_version:
ext, _, api_version = api_version.partition("/")
ext = ext.split(".")[0]
url = f'{BASE_URL_SPEC}/{kind}-{ext}-{api_version}.json'
url = f'{BASE_URL_SPEC}{kubernetes_version}/{kind}-{ext}-{api_version}.json'
else:
url = f'{BASE_URL_SPEC}/{kind}-{api_version}.json'
url = f'{BASE_URL_SPEC}{kubernetes_version}/{kind}-{api_version}.json'
request = requests.get(url)
request.raise_for_status()
schema = request.json()
schema = json.loads(
request.text.replace(
'kubernetesjsonschema.dev', 'raw.githubusercontent.com/yannh/kubernetes-json-schema/master'
)
)
return schema


Expand All @@ -64,16 +70,16 @@ def get_schema_crd(api_version, kind):


@lru_cache(maxsize=None)
def create_validator(api_version, kind):
def create_validator(api_version, kind, kubernetes_version):
schema = get_schema_crd(api_version, kind)
if not schema:
schema = get_schema_k8s(api_version, kind)
schema = get_schema_k8s(api_version, kind, kubernetes_version)
jsonschema.Draft7Validator.check_schema(schema)
validator = jsonschema.Draft7Validator(schema)
return validator


def validate_k8s_object(instance):
def validate_k8s_object(instance, kubernetes_version):
# Skip PostgresSQL chart
labels = jmespath.search("metadata.labels", instance)
if "helm.sh/chart" in labels:
Expand All @@ -84,11 +90,17 @@ def validate_k8s_object(instance):
if chart and 'postgresql' in chart:
return

validate = create_validator(instance.get("apiVersion"), instance.get("kind"))
validate = create_validator(instance.get("apiVersion"), instance.get("kind"), kubernetes_version)
validate.validate(instance)


def render_chart(name="RELEASE-NAME", values=None, show_only=None, chart_dir=None):
def render_chart(
name="RELEASE-NAME",
values=None,
show_only=None,
chart_dir=None,
kubernetes_version=DEFAULT_KUBERNETES_VERSION,
):
"""
Function that renders a helm chart into dictionaries. For helm chart testing only
"""
Expand All @@ -98,15 +110,24 @@ def render_chart(name="RELEASE-NAME", values=None, show_only=None, chart_dir=Non
content = yaml.dump(values)
tmp_file.write(content.encode())
tmp_file.flush()
command = ["helm", "template", name, chart_dir, '--values', tmp_file.name]
command = [
"helm",
"template",
name,
chart_dir,
'--values',
tmp_file.name,
'--kube-version',
kubernetes_version,
]
if show_only:
for i in show_only:
command.extend(["--show-only", i])
templates = subprocess.check_output(command, stderr=subprocess.PIPE)
k8s_objects = yaml.full_load_all(templates)
k8s_objects = [k8s_object for k8s_object in k8s_objects if k8s_object] # type: ignore
for k8s_object in k8s_objects:
validate_k8s_object(k8s_object)
validate_k8s_object(k8s_object, kubernetes_version)
return k8s_objects


Expand Down
7 changes: 7 additions & 0 deletions chart/tests/test_cleanup_pods.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ def test_should_create_cronjob_for_enabled_cleanup(self):
"readOnly": True,
} in jmespath.search("spec.jobTemplate.spec.template.spec.containers[0].volumeMounts", docs[0])

def test_should_pass_validation_with_v1beta1_api(self):
render_chart(
values={"cleanup": {"enabled": True}},
show_only=["templates/cleanup/cleanup-cronjob.yaml"],
kubernetes_version='1.16.0',
) # checks that no validation exception is raised

def test_should_change_image_when_set_airflow_image(self):
docs = render_chart(
values={
Expand Down
19 changes: 18 additions & 1 deletion chart/tests/test_ingress_flower.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,19 @@


class IngressFlowerTest(unittest.TestCase):
def test_should_pass_validation_with_just_ingress_enabled(self):
def test_should_pass_validation_with_just_ingress_enabled_v1(self):
render_chart(
values={"ingress": {"enabled": True}, "executor": "CeleryExecutor"},
show_only=["templates/flower/flower-ingress.yaml"],
) # checks that no validation exception is raised

def test_should_pass_validation_with_just_ingress_enabled_v1beta1(self):
render_chart(
values={"ingress": {"enabled": True}, "executor": "CeleryExecutor"},
show_only=["templates/flower/flower-ingress.yaml"],
kubernetes_version='1.16.0',
) # checks that no validation exception is raised

def test_should_allow_more_than_one_annotation(self):
docs = render_chart(
values={
Expand All @@ -38,3 +45,13 @@ def test_should_allow_more_than_one_annotation(self):
show_only=["templates/flower/flower-ingress.yaml"],
)
assert jmespath.search("metadata.annotations", docs[0]) == {"aa": "bb", "cc": "dd"}

def test_should_set_ingress_class_name(self):
docs = render_chart(
values={
"ingress": {"enabled": True, "flower": {"ingressClassName": "foo"}},
"executor": "CeleryExecutor",
},
show_only=["templates/flower/flower-ingress.yaml"],
)
assert "foo" == jmespath.search("spec.ingressClassName", docs[0])
Loading