Merge pull request #1588 from sgaist/refactor_image_cleaner_handling

Simplify image-cleaner handling
jupyterhub · Dec 16, 2022 · 2ba938b · 2ba938b
2 parents 4b10825 + a93f8b5
commit 2ba938b
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 33 deletions.
diff --git a/helm-chart/binderhub/schema.yaml b/helm-chart/binderhub/schema.yaml
@@ -362,8 +362,8 @@ properties:
 
   imageBuilderType:
     type: string
-    enum: ["local", "dind", "pink"]
-    default: "local"
+    enum: ["host", "dind", "pink"]
+    default: "host"
     description: |
       Selected image builder type
 
@@ -479,12 +479,12 @@ properties:
       host:
         type: object
         additionalProperties: false
-        required: [enabled, dockerSocket, dockerLibDir]
+        required: [dockerSocket, dockerLibDir]
         properties:
           enabled:
             type: boolean
             description: |
-              TODO
+              DEPRECATED: use imageCleaner.enabled if the cleaner shall not used.
           dockerSocket:
             type: string
             description: |

diff --git a/helm-chart/binderhub/templates/NOTES.txt b/helm-chart/binderhub/templates/NOTES.txt
@@ -190,6 +190,15 @@ config:
 {{- $breaking = print $breaking "\n\nimageBuilderType: dind" }}
 {{- end }}
 
+{{- if hasKey .Values.imageCleaner.host "enabled" }}
+{{- $breaking = print $breaking "\n\nCHANGED:" }}
+{{- $breaking = print $breaking "\n\nimageCleaner:" }}
+{{- $breaking = print $breaking "\n  host:" }}
+{{- $breaking = print $breaking "\n    enabled: true" }}
+{{- $breaking = print $breaking "\n\nas of version 0.3.0 is not used anymore." }}
+{{- $breaking = print $breaking "\n\nThe image cleaner is either disabled or adapted to the value of imageBuilderType." }}
+{{- end }}
+
 {{- if $breaking }}
 {{- fail (print $breaking_title $breaking) }}
 {{- end }}
diff --git a/helm-chart/binderhub/templates/container-builder/daemonset.yaml b/helm-chart/binderhub/templates/container-builder/daemonset.yaml
@@ -1,7 +1,8 @@
-{{- if ne .Values.imageBuilderType "local" -}}
+{{- if ne .Values.imageBuilderType "host" -}}
 {{- $builderName := .Values.imageBuilderType -}}
 {{- $builder := index .Values $builderName -}}
 {{- $daemonset := $builder.daemonset -}}
+
 apiVersion: apps/v1
 kind: DaemonSet
 metadata:
@@ -46,9 +47,9 @@ spec:
           {{- with $daemonset.image.pullPolicy }}
           imagePullPolicy: {{ . }}
           {{- end }}
-          {{- with $daemonset.resources }}
+          {{- with $builder.resources }}
           resources:
-            {{- $daemonset.resources | toYaml | nindent 12 }}
+            {{- $builder.resources | toYaml | nindent 12 }}
           {{- end }}
           {{- if eq $builderName "dind" }}
           args:

diff --git a/helm-chart/binderhub/templates/image-cleaner.yaml b/helm-chart/binderhub/templates/image-cleaner.yaml
@@ -1,5 +1,4 @@
 {{- if .Values.imageCleaner.enabled -}}
-{{- $Values := .Values -}}
 apiVersion: apps/v1
 kind: DaemonSet
 metadata:
@@ -36,38 +35,34 @@ spec:
       serviceAccountName: {{ .Release.Name }}-image-cleaner
       {{- end }}
       containers:
-      {{- range $i, $kind := tuple "host" "dind" "pink" }}
-      {{- if or (and (eq $kind "dind") (eq $Values.imageBuilderType "dind")) (and (eq $kind "pink") (eq $Values.imageBuilderType "pink")) (and (eq $kind "host") $Values.imageCleaner.host.enabled) }}
-      - name: image-cleaner-{{ $kind }}
-        image: {{ $Values.imageCleaner.image.name }}:{{ $Values.imageCleaner.image.tag }}
-        {{- with $Values.imageCleaner.image.pullPolicy }}
+      - name: image-cleaner-{{ .Values.imageBuilderType }}
+        image: {{ .Values.imageCleaner.image.name }}:{{ .Values.imageCleaner.image.tag }}
+        {{- with .Values.imageCleaner.image.pullPolicy }}
         imagePullPolicy: {{ . }}
         {{- end }}
         volumeMounts:
-        - name: storage-{{ $kind }}
-          mountPath: /var/lib/{{ $kind }}
-        - name: socket-{{ $kind }}
+        - name: storage-{{ .Values.imageBuilderType }}
+          mountPath: /var/lib/{{ .Values.imageBuilderType }}
+        - name: socket-{{ .Values.imageBuilderType }}
           mountPath: /var/run/docker.sock
         env:
         - name: DOCKER_IMAGE_CLEANER_NODE_NAME
           valueFrom:
             fieldRef:
               fieldPath: spec.nodeName
         - name: DOCKER_IMAGE_CLEANER_PATH_TO_CHECK
-          value: /var/lib/{{ $kind }}
+          value: /var/lib/{{ .Values.imageBuilderType }}
         - name: DOCKER_IMAGE_CLEANER_DELAY_SECONDS
-          value: {{ $Values.imageCleaner.delay | quote }}
+          value: {{ .Values.imageCleaner.delay | quote }}
         - name: DOCKER_IMAGE_CLEANER_THRESHOLD_TYPE
-          value: {{ $Values.imageCleaner.imageGCThresholdType | quote }}
+          value: {{ .Values.imageCleaner.imageGCThresholdType | quote }}
         - name: DOCKER_IMAGE_CLEANER_THRESHOLD_HIGH
-          value: {{ $Values.imageCleaner.imageGCThresholdHigh | quote }}
+          value: {{ .Values.imageCleaner.imageGCThresholdHigh | quote }}
         - name: DOCKER_IMAGE_CLEANER_THRESHOLD_LOW
-          value: {{ $Values.imageCleaner.imageGCThresholdLow | quote }}
-      {{- end }}
-      {{- end }}
+          value: {{ .Values.imageCleaner.imageGCThresholdLow | quote }}
       terminationGracePeriodSeconds: 0
       volumes:
-      {{- if .Values.imageCleaner.host.enabled }}
+      {{- if eq .Values.imageBuilderType "host" }}
       - name: storage-host
         hostPath:
           path: {{ .Values.imageCleaner.host.dockerLibDir }}
@@ -76,7 +71,7 @@ spec:
           path: {{ .Values.imageCleaner.host.dockerSocket }}
           type: Socket
       {{- end }}
-      {{- if eq $Values.imageBuilderType "dind" }}
+      {{- if eq .Values.imageBuilderType "dind" }}
       - name: storage-dind
         hostPath:
           path: {{ .Values.dind.hostLibDir }}
@@ -86,7 +81,7 @@ spec:
           path: {{ .Values.dind.hostSocketDir }}/docker.sock
           type: Socket
       {{- end }}
-      {{- if eq $Values.imageBuilderType "pink" }}
+      {{- if eq .Values.imageBuilderType "pink" }}
       - name: storage-pink
         hostPath:
           path: {{ .Values.pink.hostStorageDir }}
@@ -96,5 +91,4 @@ spec:
           path: {{ .Values.pink.hostSocketDir }}/podman.sock
           type: Socket
       {{- end }}
-
 {{- end }}
diff --git a/helm-chart/binderhub/values.yaml b/helm-chart/binderhub/values.yaml
@@ -248,7 +248,7 @@ deployment:
     timeoutSeconds: 10
   labels: {}
 
-imageBuilderType: "local"
+imageBuilderType: "host"
 
 dind:
   initContainers: []
@@ -300,8 +300,8 @@ imageCleaner:
   imageGCThresholdHigh: 80
   imageGCThresholdLow: 60
   # cull images on the host docker as well as dind
+  # configuration to use if `imageBuilderType: host` is configured
   host:
-    enabled: true
     dockerSocket: /var/run/docker.sock
     dockerLibDir: /var/lib/docker
 

diff --git a/testing/k8s-binder-k8s-hub/binderhub-chart-config.yaml b/testing/k8s-binder-k8s-hub/binderhub-chart-config.yaml
@@ -33,12 +33,12 @@ ingress:
   # used actively in our CI system.
   enabled: true
 
-# No in cluster builder to test the creation/update of the image-cleaner DaemonSet
+# No "in cluster" builder to test the creation/update of the image-cleaner DaemonSet
 # resource because it also requires us to setup a container registry to test
 # against which we haven't. We currently only test this through the use of
 # lint-and-validate-values.yaml and setting this value explicitly to make sure
 # our rendered templates are valid against a k8s api-server.
-imageBuilderType: "local"
+imageBuilderType: "host"
 
 # NOTE: This is a mirror of the jupyterhub section in
 #       jupyterhub-chart-config.yaml in testing/local-binder-k8s-hub, keep these

diff --git a/tools/templates/lint-and-validate-values.yaml b/tools/templates/lint-and-validate-values.yaml
@@ -86,7 +86,7 @@ deployment:
   livenessProbe: *probe
   labels: *labels
 
-imageBuilderType: local
+imageBuilderType: host
 
 dind:
   initContainers: &initContainers
@@ -128,7 +128,6 @@ imageCleaner:
   imageGCThresholdHigh: 2
   imageGCThresholdLow: 1
   host:
-    enabled: true
     dockerSocket: /var/run/docker.sock
     dockerLibDir: /var/lib/docker