diff --git a/CHANGELOG.md b/CHANGELOG.md index 53a03f065aa..a9770c1aca4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ #### Bugs * Fix #2748: Pass custom headers in kubernetes-client to watch api by modify WatchConnectionManager * Fix #2745: Filtering Operations can't configure PropagationPolicy +* Fix #2672: WaitUntilReady for Service resource throws IllegalArgumentException #### Improvements * Fix #2717: Remove edit() methods from RawCustomResourceOperationsImpl taking InputStream arguments diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/internal/readiness/Readiness.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/internal/readiness/Readiness.java index 4a271e0310b..347c97f4d24 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/internal/readiness/Readiness.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/internal/readiness/Readiness.java @@ -35,13 +35,19 @@ import io.fabric8.kubernetes.api.model.apps.StatefulSetSpec; import io.fabric8.kubernetes.api.model.apps.StatefulSetStatus; import io.fabric8.kubernetes.client.utils.Utils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; public class Readiness { private static final String POD_READY = "Ready"; private static final String NODE_READY = "Ready"; private static final String TRUE = "True"; - + private static final Logger logger = LoggerFactory.getLogger(Readiness.class); public static boolean isReadinessApplicable(Class itemClass) { return Deployment.class.isAssignableFrom(itemClass) @@ -59,10 +65,30 @@ public static boolean isReady(HasMetadata item) { if (isReadiableKubernetesResource(item)) { return isKubernetesResourceReady(item); } else { - throw new IllegalArgumentException("Item needs to be one of [Node, Deployment, ReplicaSet, StatefulSet, Pod, ReplicationController], but was: [" + (item != null ? item.getKind() : "Unknown (null)") + "]"); + return handleNonReadinessApplicableResource(item, getReadinessApplicableResourcesList()); } } + public static boolean handleNonReadinessApplicableResource(HasMetadata item, List readinessApplicableResources) { + boolean doesItemExist = Objects.nonNull(item); + String readinessApplicableResourcesStr = String.join(",", readinessApplicableResources); + logger.warn("{} is not a Readiableresource. It needs to be one of [{}]", + doesItemExist ? item.getKind() : "Unknown", readinessApplicableResourcesStr); + return doesItemExist; + } + + public static List getReadinessApplicableResourcesList() { + List readinessApplicableResources = new ArrayList<>(); + readinessApplicableResources.add("Node"); + readinessApplicableResources.add("Deployment"); + readinessApplicableResources.add("ReplicaSet"); + readinessApplicableResources.add("StatefulSet"); + readinessApplicableResources.add("Pod"); + readinessApplicableResources.add("ReplicationController"); + + return readinessApplicableResources; + } + private static boolean isKubernetesResourceReady(HasMetadata item) { if (item instanceof Deployment) { return isDeploymentReady((Deployment) item); diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/internal/readiness/ReadinessTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/internal/readiness/ReadinessTest.java index f28de336511..18914a9f6d7 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/internal/readiness/ReadinessTest.java +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/internal/readiness/ReadinessTest.java @@ -15,6 +15,8 @@ */ package io.fabric8.kubernetes.client.internal.readiness; +import io.fabric8.kubernetes.api.model.Service; +import io.fabric8.kubernetes.api.model.ServiceBuilder; import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.fabric8.kubernetes.api.model.apps.StatefulSetSpec; import io.fabric8.kubernetes.api.model.apps.StatefulSetStatus; @@ -94,4 +96,14 @@ public void testStatefulSetReadiness() { assertTrue(Readiness.isReady(statefulSet)); assertTrue(Readiness.isStatefulSetReady(statefulSet)); } + + @Test + void testReadinessWithNonNullResource() { + assertTrue(Readiness.isReady(new ServiceBuilder().withNewMetadata().withName("svc1").endMetadata().build())); + } + + @Test + void testReadinessNullResource() { + assertFalse(Readiness.isReady(null)); + } } diff --git a/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ServiceIT.java b/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ServiceIT.java index 963d609d5a1..5828372a978 100644 --- a/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ServiceIT.java +++ b/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ServiceIT.java @@ -26,7 +26,6 @@ import org.arquillian.cube.kubernetes.api.Session; import org.arquillian.cube.kubernetes.impl.requirement.RequiresKubernetes; import org.arquillian.cube.requirement.ArquillianConditionalRunner; -import org.assertj.core.internal.bytebuddy.build.ToStringPlugin; import org.jboss.arquillian.test.api.ArquillianResource; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -225,6 +224,22 @@ public void testExternalNameServiceCreateOrReplace() { assertEquals("his.database.example.com", service.getSpec().getExternalName()); } + @Test + public void waitUntilReady() throws InterruptedException { + // Given + String svcName = "service-wait-until-ready"; + + // When + Service service = client.services().inNamespace(session.getNamespace()) + .withName(svcName) + .waitUntilReady(10, TimeUnit.SECONDS); + + // Then + assertNotNull(service); + assertEquals(svcName, service.getMetadata().getName()); + assertTrue(client.pods().inNamespace(session.getNamespace()).withName(svcName).delete()); + } + @AfterClass public static void cleanup() { ClusterEntity.remove(ServiceIT.class.getResourceAsStream("/service-it.yml")); diff --git a/kubernetes-itests/src/test/resources/service-it.yml b/kubernetes-itests/src/test/resources/service-it.yml index a26c34ffd8f..75fb182ad65 100644 --- a/kubernetes-itests/src/test/resources/service-it.yml +++ b/kubernetes-itests/src/test/resources/service-it.yml @@ -95,3 +95,32 @@ spec: protocol: TCP port: 443 targetPort: 9377 +--- +apiVersion: v1 +kind: Service +metadata: + name: service-wait-until-ready +spec: + selector: + app: service-wait-until-ready + ports: + - name: http + protocol: TCP + port: 8080 + targetPort: 8080 +--- +apiVersion: v1 +kind: Pod +metadata: + name: service-wait-until-ready + labels: + app: service-wait-until-ready +spec: + containers: + - name: busybox + image: busybox + command: ["sleep", "36000"] + ports: + - containerPort: 8080 + name: http + protocol: TCP diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java index 5e50e12f79e..d15d4ea923c 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java @@ -15,6 +15,10 @@ */ package io.fabric8.kubernetes.client.mock; +import io.fabric8.kubernetes.api.model.EndpointAddressBuilder; +import io.fabric8.kubernetes.api.model.EndpointPortBuilder; +import io.fabric8.kubernetes.api.model.EndpointSubsetBuilder; +import io.fabric8.kubernetes.api.model.EndpointsBuilder; import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceBuilder; @@ -29,6 +33,7 @@ import java.net.HttpURLConnection; import java.util.Collections; +import java.util.concurrent.TimeUnit; import static junit.framework.TestCase.assertNotNull; import static junit.framework.TestCase.assertTrue; @@ -41,6 +46,8 @@ class ServiceTest { Service service; + private KubernetesClient client; + @BeforeEach void prepareService() { service = new ServiceBuilder() @@ -57,11 +64,11 @@ void prepareService() { .addToSelector("deploymentconfig", "httpbin") .endSpec() .build(); + client = server.getClient(); } @Test void testLoad() { - KubernetesClient client = server.getClient(); Service svc = client.services().load(getClass().getResourceAsStream("/test-service.yml")).get(); assertNotNull(svc); assertEquals("httpbin", svc.getMetadata().getName()); @@ -74,7 +81,6 @@ void testCreate() { .andReturn(200, service) .once(); - KubernetesClient client = server.getClient(); Service responseSvc = client.services().inNamespace("test").create(service); assertNotNull(responseSvc); assertEquals("httpbin", responseSvc.getMetadata().getName()); @@ -104,7 +110,6 @@ void testReplace() throws InterruptedException { .andReturn(HttpURLConnection.HTTP_OK, serviceUpdated) .once(); - KubernetesClient client = server.getClient(); Service responseSvc = client.services().inNamespace("test").createOrReplace(serviceUpdated); assertNotNull(responseSvc); assertEquals("httpbin", responseSvc.getMetadata().getName()); @@ -122,7 +127,6 @@ void testDelete() { .andReturn(200, service) .once(); - KubernetesClient client = server.getClient(); boolean isDeleted = client.services().inNamespace("test").withName("httpbin").delete(); assertTrue(isDeleted); } @@ -142,7 +146,6 @@ void testUpdate() { .andReturn(200, serviceFromServer) .once(); - KubernetesClient client = server.getClient(); Service responseFromServer = client.services().inNamespace("test").withName("httpbin").edit(s -> new ServiceBuilder(s) .editOrNewMetadata().addToLabels("foo", "bar").endMetadata() .build()); @@ -171,7 +174,6 @@ void testGetUrlFromClusterIPService() { .andReturn(HttpURLConnection.HTTP_OK, service).always(); server.expect().get().withPath("/apis/extensions/v1beta1/namespaces/ns1/ingresses") .andReturn(HttpURLConnection.HTTP_OK, new IngressListBuilder().build()).always(); - KubernetesClient client = server.getClient(); // When String url = client.services().inNamespace("ns1").withName("svc1").getURL("http"); @@ -179,4 +181,31 @@ void testGetUrlFromClusterIPService() { // Then assertEquals("tcp://187.30.14.32:8080", url); } + + @Test + void testWaitUntilReady() throws InterruptedException { + // Given + Service svc1 = new ServiceBuilder().withNewMetadata().withName("svc1").endMetadata().build(); + server.expect().get().withPath("/api/v1/namespaces/ns1/endpoints/svc1") + .andReturn(HttpURLConnection.HTTP_OK, new EndpointsBuilder() + .withNewMetadata().withName("svc1").endMetadata() + .addToSubsets(new EndpointSubsetBuilder() + .addToAddresses(new EndpointAddressBuilder().withIp("192.168.64.13").build()) + .addToPorts(new EndpointPortBuilder().withPort(8443).build()) + .build()) + .build()) + .once(); + server.expect().get().withPath("/api/v1/namespaces/ns1/services/svc1") + .andReturn(HttpURLConnection.HTTP_OK, svc1) + .times(2); + + // When + Service service = client.services() + .inNamespace("ns1") + .withName("svc1") + .waitUntilReady(60, TimeUnit.SECONDS); + + // Then + assertNotNull(service); + } } diff --git a/openshift-client/src/main/java/io/fabric8/openshift/client/internal/readiness/OpenShiftReadiness.java b/openshift-client/src/main/java/io/fabric8/openshift/client/internal/readiness/OpenShiftReadiness.java index 3e1b58b65f6..aab412cb583 100644 --- a/openshift-client/src/main/java/io/fabric8/openshift/client/internal/readiness/OpenShiftReadiness.java +++ b/openshift-client/src/main/java/io/fabric8/openshift/client/internal/readiness/OpenShiftReadiness.java @@ -22,6 +22,9 @@ import io.fabric8.openshift.api.model.DeploymentConfigSpec; import io.fabric8.openshift.api.model.DeploymentConfigStatus; +import java.util.ArrayList; +import java.util.List; + public class OpenShiftReadiness extends Readiness { public static boolean isReadinessApplicable(Class itemClass) { return Readiness.isReadinessApplicable(itemClass) @@ -35,7 +38,7 @@ public static boolean isReady(HasMetadata item) { } else if (item instanceof DeploymentConfig) { return isDeploymentConfigReady((DeploymentConfig) item); } else { - throw new IllegalArgumentException("Item needs to be one of [Node, Deployment, ReplicaSet, StatefulSet, Pod, DeploymentConfig, ReplicationController], but was: [" + (item != null ? item.getKind() : "Unknown (null)") + "]"); + return handleNonReadinessApplicableResource(item, getOpenShiftReadinessApplicableResourcesList()); } } @@ -56,4 +59,11 @@ public static boolean isDeploymentConfigReady(DeploymentConfig d) { return spec.getReplicas().intValue() == status.getReplicas() && spec.getReplicas() <= status.getAvailableReplicas(); } + + private static List getOpenShiftReadinessApplicableResourcesList() { + List openShiftReadinessApplicableResources = new ArrayList<>(Readiness.getReadinessApplicableResourcesList()); + openShiftReadinessApplicableResources.add("DeploymentConfig"); + + return openShiftReadinessApplicableResources; + } } diff --git a/openshift-client/src/test/java/io/fabric8/openshift/client/internal/readiness/OpenShiftReadinessTest.java b/openshift-client/src/test/java/io/fabric8/openshift/client/internal/readiness/OpenShiftReadinessTest.java new file mode 100644 index 00000000000..f3ff3c35881 --- /dev/null +++ b/openshift-client/src/test/java/io/fabric8/openshift/client/internal/readiness/OpenShiftReadinessTest.java @@ -0,0 +1,46 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.openshift.client.internal.readiness; + +import io.fabric8.openshift.api.model.DeploymentConfig; +import io.fabric8.openshift.api.model.DeploymentConfigBuilder; +import io.fabric8.openshift.api.model.ImageStreamBuilder; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class OpenShiftReadinessTest { + @Test + void testOpenShiftReadinessWithDeploymentConfig() { + DeploymentConfig dc = new DeploymentConfigBuilder() + .withNewSpec().withReplicas(2).endSpec() + .withNewStatus().withAvailableReplicas(2).withReplicas(2).endStatus() + .build(); + + assertTrue(OpenShiftReadiness.isReady(dc)); + } + + @Test + void testOpenShiftReadinessWithNonReadinessApplicableResource() { + assertTrue(OpenShiftReadiness.isReady(new ImageStreamBuilder().withNewMetadata().withName("is1").endMetadata().build())); + } + + @Test + void testOpenShiftReadinessWithNullResource() { + assertFalse(OpenShiftReadiness.isReady(null)); + } +}