Skip to content

Commit

Permalink
Fix eclipse-jkube#653: k8s:watch port-forward websocket error due t…
Browse files Browse the repository at this point in the history
…o 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 <rohaan@redhat.com>
  • Loading branch information
rohanKanojia committed Jul 2, 2021
1 parent 288f55e commit e08809d
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -116,7 +116,7 @@ private void startPortForward(
if (firstSelector != null) {
Map<String, String> 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"));
}
}

Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -215,13 +220,13 @@ private Pod getNewestPod(List<Pod> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public void watch(List<ImageConfiguration> configs, Collection<HasMetadata> reso
}
}

private String getPortForwardUrl(final Collection<HasMetadata> resources) {
String getPortForwardUrl(final Collection<HasMetadata> resources) {
LabelSelector selector = KubernetesHelper.extractPodLabelSelector(resources);
if (selector == null) {
log.warn("Unable to determine a selector for application pods");
Expand All @@ -122,11 +122,11 @@ private String getPortForwardUrl(final Collection<HasMetadata> 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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<HasMetadata> 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);
}};
}
}

0 comments on commit e08809d

Please sign in to comment.