Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 488 on 2.x #499

Merged
merged 3 commits into from
Feb 21, 2023
Merged

Fix 488 on 2.x #499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6ebed4
remove schema from url for service instance host, use 0 as default po…
aureamunoz Feb 17, 2023
7c3d39e
refactor cdi tests accordingly
aureamunoz Feb 17, 2023
c9de336
review: add log message and use -1 in absence of port
aureamunoz Feb 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion service-discovery/knative/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>knative-client</artifactId>
<!-- <scope>test</scope>-->
</dependency>
<dependency>
<groupId>io.vertx</groupId>
Expand Down
15 changes: 14 additions & 1 deletion ...ive/src/main/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscovery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.smallrye.stork.servicediscovery.knative;

import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -147,9 +148,21 @@ private List<ServiceInstance> toStorkServiceInstances(List<Service> knServices,
: Collections.emptyMap());

Metadata<KnativeMetadataKey> knativeMetadata = Metadata.of(KnativeMetadataKey.class);
String host = knService.getStatus().getUrl();
try {
URI uri = new URI(knService.getStatus().getUrl());
if (uri != null && uri.getScheme() != null) {
host = uri.getHost();
if (host == null) { // invalid URI
throw new IllegalArgumentException("Invalid URL used: '" + uri + "'");
}
}
} catch (Exception e) {
LOGGER.error(e.getMessage() + " for service: " + application);
}

serviceInstances
.add(new DefaultServiceInstance(ServiceInstanceIds.next(), knService.getStatus().getUrl(), 8080, secure,
.add(new DefaultServiceInstance(ServiceInstanceIds.next(), host, -1, secure,
labels,
knativeMetadata
.with(KnativeMetadataKey.META_KNATIVE_SERVICE_ID, knService.getFullResourceName())
Expand Down
30 changes: 15 additions & 15 deletions .../test/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscoveryCDITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ void shouldDiscoverNamespacedKnativeServices() {
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(-1);
Map<String, String> labels = instances.get().get(0).getLabels();
assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
entry("serving.knative.dev/lastModifier", "kubernetes-admin"));
Expand Down Expand Up @@ -123,8 +123,8 @@ void shouldDiscoverKnativeServicesInAllNs() {

String knativeService = "my-knservice";

registerKnativeServices(knativeService, "http://hello.ns1.127.0.0.1.sslip.io", "ns1");
registerKnativeServices(knativeService, "http://hello.ns2.127.0.0.1.sslip.io", "ns2");
registerKnativeServices(knativeService, "hello.ns1.127.0.0.1.sslip.io", "ns1");
registerKnativeServices(knativeService, "hello.ns2.127.0.0.1.sslip.io", "ns2");

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -138,7 +138,7 @@ void shouldDiscoverKnativeServicesInAllNs() {

assertThat(instances.get()).hasSize(2);
assertThat(instances.get().stream().map(ServiceInstance::getHost))
.containsExactlyInAnyOrder("http://hello.ns1.127.0.0.1.sslip.io", "http://hello.ns2.127.0.0.1.sslip.io");
.containsExactlyInAnyOrder("hello.ns1.127.0.0.1.sslip.io", "hello.ns2.127.0.0.1.sslip.io");
}

@Test
Expand All @@ -149,7 +149,7 @@ void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {

String knativeService = "my-knservice";

registerKnativeServices(knativeService, "http://hello.test.127.0.0.1.sslip.io", null);
registerKnativeServices(knativeService, "hello.test.127.0.0.1.sslip.io", null);

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -162,8 +162,8 @@ void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(-1);
Map<String, String> labels = instances.get().get(0).getLabels();
assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
entry("serving.knative.dev/lastModifier", "kubernetes-admin"));
Expand All @@ -179,7 +179,7 @@ void shouldHandleSecureAttribute() {

String knSvcName = "my-knservice";

registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null);
registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null);

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -192,8 +192,8 @@ void shouldHandleSecureAttribute() {
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(-1);
Map<String, String> labels = instances.get().get(0).getLabels();
assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
entry("serving.knative.dev/lastModifier", "kubernetes-admin"));
Expand All @@ -218,7 +218,7 @@ void shouldFetchInstancesFromTheClusterWhenCacheIsInvalidated() {

String knSvcName = "my-knservice";

registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null);
registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null);

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -231,8 +231,8 @@ void shouldFetchInstancesFromTheClusterWhenCacheIsInvalidated() {
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(-1);
Map<String, String> labels = instances.get().get(0).getLabels();
assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
entry("serving.knative.dev/lastModifier", "kubernetes-admin"));
Expand Down Expand Up @@ -266,7 +266,7 @@ void shouldFetchInstancesFromTheCache() throws InterruptedException {
server.expect().get().withPath("/apis/serving.knative.dev/v1/namespaces/test/services/my-knservice")
.andReply(200, r -> {
serverHit.incrementAndGet();
return buildKnService(knSvcName, "http://hello.test.127.0.0.1.sslip.io", "test");
return buildKnService(knSvcName, "hello.test.127.0.0.1.sslip.io", "test");
}).always();

config.addServiceConfig("my-knservice", null, "knative",
Expand Down
96 changes: 96 additions & 0 deletions ...va/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscoveryRealClusterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package io.smallrye.stork.servicediscovery.knative;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.awaitility.Awaitility.await;

import java.time.Duration;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import io.fabric8.kubernetes.client.Config;
import io.smallrye.stork.Stork;
import io.smallrye.stork.api.ServiceInstance;
import io.smallrye.stork.test.StorkTestUtils;
import io.smallrye.stork.test.TestConfigProvider;

@Disabled
public class KnativeServiceDiscoveryRealClusterTest {

String k8sMasterUrl;
String namespace;

@BeforeEach
void setUp() {
TestConfigProvider.clear();
System.setProperty(Config.KUBERNETES_TRUST_CERT_SYSTEM_PROPERTY, "true");
}

@Test
void shouldDiscoverHeroesKnative() {
String svc = "hero-service";

TestConfigProvider.addServiceConfig(svc, null, "knative",
null, Map.of("knative-host", "https://api.sandbox-m2.ll9k.p1.openshiftapps.com:6443", "knative-namespace",
"amunozhe-dev", "application", "rest-heroes"));

Stork stork = StorkTestUtils.getNewStorkInstance();

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

io.smallrye.stork.api.Service service = stork.getService(svc);
service.getServiceDiscovery().getServiceInstances()
.onFailure().invoke(th -> fail("Failed to get service instances from the cluster", th))
.subscribe().with(instances::set);

await().atMost(Duration.ofSeconds(40))
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
ServiceInstance svcInstance = instances.get().get(0);

// This code needs the smallrye-mutiny-vertx-web-client dependency
// WebClient client = WebClient.create(Vertx.vertx(), new WebClientOptions()
// .setDefaultHost(svcInstance.getHost()).setDefaultPort(svcInstance.getPort()).setSsl(false).setTrustAll(false));
//
// await().untilAsserted(() -> Assertions.assertEquals(200,
// client.get("/api/heroes/random").sendAndAwait().statusCode()));

}

// @Test
void shouldDiscoverNamespacedKnativeServicesWithApp() {
String svc = "my-service";

TestConfigProvider.addServiceConfig("my-service", null, "knative",
null, Map.of("knative-namespace", "default", "application", "helloworld-go"));

Stork stork = StorkTestUtils.getNewStorkInstance();

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

io.smallrye.stork.api.Service service = stork.getService(svc);
service.getServiceDiscovery().getServiceInstances()
.onFailure().invoke(th -> fail("Failed to get service instances from the cluster", th))
.subscribe().with(instances::set);

await().atMost(Duration.ofSeconds(5))
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
ServiceInstance svcInstance = instances.get().get(0);

// WebClient client = WebClient.create(Vertx.vertx(), new WebClientOptions()
// .setDefaultHost(svcInstance.getHost()).setSsl(false).setTrustAll(false));
//
// await().untilAsserted(() -> Assertions.assertEquals(200,
// client.get("").sendAndAwait().statusCode()));

}

}
32 changes: 16 additions & 16 deletions ...src/test/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscoveryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void shouldDiscoverNamespacedKnativeServices() {

String knSvcName = "my-knservice";

registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null);
registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null);

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -70,8 +70,8 @@ void shouldDiscoverNamespacedKnativeServices() {
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(-1);
Map<String, String> labels = instances.get().get(0).getLabels();
assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
entry("serving.knative.dev/lastModifier", "kubernetes-admin"));
Expand Down Expand Up @@ -109,8 +109,8 @@ void shouldDiscoverKnativeServicesInAllNs() {

String knativeService = "my-knservice";

registerKnativeServices(knativeService, "http://hello.ns1.127.0.0.1.sslip.io", "ns1");
registerKnativeServices(knativeService, "http://hello.ns2.127.0.0.1.sslip.io", "ns2");
registerKnativeServices(knativeService, "hello.ns1.127.0.0.1.sslip.io", "ns1");
registerKnativeServices(knativeService, "hello.ns2.127.0.0.1.sslip.io", "ns2");

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -124,7 +124,7 @@ void shouldDiscoverKnativeServicesInAllNs() {

assertThat(instances.get()).hasSize(2);
assertThat(instances.get().stream().map(ServiceInstance::getHost))
.containsExactlyInAnyOrder("http://hello.ns1.127.0.0.1.sslip.io", "http://hello.ns2.127.0.0.1.sslip.io");
.containsExactlyInAnyOrder("hello.ns1.127.0.0.1.sslip.io", "hello.ns2.127.0.0.1.sslip.io");
}

@Test
Expand All @@ -135,7 +135,7 @@ void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {

String knativeService = "my-knservice";

registerKnativeServices(knativeService, "http://hello.test.127.0.0.1.sslip.io", null);
registerKnativeServices(knativeService, "hello.test.127.0.0.1.sslip.io", null);

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -148,8 +148,8 @@ void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(-1);
Map<String, String> labels = instances.get().get(0).getLabels();
assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
entry("serving.knative.dev/lastModifier", "kubernetes-admin"));
Expand All @@ -165,7 +165,7 @@ void shouldHandleSecureAttribute() {

String knSvcName = "my-knservice";

registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null);
registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null);

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -178,8 +178,8 @@ void shouldHandleSecureAttribute() {
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(-1);
Map<String, String> labels = instances.get().get(0).getLabels();
assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
entry("serving.knative.dev/lastModifier", "kubernetes-admin"));
Expand All @@ -204,7 +204,7 @@ void shouldFetchInstancesFromTheClusterWhenCacheIsInvalidated() {

String knSvcName = "my-knservice";

registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null);
registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null);

AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();

Expand All @@ -217,8 +217,8 @@ void shouldFetchInstancesFromTheClusterWhenCacheIsInvalidated() {
.until(() -> instances.get() != null);

assertThat(instances.get()).hasSize(1);
assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
assertThat(instances.get().get(0).getPort()).isEqualTo(-1);
Map<String, String> labels = instances.get().get(0).getLabels();
assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
entry("serving.knative.dev/lastModifier", "kubernetes-admin"));
Expand Down Expand Up @@ -252,7 +252,7 @@ void shouldFetchInstancesFromTheCache() throws InterruptedException {
server.expect().get().withPath("/apis/serving.knative.dev/v1/namespaces/test/services/my-knservice")
.andReply(200, r -> {
serverHit.incrementAndGet();
return buildKnService(knSvcName, "http://hello.test.127.0.0.1.sslip.io", "test");
return buildKnService(knSvcName, "hello.test.127.0.0.1.sslip.io", "test");
}).always();

TestConfigProvider.addServiceConfig("my-knservice", null, "knative",
Expand Down
4 changes: 1 addition & 3 deletions ...o/smallrye/stork/servicediscovery/kubernetes/KubernetesServiceDiscoveryRealClusterIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import io.smallrye.stork.test.StorkTestUtils;
import io.smallrye.stork.test.TestConfigProvider;

@Disabled
public class KubernetesServiceDiscoveryRealClusterIT {

@BeforeEach
Expand All @@ -34,7 +35,6 @@ void setUp() {
}

@Test
@Disabled
void shouldGetServiceFromK8sDefaultNamespace() {

TestConfigProvider.addServiceConfig("rest-service", null, "kubernetes",
Expand All @@ -60,7 +60,6 @@ void shouldGetServiceFromK8sDefaultNamespace() {
}

@Test
@Disabled
void shouldGetServicesForDefaultNamespaceOnNonSpecified() {
String serviceName = "pod1";

Expand All @@ -82,7 +81,6 @@ void shouldGetServicesForDefaultNamespaceOnNonSpecified() {
}

@Test
@Disabled("doesn't work yet")
void shouldRegisterServiceInstancesInDefaultNamespace() throws InterruptedException {
TestConfigProvider.addServiceConfig("svc", null, "kubernetes",
null, Map.of("k8s-host", "https://127.0.0.1:41711/", "k8s-namespace", "stork"));
Expand Down
Loading