diff --git a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt index 2a6dc31..e149ff1 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt @@ -11,7 +11,6 @@ */ package com.redhat.devtools.gateway -import com.intellij.openapi.diagnostic.thisLogger import com.jetbrains.gateway.thinClientLink.LinkedClientManager import com.jetbrains.gateway.thinClientLink.ThinClientHandle import com.jetbrains.rd.util.lifetime.Lifetime @@ -19,6 +18,10 @@ import com.redhat.devtools.gateway.openshift.DevWorkspaces import com.redhat.devtools.gateway.openshift.Pods import com.redhat.devtools.gateway.server.RemoteIDEServer import io.kubernetes.client.openapi.ApiException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import java.io.Closeable import java.io.IOException import java.net.ServerSocket import java.net.URI @@ -46,21 +49,14 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { onDisconnected: () -> Unit ): ThinClientHandle { val workspace = devSpacesContext.devWorkspace + devSpacesContext.addWorkspace(workspace) - synchronized(devSpacesContext.activeWorkspaces) { - if (devSpacesContext.activeWorkspaces.contains(workspace)) { - throw IllegalStateException("Workspace '${workspace.name}' is already connected.") - } - devSpacesContext.activeWorkspaces.add(workspace) - } - - var client: ThinClientHandle? = null - var forwarder: AutoCloseable? = null + var remoteIdeServer: RemoteIDEServer? = null + var forwarder: Closeable? = null - try { + return try { startAndWaitDevWorkspace() - - val remoteIdeServer = RemoteIDEServer(devSpacesContext) + remoteIdeServer = RemoteIDEServer(devSpacesContext) val remoteIdeServerStatus = remoteIdeServer.getStatus() val joinLink = remoteIdeServerStatus.joinLink ?: throw IOException("Could not connect, remote IDE is not ready. No join link present.") @@ -72,7 +68,7 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { val effectiveJoinLink = joinLink.replace(":5990", ":$localPort") - client = LinkedClientManager + val client = LinkedClientManager .getInstance() .startNewClient( Lifetime.Eternal, @@ -82,101 +78,40 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { false ) - client.run { - lifetime.onTermination { - try { - forwarder.close() - } catch (_: Exception) { - // ignore cleanup errors - } - } - - lifetime.onTermination { - try { - if (remoteIdeServer.waitServerTerminated()) { - DevWorkspaces(devSpacesContext.client) - .stop( - devSpacesContext.devWorkspace.namespace, - devSpacesContext.devWorkspace.name - ) - onDevWorkspaceStopped() // UI refresh through callback - } - } finally { - synchronized(devSpacesContext.activeWorkspaces) { - devSpacesContext.activeWorkspaces.remove(workspace) - } - onDisconnected() - } - } - - lifetime.onTermination { - onDisconnected() // UI refresh through callback - } + client.clientClosed.advise(client.lifetime) { + onClientClosed(onDisconnected , onDevWorkspaceStopped, remoteIdeServer, forwarder) } - return client + client } catch (e: Exception) { - try { - disconnectAndCleanup(client, forwarder, onDevWorkspaceStopped, onDisconnected) // Cancel if started - } catch (_: Exception) {} - - try { - forwarder?.close() - } catch (_: Exception) {} - - synchronized(devSpacesContext.activeWorkspaces) { - devSpacesContext.activeWorkspaces.remove(workspace) - } - - // Make sure UI refresh still happens on failure - onDisconnected() - + onClientClosed(onDisconnected, onDevWorkspaceStopped, remoteIdeServer, forwarder) throw e } } - private fun disconnectAndCleanup( - client: ThinClientHandle?, - forwarder: AutoCloseable?, + private fun onClientClosed( + onDisconnected: () -> Unit, onDevWorkspaceStopped: () -> Unit, - onDisconnected: () -> Unit + remoteIdeServer: RemoteIDEServer?, + forwarder: Closeable? ) { - if (client == null) { - onDisconnected() - return - } - - try { - // Close the port forwarder first + CoroutineScope(Dispatchers.IO).launch { + val currentWorkspace = devSpacesContext.devWorkspace try { + onDisconnected.invoke() + if (true == remoteIdeServer?.waitServerTerminated()) { + DevWorkspaces(devSpacesContext.client) + .stop( + devSpacesContext.devWorkspace.namespace, + devSpacesContext.devWorkspace.name + ) + .also { onDevWorkspaceStopped() } + } forwarder?.close() - } catch (e: Exception) { - thisLogger().debug("Failed to close port forwarder: ${e.message}") - } - - // Stop workspace cleanly - val devWorkspaces = DevWorkspaces(devSpacesContext.client) - val workspace = devSpacesContext.devWorkspace - - try { - devWorkspaces.stop(workspace.namespace, workspace.name) - onDevWorkspaceStopped() - } catch (e: Exception) { - thisLogger().debug("Workspace stop failed: ${e.message}") - } - - // Remove from active list and update state - synchronized(devSpacesContext.activeWorkspaces) { - devSpacesContext.activeWorkspaces.remove(workspace) - } - onDisconnected() - - } catch (e: Exception) { - thisLogger().debug("Error while terminating client: ${e.message}") - synchronized(devSpacesContext.activeWorkspaces) { - devSpacesContext.activeWorkspaces.remove(devSpacesContext.devWorkspace) + } finally { + devSpacesContext.removeWorkspace(currentWorkspace) + onDisconnected() } - onDisconnected() } } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt index 638df89..ae1508f 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt @@ -17,5 +17,21 @@ import io.kubernetes.client.openapi.ApiClient class DevSpacesContext { lateinit var client: ApiClient lateinit var devWorkspace: DevWorkspace - var activeWorkspaces = mutableSetOf() // Global or companion-level variable + var activeWorkspaces = mutableSetOf() + + fun addWorkspace(workspace: DevWorkspace) { + synchronized(activeWorkspaces) { + if (activeWorkspaces.contains(workspace)) { + return + } + activeWorkspaces.add(workspace) + } + } + + fun removeWorkspace(currentWorkspace: DevWorkspace) { + synchronized(activeWorkspaces) { + activeWorkspaces.remove(currentWorkspace) + } + } + } \ No newline at end of file diff --git a/src/main/kotlin/com/redhat/devtools/gateway/openshift/Pods.kt b/src/main/kotlin/com/redhat/devtools/gateway/openshift/Pods.kt index 8b2b3b2..0929b9c 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/openshift/Pods.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/openshift/Pods.kt @@ -170,6 +170,7 @@ class Pods(private val client: ApiClient) { logger.info("Attempt #${attempt + 1}: Connecting $localPort -> $remotePort...") val portForward = PortForward(client) forwardResult = portForward.forward(pod, listOf(remotePort)) + logger.info("forward successful: $localPort -> $remotePort...") copyStreams(clientSocket, forwardResult, remotePort) return } catch (e: Exception) { diff --git a/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt b/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt index 31d5dee..a4bff27 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt @@ -19,8 +19,10 @@ import com.redhat.devtools.gateway.DevSpacesContext import com.redhat.devtools.gateway.openshift.Pods import io.kubernetes.client.openapi.models.V1Container import io.kubernetes.client.openapi.models.V1Pod -import org.bouncycastle.util.Arrays +import kotlinx.coroutines.delay +import kotlinx.coroutines.withTimeoutOrNull import java.io.IOException +import kotlin.time.Duration.Companion.seconds /** * Represent an IDE server running in a CDE. @@ -68,8 +70,8 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { } @Throws(IOException::class) - fun waitServerReady() { - doWaitServerState(true) + suspend fun waitServerReady() { + doWaitServerProjectExists(true) .also { if (!it) throw IOException( "Remote IDE server is not ready after $readyTimeout seconds.", @@ -78,19 +80,39 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { } @Throws(IOException::class) - fun waitServerTerminated(): Boolean { - return doWaitServerState(false) + suspend fun waitServerTerminated(): Boolean { + return doWaitServerProjectExists(false) } - @Throws(IOException::class) - fun doWaitServerState(isReadyState: Boolean): Boolean { - return try { - val status = getStatus() - isReadyState == !Arrays.isNullOrEmpty(status.projects) - } catch (e: Exception) { - thisLogger().debug("Failed to check remote IDE server state.", e) - false - } + /** + * Waits for the server to have or not have projects according to the given parameter. + * Times out the wait if the expected state is not reached within 10 seconds. + * + * @param expected True if projects are expected, False otherwise, + * @return True if the expected state is achieved within the timeout, False otherwise. + */ + private suspend fun doWaitServerProjectExists(expected: Boolean): Boolean { + val timeout = 10.seconds + + return withTimeoutOrNull(timeout) { + while (true) { + val hasProjects = try { + val status = getStatus() + status.projects.isNotEmpty() + } catch (e: Exception) { + thisLogger().debug("Failed to check remote IDE server state.", e) + null + } + + if (expected == hasProjects) { + return@withTimeoutOrNull true + } + + delay(500L) + } + @Suppress("UNREACHABLE_CODE") + null + } ?: false } @Throws(IOException::class) diff --git a/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt new file mode 100644 index 0000000..5ff0320 --- /dev/null +++ b/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt @@ -0,0 +1,147 @@ +/* + * Copyright (c) 2025 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package com.redhat.devtools.gateway.server + +import com.redhat.devtools.gateway.DevSpacesContext +import com.redhat.devtools.gateway.openshift.Pods +import io.kubernetes.client.openapi.models.V1Container +import io.kubernetes.client.openapi.models.V1ObjectMeta +import io.kubernetes.client.openapi.models.V1Pod +import io.kubernetes.client.openapi.models.V1PodSpec +import io.mockk.* +import kotlinx.coroutines.runBlocking +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import java.io.IOException + +class RemoteIDEServerTest { + + private lateinit var devSpacesContext: DevSpacesContext + private lateinit var remoteIDEServer: RemoteIDEServer + + @BeforeEach + fun setUp() { + devSpacesContext = mockk(relaxed = true) + + mockkConstructor(Pods::class) + val mockPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "test-pod" + } + spec = V1PodSpec().apply { + containers = listOf( + V1Container().apply { + name = "test-container" + ports = listOf( + mockk(relaxed = true) { + every { name } returns "idea-server" + } + ) + } + ) + } + } + coEvery { + anyConstructed().findFirst(any(), any()) + } returns mockPod + + remoteIDEServer = spyk(RemoteIDEServer(devSpacesContext), recordPrivateCalls = true) + } + + @AfterEach + fun tearDown() { + unmockkAll() + } + + @Test + fun `#waitServerTerminated should return true if server terminated`() { + // given + val withProjects = RemoteIDEServerStatus( + null, + "", + "", + "", + "", + arrayOf( + ProjectInfo("test", "test", "test", "test", "test") + ) + ) + val withoutProjects = RemoteIDEServerStatus( + null, + "", + "", + "", + "", + emptyArray() + ) + coEvery { + remoteIDEServer.getStatus() + } returns withProjects andThen withoutProjects + + // when + val result = runBlocking { + remoteIDEServer.waitServerTerminated() + } + + // then + assertThat(result).isTrue + } + + @Test + fun `#waitServerTerminated should return false on timeout`() { + // given + coEvery { + remoteIDEServer.getStatus() + } returns RemoteIDEServerStatus( + null, + "", + "", + "", + "", + arrayOf( + ProjectInfo( + "test", + "test", + "test", + "test", + "test" + ) + ) + ) + + // when + val result = runBlocking { + remoteIDEServer.waitServerTerminated() + } + + // then + assertThat(result).isFalse + } + + @Test + fun `#waitServerTerminated should return false on exception`() { + // given + coEvery { + remoteIDEServer.getStatus() + } throws IOException("error") + + // when + val result = runBlocking { + remoteIDEServer.waitServerTerminated() + } + + // then + assertThat(result).isFalse + } +}