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

deps: bump kubernetes-client to 6.4.0 #1988

Merged
merged 1 commit into from
Jan 25, 2023
+47 −73
Merged

deps: bump kubernetes-client to 6.4.0 #1988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 22 additions & 39 deletions .../config/resource/src/test/java/org/eclipse/jkube/kit/config/access/ClusterAccessTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,101 +13,84 @@
*/
package org.eclipse.jkube.kit.config.access;

import java.net.UnknownHostException;
import java.util.function.Consumer;

import org.eclipse.jkube.kit.common.KitLogger;

import io.fabric8.kubernetes.client.DefaultKubernetesClient;
import io.fabric8.kubernetes.api.model.APIGroupListBuilder;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient;
import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer;
import io.fabric8.openshift.client.OpenShiftClient;
import org.junit.jupiter.api.AfterEach;
import org.eclipse.jkube.kit.common.KitLogger;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.MockedConstruction;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.contains;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.ArgumentMatchers.startsWith;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockConstruction;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@EnableKubernetesMockClient(https = false)
class ClusterAccessTest {

private KitLogger logger;
private MockedConstruction<DefaultKubernetesClient> kubernetesClientMockedConstruction;
private Consumer<OpenShiftClient> onInstantiate;
private KubernetesMockServer mockServer;
private ClusterConfiguration clusterConfiguration;

@BeforeEach
void setUp() {
logger = spy(new KitLogger.SilentLogger());
onInstantiate = oc -> {
};
kubernetesClientMockedConstruction = mockConstruction(DefaultKubernetesClient.class, (mock, ctx) -> {
final OpenShiftClient oc = mock(OpenShiftClient.class);
when(mock.adapt(OpenShiftClient.class)).thenReturn(oc);
onInstantiate.accept(oc);
});
}

@AfterEach
void tearDown() {
kubernetesClientMockedConstruction.close();
clusterConfiguration = ClusterConfiguration.builder()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may be good to explain in the PR description why these changes were made and any reference to a kubernetes-client PR/issue

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are just to improve the test and make use of the Kubernetes Mock Server provided by the client instead of relying on complex Mocks.

Why now? The Kubernetes Mock server as become more stable and performant over the last couple of years. Being able to test ClusterAccess against a semi-real cluster is easier to maintain than relying on Mocks that need extra-maintenance. In this case, the Client is slowly moving to abstract away explicit KubernetesClient implementations and moving on to SPI provided and more generic ones instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is good for me, ideally in the description or in another PR

.masterUrl(mockServer.url("/"))
.build();
}

@Test
void isOpenShiftOpenShiftClusterShouldReturnTrue() {
// Given
onInstantiate = oc -> when(oc.isSupported()).thenReturn(true);
mockServer.expect().get().withPath("/apis").andReturn(200,
new APIGroupListBuilder().addNewGroup().withName("project.openshift.io").withApiVersion("v1").endGroup().build()).once();
// When
final boolean result = new ClusterAccess(logger, null).isOpenShift();
final boolean result = new ClusterAccess(logger, clusterConfiguration).isOpenShift();
// Then
assertThat(result).isTrue();
}

@Test
void isOpenShiftKubernetesClusterShouldReturnFalse() {
// Given
onInstantiate = oc -> when(oc.isSupported()).thenReturn(false);
// When
final boolean result = new ClusterAccess(logger, null).isOpenShift();
final boolean result = new ClusterAccess(logger, clusterConfiguration).isOpenShift();
// Then
assertThat(result).isFalse();
}

@Test
void isOpenShiftThrowsExceptionShouldReturnFalse() {
// Given
onInstantiate = oc -> when(oc.isSupported())
.thenThrow(new KubernetesClientException("ERROR", new UnknownHostException()));
clusterConfiguration = ClusterConfiguration.builder().masterUrl("https://unknown.example.com").build();
// When
final boolean result = new ClusterAccess(logger, null).isOpenShift();
final boolean result = new ClusterAccess(logger, clusterConfiguration).isOpenShift();
// Then
assertThat(result).isFalse();
verify(logger, times(1))
.warn(startsWith("Cannot access cluster for detecting mode"), eq("Unknown host "), isNull());
.warn(startsWith("Cannot access cluster for detecting mode"), eq(""), contains("unknown.example.com"));
}

@Test
void createDefaultClientInKubernetesShouldReturnKubernetesClient() {
// When
final KubernetesClient result = new ClusterAccess(logger, null).createDefaultClient();
final KubernetesClient result = new ClusterAccess(logger, clusterConfiguration).createDefaultClient();
// Then
assertThat(result).isNotNull().isNotInstanceOf(OpenShiftClient.class);
}

@Test
void createDefaultClientInOpenShiftShouldReturnOpenShiftClient() {
// Given
onInstantiate = oc -> when(oc.isSupported()).thenReturn(true);
mockServer.expect().get().withPath("/apis").andReturn(200,
new APIGroupListBuilder().addNewGroup().withName("project.openshift.io").withApiVersion("v1").endGroup().build()).once();
// When
final KubernetesClient result = new ClusterAccess(logger, null).createDefaultClient();
final KubernetesClient result = new ClusterAccess(logger, clusterConfiguration).createDefaultClient();
// Then
assertThat(result).isNotNull().isInstanceOf(OpenShiftClient.class);
}
Expand Down
4 changes: 2 additions & 2 deletions ...t/config/service/src/test/java/org/eclipse/jkube/kit/config/service/ApplyServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,15 +304,15 @@ void processCustomEntitiesRecreateModeTrue() throws Exception {
.addToResources(virtualServiceResource(), gatewayResource()).build()))
.times(2);
mockServer.expect().get()
.withPath("/apis/networking.istio.io/v1alpha3/namespaces/default/gateways?fieldSelector=metadata.name%3Dmygateway-https")
.withPath("/apis/networking.istio.io/v1alpha3/namespaces/default/gateways?fieldSelector=metadata.name%3Dmygateway-https&resourceVersion=0")
.andReturn(HTTP_OK, new GenericKubernetesResourceBuilder()
.withApiVersion("networking.istio.io/v1alpha3")
.withKind("Gateway")
.withNewMetadata().withName("mygateway-https").endMetadata()
.build())
.once();
mockServer.expect().get()
.withPath("/apis/networking.istio.io/v1alpha3/namespaces/default/virtualservices?fieldSelector=metadata.name%3Dreviews-route")
.withPath("/apis/networking.istio.io/v1alpha3/namespaces/default/virtualservices?fieldSelector=metadata.name%3Dreviews-route&resourceVersion=0")
.andReturn(HTTP_OK, new GenericKubernetesResourceBuilder()
.withApiVersion("networking.istio.io/v1alpha3")
.withKind("VirtualService")
Expand Down
Loading