diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/ServiceFactory.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/ServiceFactory.kt index 4078502..5a42e32 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/ServiceFactory.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/components/ServiceFactory.kt @@ -39,6 +39,7 @@ class ServiceFactory(private val serviceClient: MixedOperation, val shinyProxy = refreshShinyProxy(_shinyProxy) // refresh shinyproxy to ensure status is always up to date val existingInstance = shinyProxy.status.getInstanceByHash(shinyProxy.hashOfCurrentSpec) - 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 - } - 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 diff --git a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/isInManagedNamespace.kt b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/isInManagedNamespace.kt index 5d6216d..ba06086 100644 --- a/src/main/kotlin/eu/openanalytics/shinyproxyoperator/isInManagedNamespace.kt +++ b/src/main/kotlin/eu/openanalytics/shinyproxyoperator/isInManagedNamespace.kt @@ -23,7 +23,7 @@ package eu.openanalytics.shinyproxyoperator import eu.openanalytics.shinyproxyoperator.crd.ShinyProxy import mu.KotlinLogging -val logger = KotlinLogging.logger { } +private val logger = KotlinLogging.logger { } fun isInManagedNamespace(shinyProxy: ShinyProxy): Boolean { when (Operator.getOperatorInstance().mode) { diff --git a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/MainIntegrationTest.kt b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/MainIntegrationTest.kt index 33fa627..176b0c8 100644 --- a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/MainIntegrationTest.kt +++ b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/MainIntegrationTest.kt @@ -680,7 +680,7 @@ 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 + // idea of test: launch instance A, update config to get instance B, and then update config again to A // 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 @@ -723,14 +723,95 @@ class MainIntegrationTest : IntegrationTestBase() { // 7. wait until instance is created instanceB.waitForOneReconcile() - // 7. wait for delete to not happen + // 8. wait for delete to not happen delay(5000) - // 8. assert that two instances are correctly working + // 9. assert that two instances are correctly working instanceA.assertInstanceIsCorrect(2, false) instanceB.assertInstanceIsCorrect(2, true) - // 9. update config to again have the config of A + // 10. update config to again have the config of A + logger.debug { "Updating config to get A'" } + val instanceAPrime = ShinyProxyTestInstance( + namespace, + namespacedClient, + shinyProxyClient, + "simple_config.yaml", + reconcileListener + ) + instanceAPrime.create() + + // 11. wait until instance is created + instanceAPrime.waitForOneReconcile() + + // 12. wait for delete of instance A to happen + recyclableChecker.isRecyclable = true + instanceA.waitForDeletion(spA) + + // 13. assert instance A does not exists anymore + assertThrows("Instance not found") { + instanceA.retrieveInstance(0) + } + + // 14. wait for delete of instance B to happen + instanceB.waitForDeletion(spB) + + // 15. assert instance B does not exists anymore + assertThrows("Instance not found") { + instanceB.retrieveInstance() + } + + // 16. assert instance A' is correct + instanceAPrime.assertInstanceIsCorrect(1, true, 1) + } + + @Test + fun `restore old config version when new instance fails`() = + // see #33546 + // idea of test: launch instance A, update config to get instance B, however, instance B fails to start up, and then update config again to A + // 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( + namespace, + namespacedClient, + shinyProxyClient, + "simple_config.yaml", + reconcileListener + ) + val spA = instanceA.create() + + val (resourceRetriever, shinyProxyLister) = operator.prepare() + // 2. start the operator and let it do it's work + scope.launch { + operator.run(resourceRetriever, shinyProxyLister) + } + + // 3. wait until instance is created + instanceA.waitForOneReconcile() + + // 4. assert correctness + instanceA.assertInstanceIsCorrect() + + // 5. launch an app + startApp(spA, instanceA) + + // 6. update ShinyProxy instance + logger.debug { "Base instance created -> updating it" } + val instanceB = ShinyProxyTestInstance( + namespace, + namespacedClient, + shinyProxyClient, + "simple_config_updated_broken.yaml", + reconcileListener + ) + val spB = instanceB.create() + logger.debug { "Base instance created -> updated" } + + // 7. wait for instance to startup (startup will fail) + delay(100_000) // TODO replace by event listener + + // 8. update config to again have the config of A logger.debug { "Updating config to get A'" } val instanceAPrime = ShinyProxyTestInstance( namespace, @@ -741,27 +822,27 @@ class MainIntegrationTest : IntegrationTestBase() { ) instanceAPrime.create() - // 10. wait until instance is created + // 9. wait until instance is created instanceAPrime.waitForOneReconcile() - // 11. wait for delete of instance A to happen + // 10. wait for delete of instance A to happen recyclableChecker.isRecyclable = true instanceA.waitForDeletion(spA) - // 12. assert instance A does not exists anymore + // 11. assert instance A does not exists anymore assertThrows("Instance not found") { instanceA.retrieveInstance(0) } - // 13. wait for delete of instance B to happen + // 12. wait for delete of instance B to happen instanceB.waitForDeletion(spB) - // 14. assert instance B does not exists anymore + // 13. assert instance B does not exists anymore assertThrows("Instance not found") { instanceB.retrieveInstance() } - // 13. assert instance A' is correct + // 14. assert instance A' is correct instanceAPrime.assertInstanceIsCorrect(1, true, 1) } @@ -1025,7 +1106,7 @@ class MainIntegrationTest : IntegrationTestBase() { } @Test - fun `test additional fqns`() = + fun `test additional fqdns`() = setup(Mode.NAMESPACED) { namespace, shinyProxyClient, namespacedClient, _, operator, reconcileListener, _ -> // 1. create a SP instance val spTestInstance = ShinyProxyTestInstance( diff --git a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/IntegrationTestBase.kt b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/IntegrationTestBase.kt index 23d4226..cf8397e 100644 --- a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/IntegrationTestBase.kt +++ b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/IntegrationTestBase.kt @@ -26,15 +26,12 @@ import eu.openanalytics.shinyproxyoperator.ShinyProxyClient import eu.openanalytics.shinyproxyoperator.components.LabelFactory import eu.openanalytics.shinyproxyoperator.crd.ShinyProxy 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.dsl.Resource -import io.fabric8.kubernetes.client.dsl.RollableScalableResource import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope @@ -44,6 +41,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout +import mu.KotlinLogging import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.AfterEach @@ -55,6 +53,7 @@ abstract class IntegrationTestBase { protected val chaosEnabled = System.getenv("SPO_TEST_CHAOS") != null + private val logger = KotlinLogging.logger { } private val stableClient: NamespacedKubernetesClient = createKubernetesClient() private val chaosClient: NamespacedKubernetesClient = if (chaosEnabled) { ChaosInterceptor.createChaosKubernetesClient() diff --git a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/ShinyProxyTestInstance.kt b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/ShinyProxyTestInstance.kt index 4d7d93c..9512316 100644 --- a/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/ShinyProxyTestInstance.kt +++ b/src/test/kotlin/eu/openanalytics/shinyproxyoperator/helpers/ShinyProxyTestInstance.kt @@ -27,7 +27,6 @@ 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 @@ -35,6 +34,7 @@ import io.fabric8.kubernetes.client.readiness.Readiness import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.delay import kotlinx.coroutines.withTimeout +import mu.KotlinLogging import kotlin.test.assertEquals import kotlin.test.assertNotEquals import kotlin.test.assertNotNull @@ -47,7 +47,7 @@ class ShinyProxyTestInstance(private val namespace: String, private val reconcileListener: ReconcileListener) { lateinit var hash: String - private val objectMapper = ObjectMapper().registerKotlinModule() + private val logger = KotlinLogging.logger { } fun create(): ShinyProxy { val sp: ShinyProxy = shinyProxyClient.inNamespace(namespace).load(this.javaClass.getResourceAsStream("/configs/$fileName")).serverSideApply() @@ -59,7 +59,7 @@ class ShinyProxyTestInstance(private val namespace: String, } 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") + return sp.status.instances.firstOrNull { it.hashOfSpec == hash && it.revision == revision } ?: error("ShinyProxyInstance with hash $hash not found") } suspend fun waitForOneReconcile(): ShinyProxyInstance { @@ -87,7 +87,8 @@ class ShinyProxyTestInstance(private val namespace: String, 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) { + || client.configMaps().withName(ResourceNameFactory.createNameForReplicaSet(sp, instance)).get() != null + ) { delay(1000) logger.debug { "Pod still exists ${hash}!" } } @@ -156,7 +157,8 @@ class ShinyProxyTestInstance(private val namespace: String, assertEquals(mapOf( LabelFactory.APP_LABEL to LabelFactory.APP_LABEL_VALUE, LabelFactory.REALM_ID_LABEL to sp.realmId, - LabelFactory.LATEST_INSTANCE_LABEL to sp.status.latestInstance()!!.hashOfSpec + LabelFactory.LATEST_INSTANCE_LABEL to sp.status.latestInstance()!!.hashOfSpec, + LabelFactory.REVISION_LABEL to revision.toString() ), service.metadata.labels) assertOwnerReferenceIsCorrect(service, sp) diff --git a/src/test/resources/configs/simple_config_updated_broken.yaml b/src/test/resources/configs/simple_config_updated_broken.yaml new file mode 100644 index 0000000..2f2e3d8 --- /dev/null +++ b/src/test/resources/configs/simple_config_updated_broken.yaml @@ -0,0 +1,40 @@ +apiVersion: openanalytics.eu/v1 +kind: ShinyProxy +metadata: + name: example-shinyproxy +spec: + management: + server: + port: 9091 + fqdn: itest.local + image: openanalytics/shinyproxy-snapshot:3.1.0-SNAPSHOT-20240219.093646 + proxy: + title: Open Analytics Shiny Proxy 2 + logoUrl: http://www.openanalytics.eu/sites/www.openanalytics.eu/themes/oa/logo.png + landingPage: / + heartbeatRate: 10000 + heartbeatTimeout: -1 + port: 8080 + authentication: simple + containerBackend: kubernetes + stop-proxies-on-shutdown: false + default-stop-proxy-on-logout: false + kubernetes: + namespace: itest + internal-networking: true + users: + - name: demo + password: demo + groups: scientists + - name: demo2 + password: demo2 + groups: mathematicians + specs: + - id: 01_hello + displayName: Hello Application + description: Application which demonstrates the basics of a Shiny app + containerCmd: [ "R", "-e", "shinyproxy::run_01_hello()" ] + containerImage: openanalytics/shinyproxy-integration-test-app + - id: 06_tabsets + container-cmd: [ "R", "-e", "shinyproxy::run_06_tabsets()" ] + container-image: openanalytics/shinyproxy-integration-test-app