Merge pull request #1828 from manics/remove-incorrect-docker-socket

Remove incorrect docker socket if found
jupyterhub · Mar 2, 2024 · 92609de · 92609de
2 parents 1ec6193 + d6c6604
commit 92609de
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 37 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -299,6 +299,12 @@ jobs:
           timeout: 150
           max-restarts: 1
 
+      - name: Test the workaround for an incorrect docker.sock directory
+        if: matrix.test-variation == 'dind'
+        run: |
+          # {{ .Values.dind.hostSocketDir }}/{{ .Values.dind.hostSocketName }}
+          sudo mkdir -p /var/run/dind/docker.sock
+
       - name: Install the chart
         if: matrix.test == 'helm'
         run: |

diff --git a/helm-chart/binderhub/schema.yaml b/helm-chart/binderhub/schema.yaml
@@ -417,16 +417,20 @@ properties:
       storageDriver:
         type: string
         description: |
-          TODO
+          Docker storage driver
       resources: *resources-spec
-      hostSocketDir:
+      hostSocketDir: &hostSocketDir-spec
         type: string
         description: |
-          TODO
-      hostLibDir:
+          Host directory where the container socket will be located
+      hostLibDir: &hostStorageDir-spec
         type: string
         description: |
-          TODO
+          Host directory where the containers storage will be located
+      hostSocketName: &hostSocketName-spec
+        type: string
+        description: |
+          Name of the container socket file
 
   pink:
     type: object
@@ -446,14 +450,9 @@ properties:
           extraVolumes: *extraVolumes-spec
           extraVolumeMounts: *extraVolumeMounts-spec
       resources: *resources-spec
-      hostStorageDir:
-        type: string
-        description: |
-          Host path where the containers storage will be located
-      hostSocketDir:
-        type: string
-        description: |
-          Host path where the podman socket will be located
+      hostStorageDir: *hostStorageDir-spec
+      hostSocketDir: *hostSocketDir-spec
+      hostSocketName: *hostSocketName-spec
 
   imageCleaner:
     type: object

diff --git a/helm-chart/binderhub/templates/container-builder/daemonset.yaml b/helm-chart/binderhub/templates/container-builder/daemonset.yaml
@@ -2,6 +2,7 @@
 {{- $builderName := .Values.imageBuilderType -}}
 {{- $builder := index .Values $builderName -}}
 {{- $daemonset := $builder.daemonset -}}
+{{- $hostSocketPath := printf "%s/%s" $builder.hostSocketDir $builder.hostSocketName }}
 
 apiVersion: apps/v1
 kind: DaemonSet
@@ -36,8 +37,27 @@ spec:
         value: user
       nodeSelector: {{ .Values.config.BinderHub.build_node_selector | toJson }}
 
-      {{- with $builder.initContainers }}
       initContainers:
+        - name: filesystem
+          # Reuse the main container image since this is a simple shell command
+          image: {{ $daemonset.image.name }}:{{ $daemonset.image.tag }}
+          {{- with $daemonset.image.pullPolicy }}
+          imagePullPolicy: {{ . }}
+          {{- end }}
+          command:
+            - sh
+            - -c
+            - >
+              if [ -d "{{ $hostSocketPath }}" ]; then
+                echo "Removing incorrect socket directory {{ $hostSocketPath }}";
+                rmdir "{{ $hostSocketPath }}";
+              fi
+          securityContext:
+            privileged: true
+          volumeMounts:
+          - name: run-{{ $builderName }}
+            mountPath: {{ $builder.hostSocketDir }}
+      {{- with $builder.initContainers }}
         {{- . | toYaml | nindent 8 }}
       {{- end }}
 
@@ -55,7 +75,7 @@ spec:
           args:
             - dockerd
             - --storage-driver={{ $builder.storageDriver }}
-            - -H unix://{{ $builder.hostSocketDir }}/docker.sock
+            - -H unix://{{ $hostSocketPath }}
             {{- with $daemonset.extraArgs }}
             {{- . | toYaml | nindent 12 }}
             {{- end }}
@@ -73,7 +93,7 @@ spec:
             - system
             - service
             - --time=0
-            - unix://{{ $builder.hostSocketDir }}/podman.sock
+            - unix://{{ $hostSocketPath }}
             {{- with $daemonset.extraArgs }}
             {{- . | toYaml | nindent 12 }}
             {{- end }}

diff --git a/helm-chart/binderhub/templates/image-cleaner.yaml b/helm-chart/binderhub/templates/image-cleaner.yaml
@@ -1,4 +1,7 @@
 {{- if .Values.imageCleaner.enabled -}}
+{{- $builderName := .Values.imageBuilderType -}}
+{{- $builder := index .Values $builderName -}}
+
 apiVersion: apps/v1
 kind: DaemonSet
 metadata:
@@ -35,23 +38,23 @@ spec:
       serviceAccountName: {{ .Release.Name }}-image-cleaner
       {{- end }}
       containers:
-      - name: image-cleaner-{{ .Values.imageBuilderType }}
+      - name: image-cleaner-{{ $builderName }}
         image: {{ .Values.imageCleaner.image.name }}:{{ .Values.imageCleaner.image.tag }}
         {{- with .Values.imageCleaner.image.pullPolicy }}
         imagePullPolicy: {{ . }}
         {{- end }}
         volumeMounts:
-        - name: storage-{{ .Values.imageBuilderType }}
-          mountPath: /var/lib/{{ .Values.imageBuilderType }}
-        - name: socket-{{ .Values.imageBuilderType }}
+        - name: storage-{{ $builderName }}
+          mountPath: /var/lib/{{ $builderName }}
+        - name: socket-{{ $builderName }}
           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/{{ .Values.imageBuilderType }}
+          value: /var/lib/{{ $builderName }}
         - name: DOCKER_IMAGE_CLEANER_DELAY_SECONDS
           value: {{ .Values.imageCleaner.delay | quote }}
         - name: DOCKER_IMAGE_CLEANER_THRESHOLD_TYPE
@@ -62,7 +65,7 @@ spec:
           value: {{ .Values.imageCleaner.imageGCThresholdLow | quote }}
       terminationGracePeriodSeconds: 0
       volumes:
-      {{- if eq .Values.imageBuilderType "host" }}
+      {{- if eq $builderName "host" }}
       - name: storage-host
         hostPath:
           path: {{ .Values.imageCleaner.host.dockerLibDir }}
@@ -71,24 +74,14 @@ spec:
           path: {{ .Values.imageCleaner.host.dockerSocket }}
           type: Socket
       {{- end }}
-      {{- if eq .Values.imageBuilderType "dind" }}
-      - name: storage-dind
+      {{- if or (eq $builderName "dind") (eq $builderName "pink") }}
+      - name: storage-{{ $builderName }}
         hostPath:
-          path: {{ .Values.dind.hostLibDir }}
+          path: {{ eq $builderName "dind" | ternary $builder.hostLibDir $builder.hostStorageDir }}
           type: DirectoryOrCreate
-      - name: socket-dind
+      - name: socket-{{ $builderName }}
         hostPath:
-          path: {{ .Values.dind.hostSocketDir }}/docker.sock
-          type: Socket
-      {{- end }}
-      {{- if eq .Values.imageBuilderType "pink" }}
-      - name: storage-pink
-        hostPath:
-          path: {{ .Values.pink.hostStorageDir }}
-          type: DirectoryOrCreate
-      - name: socket-pink
-        hostPath:
-          path: {{ .Values.pink.hostSocketDir }}/podman.sock
+          path: {{ $builder.hostSocketDir }}/{{ $builder.hostSocketName }}
           type: Socket
       {{- end }}
 {{- end }}
diff --git a/helm-chart/binderhub/values.yaml b/helm-chart/binderhub/values.yaml
@@ -288,6 +288,7 @@ dind:
   resources: {}
   hostSocketDir: /var/run/dind
   hostLibDir: /var/lib/dind
+  hostSocketName: docker.sock
 
 # Podman in Kubernetes
 pink:
@@ -304,6 +305,7 @@ pink:
   resources: {}
   hostStorageDir: /var/lib/pink/storage
   hostSocketDir: /var/run/pink
+  hostSocketName: podman.sock
 
 imageCleaner:
   enabled: true