Skip to content

Commit

Permalink
Fix fabric8io#2399: Cannot change the type of the Service from Cluste…
Browse files Browse the repository at this point in the history 
…rIP to ExternalName
  • Loading branch information
@rohanKanojia @manusa
rohanKanojia authored and manusa committed Sep 16, 2020
1 parent 6b1af63 commit 03f942a
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -5,6 +5,7 @@
#### Bugs
* Fix: #2442: Wrong resource kind in `ProjectRequestHandler` causes ClassCastException when handling Project resources.
* Fix #2467: OpenShiftClient cannot replace existing resource with API version =! v1
* Fix #2399: Cannot change the type of the Service from ClusterIP to ExternalName

#### Improvements
* Fix #2473: Removed unused ValidationMessages.properties
Expand Down
56 changes: 36 additions & 20 deletions ...rc/main/java/io/fabric8/kubernetes/client/dsl/internal/core/v1/ServiceOperationsImpl.java
View file
Expand Up @@ -36,6 +36,8 @@

public class ServiceOperationsImpl extends HasMetadataOperation<Service, ServiceList, DoneableService, ServiceResource<Service, DoneableService>> implements ServiceResource<Service, DoneableService> {

public static final String EXTERNAL_NAME = "ExternalName";

public ServiceOperationsImpl(OkHttpClient client, Config config) {
this(client, config, null);
}
Expand All @@ -58,30 +60,21 @@ public ServiceOperationsImpl newInstance(OperationContext context) {

@Override
public Service replace(Service item) {
try {
Service old = fromServer().get();
return super.replace(new ServiceBuilder(item)
.editSpec()
.withClusterIP(old.getSpec().getClusterIP())
.endSpec()
.build());
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType("replace"), e);
}
return super.replace(patchClusterIpIntoServiceAndReplace(item));
}

@Override
public Service patch(Service item) {
try {
Service old = getMandatory();
return super.patch(new ServiceBuilder(item)
.editSpec()
.withClusterIP(old.getSpec().getClusterIP())
.endSpec()
.build());
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType("patch"), e);
}
try {
Service old = getMandatory();
return super.patch(new ServiceBuilder(item)
.editSpec()
.withClusterIP(old.getSpec().getClusterIP())
.endSpec()
.build());
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType("patch"), e);
}
}

@Override
Expand Down Expand Up @@ -161,4 +154,27 @@ public int compare(ServiceToURLProvider first, ServiceToURLProvider second) {
return first.getPriority() - second.getPriority();
}
}

private Service patchClusterIpIntoServiceAndReplace(Service item) {
if (!isExternalNameService(item)) {
try {
Service old = fromServer().get();
return new ServiceBuilder(item)
.editSpec()
.withClusterIP(old.getSpec().getClusterIP())
.endSpec()
.build();
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType("replace"), e);
}
}
return item;
}

private boolean isExternalNameService(Service item) {
if (item != null && item.getSpec() != null && item.getSpec().getType() != null) {
return item.getSpec().getType().equals(EXTERNAL_NAME);
}
return false;
}
}
131 changes: 131 additions & 0 deletions kubernetes-itests/src/test/java/io/fabric8/kubernetes/ServiceIT.java
View file
Expand Up @@ -20,11 +20,13 @@
import io.fabric8.commons.ReadyEntity;
import io.fabric8.kubernetes.api.model.IntOrString;
import io.fabric8.kubernetes.api.model.Service;
import io.fabric8.kubernetes.api.model.ServiceBuilder;
import io.fabric8.kubernetes.api.model.ServiceList;
import io.fabric8.kubernetes.client.KubernetesClient;
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;
Expand Down Expand Up @@ -94,6 +96,135 @@ public void delete() {
assertTrue(bDeleted);
}

@Test
public void testChangeServiceType() {
// Given
Service svc = client.services().inNamespace(session.getNamespace()).withName("service-change-service-type").get();

// When
svc.getSpec().setType("ExternalName");
svc.getSpec().setExternalName("my.database.example.com");
svc.getSpec().setClusterIP("");
svc = client.services().inNamespace(session.getNamespace()).createOrReplace(svc);

// Then
assertNotNull(svc);
assertEquals("ExternalName", svc.getSpec().getType());
assertEquals("my.database.example.com", svc.getSpec().getExternalName());
}

@Test
public void testClusterIPCreateOrReplace() {
// Given
Service clusterIPSvc = new ServiceBuilder()
.withNewMetadata().withName("serviceit-clusterip-createorreplace").endMetadata()
.withNewSpec()
.addToSelector("app", "myapp")
.addNewPort()
.withName("http")
.withProtocol("TCP")
.withPort(80)
.withTargetPort(new IntOrString(9376))
.endPort()
.endSpec()
.build();

// When
// Create resource
client.services().inNamespace(session.getNamespace()).createOrReplace(clusterIPSvc);
// Modify resource
clusterIPSvc.getSpec().getPorts().get(0).setTargetPort(new IntOrString(9380));
// Do createOrReplace again; resource should get updated
clusterIPSvc = client.services().inNamespace(session.getNamespace()).createOrReplace(clusterIPSvc);

// Then
assertNotNull(clusterIPSvc);
assertEquals("ClusterIP", clusterIPSvc.getSpec().getType());
assertEquals(9380, clusterIPSvc.getSpec().getPorts().get(0).getTargetPort().getIntVal().intValue());
}

@Test
public void testNodePortCreateOrReplace() {
// Given
Service clusterIPSvc = new ServiceBuilder()
.withNewMetadata().withName("serviceit-nodeport-createorreplace").endMetadata()
.withNewSpec()
.withType("NodePort")
.addToSelector("app", "myapp")
.addNewPort()
.withPort(80)
.withTargetPort(new IntOrString(80))
.endPort()
.endSpec()
.build();

// When
// Create resource
client.services().inNamespace(session.getNamespace()).createOrReplace(clusterIPSvc);
// Modify resource
clusterIPSvc.getSpec().getPorts().get(0).setTargetPort(new IntOrString(81));
// Do createOrReplace again; resource should get updated
clusterIPSvc = client.services().inNamespace(session.getNamespace()).createOrReplace(clusterIPSvc);

// Then
assertNotNull(clusterIPSvc);
assertEquals("NodePort", clusterIPSvc.getSpec().getType());
assertEquals(81, clusterIPSvc.getSpec().getPorts().get(0).getTargetPort().getIntVal().intValue());
}

@Test
public void testLoadBalancerCreateOrReplace() {
// Given
Service clusterIPSvc = new ServiceBuilder()
.withNewMetadata().withName("serviceit-loadbalancer-createorreplace").endMetadata()
.withNewSpec()
.withType("LoadBalancer")
.addToSelector("app", "myapp")
.addNewPort()
.withProtocol("TCP")
.withPort(80)
.withTargetPort(new IntOrString(9376))
.endPort()
.endSpec()
.build();

// When
// Create resource
client.services().inNamespace(session.getNamespace()).createOrReplace(clusterIPSvc);
// Modify resource
clusterIPSvc.getSpec().getPorts().get(0).setTargetPort(new IntOrString(9380));
// Do createOrReplace again; resource should get updated
clusterIPSvc = client.services().inNamespace(session.getNamespace()).createOrReplace(clusterIPSvc);

// Then
assertNotNull(clusterIPSvc);
assertEquals("LoadBalancer", clusterIPSvc.getSpec().getType());
assertEquals(9380, clusterIPSvc.getSpec().getPorts().get(0).getTargetPort().getIntVal().intValue());
}

@Test
public void testExternalNameCreateOrReplace() {
// Given
Service service = new ServiceBuilder()
.withNewMetadata().withName("serviceit-externalname-createorreplace").endMetadata()
.withNewSpec()
.withType("ExternalName")
.withExternalName("my.database.example.com")
.endSpec()
.build();

// When
client.services().inNamespace(session.getNamespace()).createOrReplace(service);
service.getSpec().setExternalName("his.database.example.com");
service = client.services().inNamespace(session.getNamespace()).createOrReplace(service);

// Then
assertNotNull(service);
assertEquals("serviceit-externalname-createorreplace", service.getMetadata().getName());
assertEquals("ExternalName", service.getSpec().getType());
assertEquals("his.database.example.com", service.getSpec().getExternalName());
}

@AfterClass
public static void cleanup() {
ClusterEntity.remove(ServiceIT.class.getResourceAsStream("/service-it.yml"));
Expand Down
17 changes: 17 additions & 0 deletions kubernetes-itests/src/test/resources/service-it.yml
View file
Expand Up @@ -78,3 +78,20 @@ spec:
protocol: TCP
port: 443
targetPort: 9377
---
apiVersion: v1
kind: Service
metadata:
name: service-change-service-type
spec:
selector:
app: MyApp
ports:
- name: http
protocol: TCP
port: 80
targetPort: 9376
- name: https
protocol: TCP
port: 443
targetPort: 9377

0 comments on commit 03f942a

Please sign in to comment.