From 767913086e93126631abc217b7b02b1da79978e6 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 20 Feb 2024 13:06:27 +0100 Subject: [PATCH] Fix #32402: create new ShinyProxy server when going back to old config --- .../bases/clustered/crds/shinyproxy.crd.yaml | 3 +- .../bases/namespaced/crds/shinyproxy.crd.yaml | 3 +- .../components/LabelFactory.kt | 4 +- .../components/ResourceNameFactory.kt | 4 +- .../controller/PodRetriever.kt | 6 +- .../controller/ShinyProxyController.kt | 20 +++-- .../shinyproxyoperator/crd/ShinyProxy.kt | 3 +- .../crd/ShinyProxyInstance.kt | 2 +- .../crd/ShinyProxyStatus.kt | 2 +- .../shinyproxyoperator/MainIntegrationTest.kt | 74 +++++++++---------- .../helpers/IntegrationTestBase.kt | 11 ++- .../helpers/ShinyProxyTestInstance.kt | 53 ++++++++----- src/test/resources/configs/conflict.yaml | 2 +- src/test/resources/configs/simple_config.yaml | 4 +- .../configs/simple_config_updated.yaml | 4 +- src/test/resources/crd.yaml | 3 +- 16 files changed, 116 insertions(+), 82 deletions(-) diff --git a/docs/deployment/bases/clustered/crds/shinyproxy.crd.yaml b/docs/deployment/bases/clustered/crds/shinyproxy.crd.yaml index 5b017d2..f461d86 100644 --- a/docs/deployment/bases/clustered/crds/shinyproxy.crd.yaml +++ b/docs/deployment/bases/clustered/crds/shinyproxy.crd.yaml @@ -148,6 +148,8 @@ spec: type: string isLatestInstance: type: boolean + revision: + type: integer subresources: status: {} - name: v1alpha1 @@ -290,4 +292,3 @@ spec: type: boolean subresources: status: {} - diff --git a/docs/deployment/bases/namespaced/crds/shinyproxy.crd.yaml b/docs/deployment/bases/namespaced/crds/shinyproxy.crd.yaml index 5b017d2..f461d86 100644 --- a/docs/deployment/bases/namespaced/crds/shinyproxy.crd.yaml +++ b/docs/deployment/bases/namespaced/crds/shinyproxy.crd.yaml @@ -148,6 +148,8 @@ spec: type: string isLatestInstance: type: boolean + revision: + type: integer subresources: status: {} - name: v1alpha1 @@ -290,4 +292,3 @@ spec: type: boolean subresources: status: {} - diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/LabelFactory.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/LabelFactory.kt index 5ef8d8d..bc1c98c 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/LabelFactory.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/LabelFactory.kt @@ -30,7 +30,8 @@ object LabelFactory { return mapOf( APP_LABEL to APP_LABEL_VALUE, REALM_ID_LABEL to shinyProxy.realmId, - INSTANCE_LABEL to hashOfSpec + INSTANCE_LABEL to hashOfSpec, + REVISION_LABEL to shinyProxyInstance.revision.toString() ) } @@ -45,6 +46,7 @@ object LabelFactory { const val APP_LABEL_VALUE = "shinyproxy" const val REALM_ID_LABEL = "openanalytics.eu/sp-realm-id" const val INSTANCE_LABEL = "openanalytics.eu/sp-instance" + const val REVISION_LABEL = "openanalytics.eu/sp-instance-revision" const val LATEST_INSTANCE_LABEL = "openanalytics.eu/sp-latest-instance" const val PROXIED_APP = "openanalytics.eu/sp-proxied-app" diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/ResourceNameFactory.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/ResourceNameFactory.kt index bc7c410..58df159 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/ResourceNameFactory.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/ResourceNameFactory.kt @@ -32,11 +32,11 @@ object ResourceNameFactory { } fun createNameForPod(shinyProxy: ShinyProxy, shinyProxyInstance: ShinyProxyInstance): String { - return "sp-${shinyProxy.metadata.name}-pod-${shinyProxyInstance.hashOfSpec}".take(KUBE_RESOURCE_NAME_MAX_LENGTH) + return "sp-${shinyProxy.metadata.name}-pod-${shinyProxyInstance.revision}-${shinyProxyInstance.hashOfSpec}".take(KUBE_RESOURCE_NAME_MAX_LENGTH) } fun createNameForReplicaSet(shinyProxy: ShinyProxy, shinyProxyInstance: ShinyProxyInstance): String { - return "sp-${shinyProxy.metadata.name}-rs-${shinyProxyInstance.hashOfSpec}".take(KUBE_RESOURCE_NAME_MAX_LENGTH) + return "sp-${shinyProxy.metadata.name}-rs-${shinyProxyInstance.revision}-${shinyProxyInstance.hashOfSpec}".take(KUBE_RESOURCE_NAME_MAX_LENGTH) } fun createNameForService(shinyProxy: ShinyProxy): String { diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/controller/PodRetriever.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/controller/PodRetriever.kt index 832caeb..f811294 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/controller/PodRetriever.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/controller/PodRetriever.kt @@ -29,11 +29,7 @@ import io.fabric8.kubernetes.client.NamespacedKubernetesClient class PodRetriever(private val client: NamespacedKubernetesClient) { fun getShinyProxyPods(shinyProxy: ShinyProxy, shinyProxyInstance: ShinyProxyInstance): List { - val labels = mapOf( - LabelFactory.APP_LABEL to LabelFactory.APP_LABEL_VALUE, - LabelFactory.INSTANCE_LABEL to shinyProxyInstance.hashOfSpec - ) - return client.pods().inNamespace(shinyProxy.metadata.namespace).withLabels(labels).list().items + return client.pods().inNamespace(shinyProxy.metadata.namespace).withLabels(LabelFactory.labelsForShinyProxyInstance(shinyProxy, shinyProxyInstance)).list().items } } diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/controller/ShinyProxyController.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/controller/ShinyProxyController.kt index cc60408..b8160b1 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/controller/ShinyProxyController.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/controller/ShinyProxyController.kt @@ -130,19 +130,23 @@ class ShinyProxyController(private val channel: Channel, if (existingInstance != null && existingInstance.isLatestInstance) { logger.warn { "${shinyProxy.logPrefix(existingInstance)} Trying to create new instance which already exists and is the latest instance" } return existingInstance - } else if (existingInstance != null && !existingInstance.isLatestInstance) { + } + + val revision = if (existingInstance != null) { logger.info { "${shinyProxy.logPrefix(existingInstance)} Trying to create new instance which already exists and is not the latest instance. Therefore this instance will become the latest again" } // reconcile will take care of making this the latest instance again - return existingInstance + existingInstance.revision + 1 + } else { + 0 } // create new instance and add it to the list of instances // initial the instance is not the latest. Only when the ReplicaSet is created and fully running // the latestInstance marker will change to the new instance. - val newInstance = ShinyProxyInstance(shinyProxy.hashOfCurrentSpec, false) + val newInstance = ShinyProxyInstance(shinyProxy.hashOfCurrentSpec, false, revision) updateStatus(shinyProxy) { // Extra check, if this check is positive we have some bug - val checkExistingInstance = it.status.instances.firstOrNull { instance -> instance.hashOfSpec == newInstance.hashOfSpec } + val checkExistingInstance = it.status.instances.firstOrNull { instance -> instance.hashOfSpec == newInstance.hashOfSpec && instance.revision == newInstance.revision } if (checkExistingInstance != null) { // status has already been updated (e.g. after an HTTP 409 Conflict response) // remove the existing instance and add the new one, to ensure that all values are correct. @@ -189,8 +193,7 @@ class ShinyProxyController(private val channel: Channel, } private fun updateLatestMarker(shinyProxy: ShinyProxy, shinyProxyInstance: ShinyProxyInstance) { - val latestInstance = shinyProxy.status.instances.firstOrNull { it.hashOfSpec == shinyProxy.hashOfCurrentSpec } - ?: return + val latestInstance = shinyProxy.status.getInstanceByHash(shinyProxy.hashOfCurrentSpec) ?: return if (latestInstance.isLatestInstance) { // already updated marker return @@ -204,7 +207,7 @@ class ShinyProxyController(private val channel: Channel, updateStatus(shinyProxy) { it.status.instances.forEach { inst -> inst.isLatestInstance = false } - it.status.instances.first { inst -> inst.hashOfSpec == latestInstance.hashOfSpec }.isLatestInstance = true + it.status.getInstanceByHash(latestInstance.hashOfSpec)?.isLatestInstance = true } } @@ -282,7 +285,8 @@ class ShinyProxyController(private val channel: Channel, // take a copy of the list to check to prevent concurrent modification val instancesToCheck = shinyProxy.status.instances.toList() for (shinyProxyInstance in instancesToCheck) { - if (shinyProxyInstance.isLatestInstance || shinyProxyInstance.hashOfSpec == shinyProxy.hashOfCurrentSpec) { + val latestRevision = shinyProxy.status.getInstanceByHash(shinyProxyInstance.hashOfSpec)?.revision ?: 0 + if (shinyProxyInstance.isLatestInstance || (shinyProxyInstance.hashOfSpec == shinyProxy.hashOfCurrentSpec && shinyProxyInstance.revision >= latestRevision)) { // shinyProxyInstance is either the latest or the soon to be latest instance continue } diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxy.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxy.kt index 921d039..ae2d5ec 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxy.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxy.kt @@ -33,6 +33,7 @@ import io.fabric8.kubernetes.model.annotation.Group import io.fabric8.kubernetes.model.annotation.Version import javax.json.JsonPatch + @Version("v1") @Group("openanalytics.eu") class ShinyProxy : CustomResource(), Namespaced { @@ -202,7 +203,7 @@ class ShinyProxy : CustomResource(), Namespaced { } fun logPrefix(shinyProxyInstance: ShinyProxyInstance): String { - return "[${metadata.namespace}/${metadata.name}/${shinyProxyInstance.hashOfSpec}]" + return "[${metadata.namespace}/${metadata.name}/${shinyProxyInstance.hashOfSpec}/${shinyProxyInstance.revision}]" } fun logPrefix(): String { diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxyInstance.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxyInstance.kt index 377260d..3d41841 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxyInstance.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxyInstance.kt @@ -20,4 +20,4 @@ */ package eu.openanalytics.shinyproxyoperator.crd -data class ShinyProxyInstance(val hashOfSpec: String, var isLatestInstance: Boolean) \ No newline at end of file +data class ShinyProxyInstance(val hashOfSpec: String, var isLatestInstance: Boolean, val revision: Int = 0) diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxyStatus.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxyStatus.kt index f35c351..e2cce22 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxyStatus.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/crd/ShinyProxyStatus.kt @@ -29,7 +29,7 @@ import io.fabric8.kubernetes.api.model.KubernetesResource data class ShinyProxyStatus(val instances: ArrayList = arrayListOf()) : KubernetesResource { fun getInstanceByHash(hash: String): ShinyProxyInstance? { - return instances.find { it.hashOfSpec == hash } + return instances.filter { it.hashOfSpec == hash }.maxByOrNull { it.revision } } fun latestInstance(): ShinyProxyInstance? { diff --git a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/MainIntegrationTest.kt b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/MainIntegrationTest.kt index 4aa0e74..b868162 100644 --- a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/MainIntegrationTest.kt +++ b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/MainIntegrationTest.kt @@ -165,19 +165,26 @@ class MainIntegrationTest : IntegrationTestBase() { spTestInstance.assertInstanceIsCorrect() // 5. Delete Replicaset -> reconcile -> assert it is still ok + val replicaSetName = "sp-${sp.metadata.name}-rs-0-${spTestInstance.hash}".take(63) executeAsyncAfter100ms { - stableClient.apps().replicaSets() - .withName("sp-${sp.metadata.name}-rs-${spTestInstance.hash}".take(63)).delete() + getAndDelete(stableClient.apps().replicaSets().withName(replicaSetName)) logger.info { "Deleted ReplicaSet" } } spTestInstance.waitForReconcileCycle() logger.info { "Reconciled after deleting RS" } + // wait for replicaset to be ready + withTimeout(50_000) { + while (stableClient.apps().replicaSets().withName(replicaSetName)?.get()?.status?.readyReplicas != 1){ + logger.info { "Replicaset not yet ready" } + delay(1000) + } + } + logger.info { "Replicaset ready" } spTestInstance.assertInstanceIsCorrect() // 6. Delete ConfigMap -> reconcile -> assert it is still ok executeAsyncAfter100ms { - stableClient.configMaps().withName("sp-${sp.metadata.name}-cm-${spTestInstance.hash}".take(63)) - .delete() + getAndDelete(stableClient.configMaps().withName("sp-${sp.metadata.name}-cm-${spTestInstance.hash}".take(63))) logger.info { "Deleted ConfigMap" } } spTestInstance.waitForOneReconcile() @@ -186,8 +193,7 @@ class MainIntegrationTest : IntegrationTestBase() { // 7. Delete Service -> reconcile -> assert it is still ok executeAsyncAfter100ms { - stableClient.services().withName("sp-${sp.metadata.name}-svc".take(63)) - .delete() + getAndDelete(stableClient.services().withName("sp-${sp.metadata.name}-svc".take(63))) logger.info { "Deleted Service" } } spTestInstance.waitForOneReconcile() @@ -196,8 +202,7 @@ class MainIntegrationTest : IntegrationTestBase() { // 8. Delete Ingress -> reconcile -> assert it is still ok executeAsyncAfter100ms { - stableClient.network().v1().ingresses() - .withName("sp-${sp.metadata.name}-ing".take(63)).delete() + getAndDelete(stableClient.network().v1().ingresses().withName("sp-${sp.metadata.name}-ing".take(63))) logger.info { "Deleted Ingress" } } spTestInstance.waitForReconcileCycle() @@ -354,12 +359,7 @@ class MainIntegrationTest : IntegrationTestBase() { recyclableChecker.isRecyclable = true // 8. wait for delete to happen - while (stableClient.pods().withName("sp-${sp.metadata.name}-pod-${spTestInstanceOriginal.hash}".take(63)).get() != null - || stableClient.configMaps().withName("sp-${sp.metadata.name}-cm-${spTestInstanceOriginal.hash}".take(63)).get() != null - || stableClient.services().withName("sp-${sp.metadata.name}-svc-${spTestInstanceOriginal.hash}".take(63)).get() != null) { - delay(1000) - logger.debug { "Pod still exists!" } - } + spTestInstanceOriginal.waitForDeletion(sp) // 9. assert correctness spTestInstanceUpdated.assertInstanceIsCorrect() @@ -427,12 +427,7 @@ class MainIntegrationTest : IntegrationTestBase() { recyclableChecker.isRecyclable = true // 10. wait for delete to happen - while (stableClient.pods().withName("sp-${sp.metadata.name}-pod-${spTestInstanceOriginal.hash}".take(63)).get() != null - || stableClient.configMaps().withName("sp-${sp.metadata.name}-cm-${spTestInstanceOriginal.hash}".take(63)).get() != null - || stableClient.services().withName("sp-${sp.metadata.name}-svc-${spTestInstanceOriginal.hash}".take(63)).get() != null) { - delay(1000) - logger.debug { "Pod still exists!" } - } + spTestInstanceOriginal.waitForDeletion(sp) // 11. assert older instance does not exist anymore assertThrows("Instance not found") { @@ -636,12 +631,7 @@ class MainIntegrationTest : IntegrationTestBase() { spTestInstanceUpdated.waitForOneReconcile() // 7. wait for delete to happen - while (stableClient.pods().withName("sp-${sp.metadata.name}-pod-${spTestInstanceOriginal.hash}".take(63)).get() != null - || stableClient.configMaps().withName("sp-${sp.metadata.name}-cm-${spTestInstanceOriginal.hash}".take(63)).get() != null - || stableClient.services().withName("sp-${sp.metadata.name}-svc-${spTestInstanceOriginal.hash}".take(63)).get() != null) { - delay(1000) - logger.debug { "Pod still exists!" } - } + spTestInstanceOriginal.waitForDeletion(sp) // 8. assert correctness spTestInstanceUpdated.assertInstanceIsCorrect() @@ -712,8 +702,8 @@ class MainIntegrationTest : IntegrationTestBase() { @Test fun `restore old config version`() = - // idea of test: launch instance A, update config to get instance B, and the update config again - // using the same config as A, resulting in instance A' (which is the same instance as A, as A was never removed!) + // idea of test: launch instance A, update config to get instance B, and the update config again + // the operator will start a new instance, with an increased revision setup(Mode.NAMESPACED) { namespace, shinyProxyClient, namespacedClient, stableClient, operator, reconcileListener, recyclableChecker -> // 1. create a SP instance val instanceA = ShinyProxyTestInstance( @@ -776,22 +766,25 @@ class MainIntegrationTest : IntegrationTestBase() { // 10. wait until instance is created instanceAPrime.waitForOneReconcile() - // 11. wait for delete to happen + // 11. wait for delete of instance A to happen recyclableChecker.isRecyclable = true - while (stableClient.pods().withName("sp-${spB.metadata.name}-pod-${instanceB.hash}".take(63)).get() != null - || stableClient.configMaps().withName("sp-${spB.metadata.name}-cm-${instanceB.hash}".take(63)).get() != null - || stableClient.services().withName("sp-${spB.metadata.name}-svc-${instanceB.hash}".take(63)).get() != null) { - delay(1000) - logger.debug { "Pod still exists!" } + instanceA.waitForDeletion(spA) + + // 12. assert instance A does not exists anymore + assertThrows("Instance not found") { + instanceA.retrieveInstance(0) } - // 12. assert instance B does not exists anymore + // 13. wait for delete of instance B to happen + instanceB.waitForDeletion(spB) + + // 14. assert instance B does not exists anymore assertThrows("Instance not found") { instanceB.retrieveInstance() } // 13. assert instance A' is correct - instanceAPrime.assertInstanceIsCorrect(1, true) + instanceAPrime.assertInstanceIsCorrect(1, true, 1) job.cancel() @@ -952,7 +945,8 @@ class MainIntegrationTest : IntegrationTestBase() { assertEquals(mapOf( LabelFactory.APP_LABEL to LabelFactory.APP_LABEL_VALUE, LabelFactory.REALM_ID_LABEL to sp.realmId, - LabelFactory.INSTANCE_LABEL to spTestInstance.hash + LabelFactory.INSTANCE_LABEL to spTestInstance.hash, + LabelFactory.REVISION_LABEL to "0" ), rule.podAffinityTerm.labelSelector.matchLabels) job.cancel() @@ -1002,7 +996,8 @@ class MainIntegrationTest : IntegrationTestBase() { assertEquals(mapOf( LabelFactory.APP_LABEL to LabelFactory.APP_LABEL_VALUE, LabelFactory.REALM_ID_LABEL to sp.realmId, - LabelFactory.INSTANCE_LABEL to spTestInstance.hash + LabelFactory.INSTANCE_LABEL to spTestInstance.hash, + LabelFactory.REVISION_LABEL to "0" ), rule.labelSelector.matchLabels) job.cancel() @@ -1054,7 +1049,8 @@ class MainIntegrationTest : IntegrationTestBase() { assertEquals(mapOf( LabelFactory.APP_LABEL to LabelFactory.APP_LABEL_VALUE, LabelFactory.REALM_ID_LABEL to sp.realmId, - LabelFactory.INSTANCE_LABEL to spTestInstance.hash + LabelFactory.INSTANCE_LABEL to spTestInstance.hash, + LabelFactory.REVISION_LABEL to "0" ), rule.podAffinityTerm.labelSelector.matchLabels) job.cancel() diff --git a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/IntegrationTestBase.kt b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/IntegrationTestBase.kt index 4e8e787..8f48676 100644 --- a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/IntegrationTestBase.kt +++ b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/IntegrationTestBase.kt @@ -29,10 +29,12 @@ import eu.openanalytics.shinyproxyoperator.createKubernetesClient import eu.openanalytics.shinyproxyoperator.logger import io.fabric8.kubernetes.api.model.NamespaceBuilder import io.fabric8.kubernetes.api.model.PodList +import io.fabric8.kubernetes.api.model.apps.ReplicaSet import io.fabric8.kubernetes.client.KubernetesClient import io.fabric8.kubernetes.client.KubernetesClientException import io.fabric8.kubernetes.client.NamespacedKubernetesClient -import io.fabric8.kubernetes.client.extended.run.RunConfigBuilder +import io.fabric8.kubernetes.client.dsl.Resource +import io.fabric8.kubernetes.client.dsl.RollableScalableResource import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.Job import kotlinx.coroutines.delay @@ -221,6 +223,13 @@ abstract class IntegrationTestBase { )).list() } + protected fun getAndDelete(resource: Resource) { + if (resource.get() == null) { + throw IllegalStateException("Trying to delete resource but it does not exist!") + } + resource.delete() + } + } fun KubernetesClient.isStartupProbesSupported(): Boolean { diff --git a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/ShinyProxyTestInstance.kt b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/ShinyProxyTestInstance.kt index 4a69ce4..ece172a 100644 --- a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/ShinyProxyTestInstance.kt +++ b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/ShinyProxyTestInstance.kt @@ -24,13 +24,16 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.module.kotlin.registerKotlinModule import eu.openanalytics.shinyproxyoperator.ShinyProxyClient import eu.openanalytics.shinyproxyoperator.components.LabelFactory +import eu.openanalytics.shinyproxyoperator.components.ResourceNameFactory import eu.openanalytics.shinyproxyoperator.crd.ShinyProxy import eu.openanalytics.shinyproxyoperator.crd.ShinyProxyInstance +import eu.openanalytics.shinyproxyoperator.logger import io.fabric8.kubernetes.api.model.HasMetadata import io.fabric8.kubernetes.api.model.IntOrString import io.fabric8.kubernetes.client.NamespacedKubernetesClient import io.fabric8.kubernetes.client.readiness.Readiness import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.delay import kotlinx.coroutines.withTimeout import kotlin.test.assertEquals import kotlin.test.assertNotEquals @@ -55,6 +58,10 @@ class ShinyProxyTestInstance(private val namespace: String, return sp } + fun instance(sp: ShinyProxy, revision: Int = 0): ShinyProxyInstance { + return sp.status.instances.firstOrNull { it.hashOfSpec == hash && it.revision == revision } ?: error("ShinyProxyInstance with hash $hash not found") + } + suspend fun waitForOneReconcile(): ShinyProxyInstance { return withTimeout(120_000) { reconcileListener.waitForNextReconcile(hash).await() @@ -77,8 +84,18 @@ class ShinyProxyTestInstance(private val namespace: String, } } - fun assertInstanceIsCorrect(numInstancesRunning: Int = 1, isLatest: Boolean = true) { - val sp = retrieveInstance() + suspend fun waitForDeletion(sp: ShinyProxy, revision: Int = 0) { + val instance = ShinyProxyInstance(hash, false, revision) + while (client.apps().replicaSets().withName(ResourceNameFactory.createNameForReplicaSet(sp, instance)).get() != null + || client.configMaps().withName(ResourceNameFactory.createNameForReplicaSet(sp, instance)).get() != null) { + delay(1000) + logger.debug { "Pod still exists ${hash}!" } + } + } + + + fun assertInstanceIsCorrect(numInstancesRunning: Int = 1, isLatest: Boolean = true, revision: Int = 0) { + val sp = retrieveInstance(revision) assertNotNull(sp) val instance = sp.status.instances.firstOrNull { it.hashOfSpec == hash } assertNotNull(instance) @@ -86,13 +103,13 @@ class ShinyProxyTestInstance(private val namespace: String, assertEquals(numInstancesRunning, sp.status.instances.size) // check configmap - assertConfigMapIsCorrect(sp, numInstancesRunning, isLatest) + assertConfigMapIsCorrect(sp, numInstancesRunning, isLatest, revision) // check replicaset - assertReplicaSetIsCorrect(sp, numInstancesRunning) + assertReplicaSetIsCorrect(sp, numInstancesRunning, revision) // check service - assertServiceIsCorrect(sp) + assertServiceIsCorrect(sp, revision) // check ingress assertIngressIsCorrect(sp) @@ -124,7 +141,7 @@ class ShinyProxyTestInstance(private val namespace: String, assertEquals(80, path.backend.service.port.number) } - fun assertServiceIsCorrect(sp: ShinyProxy) { + fun assertServiceIsCorrect(sp: ShinyProxy, revision: Int = 0) { val services = client.inNamespace(namespace).services().list().items assertEquals(1, services.size) val service = services.firstOrNull { it.metadata.name == "sp-${sp.metadata.name}-svc".take(63) } @@ -145,12 +162,13 @@ class ShinyProxyTestInstance(private val namespace: String, assertEquals(mapOf( LabelFactory.APP_LABEL to LabelFactory.APP_LABEL_VALUE, LabelFactory.REALM_ID_LABEL to sp.realmId, - LabelFactory.INSTANCE_LABEL to sp.status.latestInstance()!!.hashOfSpec // TODO + LabelFactory.INSTANCE_LABEL to sp.status.latestInstance()!!.hashOfSpec, + LabelFactory.REVISION_LABEL to revision.toString() ), service.spec.selector) } - fun assertConfigMapIsCorrect(sp: ShinyProxy, numInstancesRunning: Int = 1, isLatest: Boolean = true) { + fun assertConfigMapIsCorrect(sp: ShinyProxy, numInstancesRunning: Int = 1, isLatest: Boolean = true, revision: Int = 0) { val configMaps = client.inNamespace(namespace).configMaps().list().items.filter { it.metadata.name != "kube-root-ca.crt" } assertEquals(numInstancesRunning, configMaps.size) val configMap = configMaps.firstOrNull { it.metadata.labels[LabelFactory.INSTANCE_LABEL] == hash } @@ -163,13 +181,13 @@ class ShinyProxyTestInstance(private val namespace: String, assertNotEquals(sp.specAsYaml, configMap.data["application.yml"]) } - assertLabelsAreCorrect(configMap, sp) + assertLabelsAreCorrect(configMap, sp, revision) assertOwnerReferenceIsCorrect(configMap, sp) // assertTrue(configMap.immutable) // TODO make the configmap immutable? } - fun assertReplicaSetIsCorrect(sp: ShinyProxy, numInstancesRunning: Int = 1) { + fun assertReplicaSetIsCorrect(sp: ShinyProxy, numInstancesRunning: Int = 1, revision: Int = 0) { val replicaSets = client.inNamespace(namespace).apps().replicaSets().list().items assertEquals(numInstancesRunning, replicaSets.size) val replicaSet = replicaSets.firstOrNull { it.metadata.labels[LabelFactory.INSTANCE_LABEL] == hash } @@ -177,8 +195,8 @@ class ShinyProxyTestInstance(private val namespace: String, assertEquals(1, replicaSet.status.replicas) assertEquals(1, replicaSet.status.readyReplicas) assertEquals(1, replicaSet.status.availableReplicas) - assertEquals("sp-${sp.metadata.name}-rs-${hash}".take(63), replicaSet.metadata.name) - assertLabelsAreCorrect(replicaSet, sp) + assertEquals("sp-${sp.metadata.name}-rs-${revision}-${hash}".take(63), replicaSet.metadata.name) + assertLabelsAreCorrect(replicaSet, sp, revision) assertOwnerReferenceIsCorrect(replicaSet, sp) val templateSpec = replicaSet.spec.template.spec @@ -192,7 +210,7 @@ class ShinyProxyTestInstance(private val namespace: String, assertNotNull(templateSpec.containers[0].env.firstOrNull { it.name == "SP_KUBE_POD_NAME" }) assertNotNull(templateSpec.containers[0].env.firstOrNull { it.name == "PROXY_REALM_ID" }) assertNotNull(templateSpec.containers[0].env.firstOrNull { it.name == "PROXY_VERSION" }) - assertEquals(sp.metadata.name + '-'+ sp.metadata.namespace, templateSpec.containers[0].env.firstOrNull { it.name == "PROXY_REALM_ID" }?.value) + assertEquals(sp.metadata.name + '-' + sp.metadata.namespace, templateSpec.containers[0].env.firstOrNull { it.name == "PROXY_REALM_ID" }?.value) assertEquals(1, templateSpec.containers[0].volumeMounts.size) assertEquals("config-volume", templateSpec.containers[0].volumeMounts[0].name) @@ -222,11 +240,12 @@ class ShinyProxyTestInstance(private val namespace: String, assertTrue(Readiness.getInstance().isReady(replicaSet)) } - fun assertLabelsAreCorrect(resource: HasMetadata, sp: ShinyProxy) { + fun assertLabelsAreCorrect(resource: HasMetadata, sp: ShinyProxy, revision: Int) { assertEquals(mapOf( LabelFactory.APP_LABEL to LabelFactory.APP_LABEL_VALUE, LabelFactory.REALM_ID_LABEL to sp.realmId, - LabelFactory.INSTANCE_LABEL to hash + LabelFactory.INSTANCE_LABEL to hash, + LabelFactory.REVISION_LABEL to revision.toString() ), resource.metadata.labels) } @@ -239,9 +258,9 @@ class ShinyProxyTestInstance(private val namespace: String, assertEquals(sp.metadata.uid, resource.metadata.ownerReferences[0].uid) } - fun retrieveInstance(): ShinyProxy { + fun retrieveInstance(revision: Int = 0): ShinyProxy { for (sp in shinyProxyClient.inNamespace(namespace).list().items) { - if (sp != null && sp.status.instances.find { it.hashOfSpec == hash } != null) { + if (sp != null && sp.status.instances.find { it.hashOfSpec == hash && it.revision == revision } != null) { return sp } } diff --git a/src/test/resources/configs/conflict.yaml b/src/test/resources/configs/conflict.yaml index 462e623..d9598f8 100644 --- a/src/test/resources/configs/conflict.yaml +++ b/src/test/resources/configs/conflict.yaml @@ -1,7 +1,7 @@ apiVersion: v1 kind: ConfigMap metadata: - name: sp-example-shinyproxy-cm-502467f68c769f1a6a3f641603ec053e15a350 + name: sp-example-shinyproxy-cm-f5b84adc3f917d9256a0c779fd53080f13033e namespace: itest data: application.yml: | diff --git a/src/test/resources/configs/simple_config.yaml b/src/test/resources/configs/simple_config.yaml index 481563c..f7fda83 100644 --- a/src/test/resources/configs/simple_config.yaml +++ b/src/test/resources/configs/simple_config.yaml @@ -10,12 +10,14 @@ spec: logoUrl: http://www.openanalytics.eu/sites/www.openanalytics.eu/themes/oa/logo.png landingPage: / heartbeatRate: 10000 - heartbeatTimeout: 60000 + heartbeatTimeout: -1 port: 8080 authentication: simple containerBackend: kubernetes + stop-proxies-on-shutdown: false kubernetes: namespace: itest + internal-networking: true users: - name: demo password: demo diff --git a/src/test/resources/configs/simple_config_updated.yaml b/src/test/resources/configs/simple_config_updated.yaml index 2a165d0..33d85e6 100644 --- a/src/test/resources/configs/simple_config_updated.yaml +++ b/src/test/resources/configs/simple_config_updated.yaml @@ -10,12 +10,14 @@ spec: logoUrl: http://www.openanalytics.eu/sites/www.openanalytics.eu/themes/oa/logo.png landingPage: / heartbeatRate: 10000 - heartbeatTimeout: 60000 + heartbeatTimeout: -1 port: 8080 authentication: simple containerBackend: kubernetes + stop-proxies-on-shutdown: false kubernetes: namespace: itest + internal-networking: true users: - name: demo password: demo diff --git a/src/test/resources/crd.yaml b/src/test/resources/crd.yaml index 5b017d2..f461d86 100644 --- a/src/test/resources/crd.yaml +++ b/src/test/resources/crd.yaml @@ -148,6 +148,8 @@ spec: type: string isLatestInstance: type: boolean + revision: + type: integer subresources: status: {} - name: v1alpha1 @@ -290,4 +292,3 @@ spec: type: boolean subresources: status: {} -