Fix #35098: support imagePullPolicy in Docker

LEDfan · LEDfan · commit 55d9d4ff071e · 2025-05-20T17:47:08.000+02:00
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/CaddyConfig.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/CaddyConfig.kt
@@ -52,7 +52,7 @@ class CaddyConfig(private val dockerClient: DockerClient, mainDataDir: Path, con
     private val dataDir: Path = mainDataDir.resolve(containerName)
     private val shinyProxies = mutableMapOf<String, Pair<ShinyProxy, ShinyProxyInstance>>()
     private val craneServers = hashMapOf<String, CraneServer>()
-    private val dockerActions = DockerActions(dockerClient)
+    private val dockerActions = DockerActions(dockerClient, config)
     private val logger = KotlinLogging.logger {}
     private val objectMapper = ObjectMapper()
     private val yamlMapper = ObjectMapper(YAMLFactory())
@@ -275,7 +275,7 @@ class CaddyConfig(private val dockerClient: DockerClient, mainDataDir: Path, con
         var containerId = dockerActions.getContainerByName(containerName)?.id()
         if (containerId == null) {
             logger.info { "[Caddy] Pulling image" }
-            dockerClient.pull(caddyImage)
+            dockerActions.pullImage(caddyImage)
 
             val ports = if (enableTls) listOf("80", "443") else listOf("80")
             val hostConfig = HostConfig.builder()
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/CraneConfig.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/CraneConfig.kt
@@ -91,7 +91,7 @@ class CraneConfig(private val dockerClient: DockerClient,
         if (containerId == null) {
             logger.info { "${logPrefix(shinyProxyInstance)} [Crane] Pulling image" }
             val image = getImage(spec)
-            dockerClient.pull(image)
+            dockerActions.pullImage(image)
 
             val suffix = RandomStringUtils.randomAlphanumeric(10)
             val containerName = "sp-crane-${shinyProxyInstance.realmId}-${hash}-${suffix}"
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/DockerActions.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/DockerActions.kt
@@ -20,6 +20,7 @@
  */
 package eu.openanalytics.shinyproxyoperator.impl.docker
 
+import eu.openanalytics.shinyproxyoperator.Config
 import eu.openanalytics.shinyproxyoperator.LabelFactory
 import eu.openanalytics.shinyproxyoperator.LabelFactory.APP_LABEL
 import eu.openanalytics.shinyproxyoperator.LabelFactory.REALM_ID_LABEL
@@ -31,12 +32,14 @@ import io.github.oshai.kotlinlogging.KotlinLogging
 import org.mandas.docker.client.DockerClient
 import org.mandas.docker.client.exceptions.ContainerNotFoundException
 import org.mandas.docker.client.exceptions.NetworkNotFoundException
+import org.mandas.docker.client.exceptions.NotFoundException
 import org.mandas.docker.client.messages.Container
 import org.mandas.docker.client.messages.NetworkConfig
 
-class DockerActions(private val dockerClient: DockerClient) {
+class DockerActions(private val dockerClient: DockerClient, config: Config) {
 
     private val logger = KotlinLogging.logger {}
+    private val imagePullPolicy = config.readConfigValue("Always", "SPO_IMAGE_PULL_POLICY") { it }
 
     fun networkExists(name: String): Boolean {
         try {
@@ -181,4 +184,26 @@ class DockerActions(private val dockerClient: DockerClient) {
         return labels
     }
 
+    /**
+     * Pulls a docker image, respecting the given pull policy.
+     */
+    fun pullImage(image: String, imagePullPolicy: String) {
+        if (imagePullPolicy.equals("Always", true) || (imagePullPolicy.equals("IfNotPresent", true) && !isImagePresent(image))) {
+            dockerClient.pull(image)
+        }
+    }
+
+    fun pullImage(image: String) {
+        pullImage(image, imagePullPolicy)
+    }
+
+    fun isImagePresent(image: String): Boolean {
+        try {
+            dockerClient.inspectImage(image)
+            return true
+        } catch (_: NotFoundException) {
+            return false
+        }
+    }
+
 }
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/DockerOrchestrator.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/DockerOrchestrator.kt
@@ -102,7 +102,7 @@ class DockerOrchestrator(channel: Channel<ShinyProxyEvent>,
             .build()
         dataDirUid = getDatadirUId()
         caddyConfig = CaddyConfig(dockerClient, dataDir, config)
-        dockerActions = DockerActions(dockerClient)
+        dockerActions = DockerActions(dockerClient, config)
         shinyProxyReadyChecker = ShinyProxyReadyChecker(channel, dockerActions, dockerClient, dataDir)
         redisConfig = RedisConfig(dockerClient, dockerActions, persistentState, dataDir, dataDirUid, config)
         craneConfig = CraneConfig(dockerClient, dockerActions, dataDir, inputDir, redisConfig, caddyConfig, persistentState, dataDirUid)
@@ -177,7 +177,7 @@ class DockerOrchestrator(channel: Channel<ShinyProxyEvent>,
             }
 
             logger.info { "${logPrefix(shinyProxyInstance)} [Docker] Pulling image" }
-            dockerClient.pull(shinyProxy.image)
+            dockerActions.pullImage(shinyProxy.image, shinyProxy.imagePullPolicy)
 
             val version = if (shinyProxyInstance.hashOfSpec == shinyProxy.hashOfCurrentSpec) {
                 System.currentTimeMillis()
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/RedisConfig.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/RedisConfig.kt
@@ -76,7 +76,7 @@ class RedisConfig(private val dockerClient: DockerClient,
                 fileManager.createDirectories(dataDir.resolve("data"))
             }
             logger.info { "[Redis] Pulling image" }
-            dockerClient.pull(redisImage)
+            dockerActions.pullImage(redisImage)
 
             withContext(Dispatchers.IO) {
                 fileManager.writeFile(dataDir.resolve("redis.conf"),
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/monitoring/CAdvisorConfig.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/monitoring/CAdvisorConfig.kt
@@ -43,7 +43,7 @@ class CAdvisorConfig(private val dockerClient: DockerClient, private val dockerA
         logger.info { "[cAdvisor] Reconciling" }
         dockerActions.stopAndRemoveContainer(containerName)
         logger.info { "[cAdvisor] Pulling image" }
-        dockerClient.pull(cAdvisorImage)
+        dockerActions.pullImage(cAdvisorImage)
 
         val hostConfig = HostConfig.builder()
             .networkMode(DockerOrchestrator.SHARED_NETWORK_NAME)
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/monitoring/GrafanaConfig.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/monitoring/GrafanaConfig.kt
@@ -81,7 +81,7 @@ class GrafanaConfig(private val dockerClient: DockerClient, private val dockerAc
         logger.info { "[Grafana] Reconciling" }
         dockerActions.stopAndRemoveContainer(containerName)
         logger.info { "${logPrefix(shinyProxy)} [Grafana] Pulling image" }
-        dockerClient.pull(grafanaImage)
+        dockerActions.pullImage(grafanaImage)
 
         val hostConfig = HostConfig.builder()
             .networkMode(DockerOrchestrator.SHARED_NETWORK_NAME)
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/monitoring/GrafanaLokiConfig.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/monitoring/GrafanaLokiConfig.kt
@@ -57,7 +57,7 @@ class GrafanaLokiConfig(private val dockerClient: DockerClient,
         logger.info { "[Grafana Loki] Reconciling" }
         dockerActions.stopAndRemoveContainer(containerName)
         logger.info { "[Grafana Loki] Pulling image" }
-        dockerClient.pull(lokiImage)
+        dockerActions.pullImage(lokiImage)
 
         val hostConfig = HostConfig.builder()
             .networkMode(DockerOrchestrator.SHARED_NETWORK_NAME)
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/monitoring/PrometheusConfig.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/monitoring/PrometheusConfig.kt
@@ -58,7 +58,7 @@ class PrometheusConfig(private val dockerClient: DockerClient,
         logger.info { "[Prometheus] Reconciling" }
         dockerActions.stopAndRemoveContainer(containerName)
         logger.info { "[Prometheus] Pulling image" }
-        dockerClient.pull(prometheusImage)
+        dockerActions.pullImage(prometheusImage)
 
         val hostConfig = HostConfig.builder()
             .networkMode(DockerOrchestrator.SHARED_NETWORK_NAME)
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/kubernetes/ShinyProxy.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/kubernetes/ShinyProxy.kt
@@ -59,13 +59,6 @@ fun ShinyProxy.getAntiAffinityTopologyKey(): String {
     return "kubernetes.io/hostname"
 }
 
-fun ShinyProxy.getImagePullPolicy(): String {
-    if (getSpec().get("imagePullPolicy")?.isTextual == true) {
-        return getSpec().get("imagePullPolicy").textValue()
-    }
-    return "Always"
-}
-
 fun ShinyProxy.getParsedServicePatches(): JsonPatch? {
     if (getSpec().get("kubernetesServicePatches")?.isTextual == true) {
         try {
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/kubernetes/components/PodTemplateSpecFactory.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/impl/kubernetes/components/PodTemplateSpecFactory.kt
@@ -24,7 +24,6 @@ import eu.openanalytics.shinyproxyoperator.Config
 import eu.openanalytics.shinyproxyoperator.LabelFactory
 import eu.openanalytics.shinyproxyoperator.impl.kubernetes.getAntiAffinityRequired
 import eu.openanalytics.shinyproxyoperator.impl.kubernetes.getAntiAffinityTopologyKey
-import eu.openanalytics.shinyproxyoperator.impl.kubernetes.getImagePullPolicy
 import eu.openanalytics.shinyproxyoperator.impl.kubernetes.getParsedKubernetesPodTemplateSpecPatches
 import eu.openanalytics.shinyproxyoperator.model.ShinyProxy
 import eu.openanalytics.shinyproxyoperator.model.ShinyProxyInstance
@@ -68,7 +67,7 @@ class PodTemplateSpecFactory(config: Config) {
                     .addNewContainer()
                         .withName("shinyproxy")
                         .withImage(shinyProxy.image)
-                        .withImagePullPolicy(shinyProxy.getImagePullPolicy())
+                        .withImagePullPolicy(shinyProxy.imagePullPolicy)
                         .addNewPort()
                             .withContainerPort(8080)
                             .withName("http")
diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/model/ShinyProxy.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/model/ShinyProxy.kt
@@ -39,6 +39,14 @@ class ShinyProxy(private val spec: JsonNode, val name: String, val namespace: St
         return@lazy "openanalytics/shinyproxy:latest"
     }
 
+    @get:JsonIgnore
+    val imagePullPolicy: String by lazy {
+        if (getSpec().get("imagePullPolicy")?.isTextual == true) {
+            return@lazy getSpec().get("imagePullPolicy").textValue()
+        }
+        return@lazy "Always"
+    }
+
     @get:JsonIgnore
     val fqdn: String by lazy {
         if (spec.get("fqdn")?.isTextual == true) {
diff --git a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/helpers/IntegrationTestBase.kt b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/impl/docker/helpers/IntegrationTestBase.kt
@@ -73,7 +73,7 @@ abstract class IntegrationTestBase {
         .uri("unix://" + Config().readConfigValue("/var/run/docker.sock", "SPO_DOCKER_SOCKET") { it })
         .readTimeoutMillis(0) // no timeout, needed for startContainer and logs, #32606
         .build()
-    protected val dockerActions = DockerActions(dockerClient)
+    protected val dockerActions = DockerActions(dockerClient, Config())
 
     @AfterEach
     fun cleanup() {
diff --git a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/impl/kubernetes/helpers/ShinyProxyTestInstance.kt b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/impl/kubernetes/helpers/ShinyProxyTestInstance.kt
@@ -27,7 +27,6 @@ import eu.openanalytics.shinyproxyoperator.LabelFactory.INSTANCE_LABEL
 import eu.openanalytics.shinyproxyoperator.LabelFactory.REALM_ID_LABEL
 import eu.openanalytics.shinyproxyoperator.impl.kubernetes.ShinyProxyClient
 import eu.openanalytics.shinyproxyoperator.impl.kubernetes.crd.ShinyProxyCustomResource
-import eu.openanalytics.shinyproxyoperator.impl.kubernetes.getImagePullPolicy
 import eu.openanalytics.shinyproxyoperator.model.ShinyProxy
 import eu.openanalytics.shinyproxyoperator.model.ShinyProxyInstance
 import eu.openanalytics.shinyproxyoperator.model.ShinyProxyStatus
@@ -173,7 +172,7 @@ class ShinyProxyTestInstance(private val namespace: String,
         assertEquals(1, templateSpec.containers.size)
         assertEquals("shinyproxy", templateSpec.containers[0].name)
         assertEquals(sp.image, templateSpec.containers[0].image)
-        assertEquals(sp.getImagePullPolicy(), templateSpec.containers[0].imagePullPolicy)
+        assertEquals(sp.imagePullPolicy, templateSpec.containers[0].imagePullPolicy)
 
         assertEquals(4, templateSpec.containers[0].env.size)
         assertNotNull(templateSpec.containers[0].env.firstOrNull { it.name == "SP_KUBE_POD_UID" })

Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,7 @@ class RedisConfig(private val dockerClient: DockerClient,`
`76`	`76`	`fileManager.createDirectories(dataDir.resolve("data"))`
`77`	`77`	`}`
`78`	`78`	`logger.info { "[Redis] Pulling image" }`
`79`		`- dockerClient.pull(redisImage)`
	`79`	`+ dockerActions.pullImage(redisImage)`
`80`	`80`
`81`	`81`	`withContext(Dispatchers.IO) {`
`82`	`82`	`fileManager.writeFile(dataDir.resolve("redis.conf"),`