From e08809ded37640946517c37ae4ed54cf9774f6ec Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Thu, 1 Jul 2021 22:57:42 +0530 Subject: [PATCH] Fix #653: `k8s:watch` port-forward websocket error due to wrong arguments in PortForwardService Make order of port forward arguments consistent across Service classes. Now all methods have containerPort, remotePort argument format Signed-off-by: Rohan Kumar --- CHANGELOG.md | 1 + .../kit/config/service/DebugService.java | 10 +- .../config/service/PortForwardService.java | 19 ++-- .../PortForwardServicePortOrderTest.java | 43 ++++++++ .../service/PortForwardServiceTest.java | 4 +- .../springboot/watcher/SpringBootWatcher.java | 8 +- .../watcher/SpringBootWatcherTest.java | 103 ++++++++++++++++++ 7 files changed, 170 insertions(+), 18 deletions(-) create mode 100644 jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServicePortOrderTest.java create mode 100644 jkube-kit/jkube-kit-spring-boot/src/test/java/org/eclipse/jkube/springboot/watcher/SpringBootWatcherTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index e63aa50f96..e0103070b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Usage: ### 1.4.0-SNAPSHOT * Fix #741: Private constructor added to Utility classes * Fix #725: Upgrade HttpClient from 4.5.10 to 4.5.13 +* Fix #653: `k8s:watch` port-forward websocket error due to wrong arguments in PortForwardService * Fix #704: NPE when using Service fragment with no port specified * Fix #705: JIB assembly works on Windows * Fix #714: feat: Helm support for Golang expressions diff --git a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/DebugService.java b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/DebugService.java index 37901a9420..2c76960e64 100644 --- a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/DebugService.java +++ b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/DebugService.java @@ -60,7 +60,7 @@ public class DebugService { private final PortForwardService portForwardService; private final ApplyService applyService; private String debugSuspendValue; - private String remoteDebugPort = DebugConstants.ENV_VAR_JAVA_DEBUG_PORT_DEFAULT; + private String debugPortInContainer = DebugConstants.ENV_VAR_JAVA_DEBUG_PORT_DEFAULT; public DebugService(KitLogger log, KubernetesClient kubernetesClient, PortForwardService portForwardService, ApplyService applyService) { this.log = log; @@ -116,7 +116,7 @@ private void startPortForward( if (firstSelector != null) { Map envVars = initDebugEnvVarsMap(debugSuspend); String podName = waitForRunningPodWithEnvVar(namespace, firstSelector, envVars, podWaitLog); - portForwardService.startPortForward(podName, namespace, portToInt(remoteDebugPort, "remoteDebugPort"), portToInt(localDebugPort, "localDebugPort")); + portForwardService.startPortForward(podName, namespace, portToInt(debugPortInContainer, "containerDebugPort"), portToInt(localDebugPort, "localDebugPort")); } } @@ -257,8 +257,8 @@ private boolean addDebugContainerPort(Container container) { if (ports == null) { ports = new ArrayList<>(); } - if (!KubernetesHelper.containsPort(ports, remoteDebugPort)) { - ContainerPort port = KubernetesHelper.addPort(remoteDebugPort, "debug", log); + if (!KubernetesHelper.containsPort(ports, debugPortInContainer)) { + ContainerPort port = KubernetesHelper.addPort(debugPortInContainer, "debug", log); if (port != null) { ports.add(port); container.setPorts(ports); @@ -274,7 +274,7 @@ private boolean setDebugEnvVar(Container container, boolean debugSuspend) { if (env == null) { env = new ArrayList<>(); } - remoteDebugPort = KubernetesHelper.getEnvVar(env, DebugConstants.ENV_VAR_JAVA_DEBUG_PORT, DebugConstants.ENV_VAR_JAVA_DEBUG_PORT_DEFAULT); + debugPortInContainer = KubernetesHelper.getEnvVar(env, DebugConstants.ENV_VAR_JAVA_DEBUG_PORT, DebugConstants.ENV_VAR_JAVA_DEBUG_PORT_DEFAULT); if (KubernetesHelper.setEnvVar(env, DebugConstants.ENV_VAR_JAVA_DEBUG, "true")) { container.setEnv(env); enabled = true; diff --git a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/PortForwardService.java b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/PortForwardService.java index cd213472ab..6f02ecbdd8 100644 --- a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/PortForwardService.java +++ b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/PortForwardService.java @@ -56,8 +56,13 @@ public PortForwardService(KubernetesClient kubernetes, KitLogger log) { /** * Forwards a port to the newest pod matching the given selector. * If another pod is created, it forwards connections to the new pod once it's ready. + * + * @param podSelector Pod label selector + * @param containerPort port inside Pod container running inside Kubernetes Cluster + * @param localPort port at remote machine outside Kubernetes Cluster + * @return {@link Closeable} Closeable */ - public Closeable forwardPortAsync(final LabelSelector podSelector, final int remotePort, final int localPort) { + public Closeable forwardPortAsync(final LabelSelector podSelector, final int containerPort, final int localPort) { final Lock monitor = new ReentrantLock(true); final Condition podChanged = monitor.newCondition(); @@ -90,7 +95,7 @@ public void run() { if (nextPod != null) { log.info("Starting port-forward to pod %s", KubernetesHelper.getName(nextPod)); - currentPortForward = forwardPortAsync( KubernetesHelper.getName(nextPod), KubernetesHelper.getNamespace(nextPod), remotePort, localPort); + currentPortForward = forwardPortAsync(KubernetesHelper.getName(nextPod), KubernetesHelper.getNamespace(nextPod), containerPort, localPort); } else { log.info("Waiting for a pod to become ready before starting port-forward"); } @@ -215,13 +220,13 @@ private Pod getNewestPod(List items) { } // Visible for test - LocalPortForward forwardPortAsync(String podName, String namespace, int remotePort, int localPort) { - return kubernetes.pods().inNamespace(namespace).withName(podName).portForward(localPort, remotePort); + LocalPortForward forwardPortAsync(String podName, String namespace, int containerPort, int localPort) { + return kubernetes.pods().inNamespace(namespace).withName(podName).portForward(containerPort, localPort); } - void startPortForward(String pod, String namespace, int remotePort, int localPort) { - log.info("Starting port forwarding to port %s on pod %s", remotePort, pod); - LocalPortForward localPortForward = forwardPortAsync(pod, namespace, remotePort, localPort); + void startPortForward(String pod, String namespace, int containerPort, int localPort) { + log.info("Starting port forwarding to port %s on pod %s", localPort, pod); + LocalPortForward localPortForward = forwardPortAsync(pod, namespace, containerPort, localPort); log.info("Port Forwarding started"); log.info("Now you can start a Remote debug session by using localhost and the debug port %s", localPort); diff --git a/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServicePortOrderTest.java b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServicePortOrderTest.java new file mode 100644 index 0000000000..a01da27190 --- /dev/null +++ b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServicePortOrderTest.java @@ -0,0 +1,43 @@ +/** + * Copyright (c) 2019 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 org.eclipse.jkube.kit.config.service; + +import io.fabric8.kubernetes.client.KubernetesClient; +import mockit.Mocked; +import mockit.Verifications; +import org.eclipse.jkube.kit.common.KitLogger; +import org.junit.Test; + +public class PortForwardServicePortOrderTest { + @Mocked + private KubernetesClient kubernetesClient; + + @Mocked + private KitLogger logger; + + @Test + public void testPortsSpecifiedInCorrectOrderPortForward() { + // Given + PortForwardService portForwardService = new PortForwardService(kubernetesClient, logger); + + // When + portForwardService.forwardPortAsync("foo-pod", "foo-ns", 8080, 312323); + + // Then + new Verifications() {{ + kubernetesClient.pods().inNamespace("foo-ns").withName("foo-pod").portForward(8080, 312323); + times = 1; + }}; + } +} \ No newline at end of file diff --git a/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServiceTest.java b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServiceTest.java index f90fa3d572..5b1a886a41 100644 --- a/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServiceTest.java +++ b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/PortForwardServiceTest.java @@ -82,8 +82,8 @@ public void testSimpleScenario() throws Exception { OpenShiftClient client = mockServer.getOpenshiftClient(); PortForwardService service = new PortForwardService(client, logger) { @Override - public LocalPortForward forwardPortAsync(String podName, String namespace, int remotePort, int localPort) { - return client.pods().inNamespace(namespace).withName(podName).portForward(localPort, remotePort); + public LocalPortForward forwardPortAsync(String podName, String namespace, int containerPort, int localPort) { + return client.pods().inNamespace(namespace).withName(podName).portForward(containerPort, localPort); } }; diff --git a/jkube-kit/jkube-kit-spring-boot/src/main/java/org/eclipse/jkube/springboot/watcher/SpringBootWatcher.java b/jkube-kit/jkube-kit-spring-boot/src/main/java/org/eclipse/jkube/springboot/watcher/SpringBootWatcher.java index c4ce82c903..043cc3198d 100644 --- a/jkube-kit/jkube-kit-spring-boot/src/main/java/org/eclipse/jkube/springboot/watcher/SpringBootWatcher.java +++ b/jkube-kit/jkube-kit-spring-boot/src/main/java/org/eclipse/jkube/springboot/watcher/SpringBootWatcher.java @@ -110,7 +110,7 @@ public void watch(List configs, Collection reso } } - private String getPortForwardUrl(final Collection resources) { + String getPortForwardUrl(final Collection resources) { LabelSelector selector = KubernetesHelper.extractPodLabelSelector(resources); if (selector == null) { log.warn("Unable to determine a selector for application pods"); @@ -122,11 +122,11 @@ private String getPortForwardUrl(final Collection resources) { SpringBootConfigurationHelper propertyHelper = new SpringBootConfigurationHelper( SpringBootUtil.getSpringBootVersion(getContext().getBuildContext().getProject())); - int port = IoUtil.getFreeRandomPort(); + int localHostPort = IoUtil.getFreeRandomPort(); int containerPort = propertyHelper.getServerPort(properties); - portForwardService.forwardPortAsync(selector, containerPort, port); + portForwardService.forwardPortAsync(selector, containerPort, localHostPort); - return createForwardUrl(propertyHelper, properties, port); + return createForwardUrl(propertyHelper, properties, localHostPort); } private String createForwardUrl(SpringBootConfigurationHelper propertyHelper, Properties properties, int localPort) { diff --git a/jkube-kit/jkube-kit-spring-boot/src/test/java/org/eclipse/jkube/springboot/watcher/SpringBootWatcherTest.java b/jkube-kit/jkube-kit-spring-boot/src/test/java/org/eclipse/jkube/springboot/watcher/SpringBootWatcherTest.java new file mode 100644 index 0000000000..f897298d27 --- /dev/null +++ b/jkube-kit/jkube-kit-spring-boot/src/test/java/org/eclipse/jkube/springboot/watcher/SpringBootWatcherTest.java @@ -0,0 +1,103 @@ +/** + * Copyright (c) 2019 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 org.eclipse.jkube.springboot.watcher; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.LabelSelector; +import io.fabric8.kubernetes.api.model.ServiceBuilder; +import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; +import mockit.Expectations; +import mockit.Mocked; +import mockit.Verifications; +import org.eclipse.jkube.kit.common.JKubeConfiguration; +import org.eclipse.jkube.kit.common.JavaProject; +import org.eclipse.jkube.kit.common.util.KubernetesHelper; +import org.eclipse.jkube.kit.common.util.SpringBootUtil; +import org.eclipse.jkube.kit.config.service.PortForwardService; +import org.eclipse.jkube.watcher.api.WatcherContext; +import org.junit.Test; + +import java.net.URLClassLoader; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Properties; + +import static org.junit.Assert.assertTrue; + +@SuppressWarnings({"ResultOfMethodCallIgnored", "AccessStaticViaInstance", "unused"}) +public class SpringBootWatcherTest { + @Mocked + private PortForwardService portForwardService; + + @Mocked + private WatcherContext watcherContext; + + @Mocked + private JKubeConfiguration jkubeConfiguration; + + @Mocked + private JavaProject javaProject; + + @Mocked + private KubernetesHelper kubernetesHelper; + + @Mocked + private SpringBootUtil springBootUtil; + + @Test + public void testGetPortForwardUrl() { + // Given + List resources = new ArrayList<>(); + resources.add(new DeploymentBuilder().withNewMetadata().withName("d1").endMetadata() + .withNewSpec() + .withNewSelector() + .withMatchLabels(Collections.singletonMap("foo", "bar")) + .endSelector() + .endSpec() + .build()); + resources.add(new ServiceBuilder().withNewMetadata().withName("s1").endMetadata().build()); + new Expectations() {{ + watcherContext.getBuildContext(); + result = jkubeConfiguration; + + jkubeConfiguration.getProject(); + result = javaProject; + + javaProject.getProperties(); + result = new Properties(); + + javaProject.getCompileClassPathElements(); + result = Collections.singletonList("/foo"); + + javaProject.getOutputDirectory().getAbsolutePath(); + result = "target/classes"; + + Properties properties = new Properties(); + properties.put("server.port", "9001"); + springBootUtil.getSpringBootApplicationProperties((URLClassLoader) any); + result = properties; + }}; + SpringBootWatcher springBootWatcher = new SpringBootWatcher(watcherContext); + + // When + String portForwardUrl = springBootWatcher.getPortForwardUrl(resources); + + // Then + assertTrue(portForwardUrl.contains("http://localhost:")); + new Verifications() {{ + portForwardService.forwardPortAsync((LabelSelector) any, 9001, anyInt); + }}; + } +}