Skip to content

Commit 47f4fe1

Browse files
wind57wind57
authored
Cleanup fabric8 loadbalancer mapper (#1605)
1 parent cc43ce5 commit 47f4fe1

File tree

5 files changed

+67
-44
lines changed

5 files changed

+67
-44
lines changed

‎spring-cloud-kubernetes-fabric8-autoconfig/src/main/java/org/springframework/cloud/kubernetes/fabric8/Fabric8Utils.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,16 @@
1616

1717
package org.springframework.cloud.kubernetes.fabric8;
1818

19+
import io.fabric8.kubernetes.api.model.ObjectMeta;
20+
import io.fabric8.kubernetes.api.model.Service;
21+
import io.fabric8.kubernetes.api.model.ServiceSpec;
1922
import io.fabric8.kubernetes.client.KubernetesClient;
2023
import jakarta.annotation.Nullable;
2124
import org.apache.commons.logging.LogFactory;
2225

2326
import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider;
2427
import org.springframework.cloud.kubernetes.commons.config.NamespaceResolutionFailedException;
28+
import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadata;
2529
import org.springframework.core.log.LogAccessor;
2630
import org.springframework.util.StringUtils;
2731

@@ -37,6 +41,13 @@ private Fabric8Utils() {
3741

3842
}
3943

44+
public static ServiceMetadata serviceMetadata(Service service) {
45+
ObjectMeta metadata = service.getMetadata();
46+
ServiceSpec serviceSpec = service.getSpec();
47+
return new ServiceMetadata(metadata.getName(), metadata.getNamespace(), serviceSpec.getType(),
48+
metadata.getLabels(), metadata.getAnnotations());
49+
}
50+
4051
private static final LogAccessor LOG = new LogAccessor(LogFactory.getLog(Fabric8Utils.class));
4152

4253
/**

‎spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,8 @@
2929
import io.fabric8.kubernetes.api.model.EndpointSubset;
3030
import io.fabric8.kubernetes.api.model.Endpoints;
3131
import io.fabric8.kubernetes.api.model.EndpointsList;
32-
import io.fabric8.kubernetes.api.model.ObjectMeta;
3332
import io.fabric8.kubernetes.api.model.Service;
3433
import io.fabric8.kubernetes.api.model.ServiceList;
35-
import io.fabric8.kubernetes.api.model.ServiceSpec;
3634
import io.fabric8.kubernetes.client.KubernetesClient;
3735
import io.fabric8.kubernetes.client.dsl.FilterNested;
3836
import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable;
@@ -43,7 +41,6 @@
4341

4442
import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider;
4543
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
46-
import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadata;
4744
import org.springframework.cloud.kubernetes.fabric8.Fabric8Utils;
4845
import org.springframework.core.log.LogAccessor;
4946
import org.springframework.util.CollectionUtils;
@@ -198,13 +195,6 @@ static Map<String, Integer> endpointSubsetsPortData(List<EndpointSubset> endpoin
198195
EndpointPort::getPort));
199196
}
200197

201-
static ServiceMetadata serviceMetadata(Service service) {
202-
ObjectMeta metadata = service.getMetadata();
203-
ServiceSpec serviceSpec = service.getSpec();
204-
return new ServiceMetadata(metadata.getName(), metadata.getNamespace(), serviceSpec.getType(),
205-
metadata.getLabels(), metadata.getAnnotations());
206-
}
207-
208198
/**
209199
* serviceName can be null, in which case, such a filter will not be applied.
210200
*/

‎spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,12 @@
4444
import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.serviceInstance;
4545
import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.serviceInstanceMetadata;
4646
import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.EXTERNAL_NAME;
47+
import static org.springframework.cloud.kubernetes.fabric8.Fabric8Utils.serviceMetadata;
4748
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8InstanceIdHostPodNameSupplier.externalName;
4849
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8InstanceIdHostPodNameSupplier.nonExternalName;
4950
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.addresses;
5051
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.endpointSubsetsPortData;
5152
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.endpoints;
52-
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.serviceMetadata;
5353
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.services;
5454
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8PodLabelsAndAnnotationsSupplier.externalName;
5555
import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8PodLabelsAndAnnotationsSupplier.nonExternalName;

‎spring-cloud-kubernetes-fabric8-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapper.java

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.cloud.kubernetes.fabric8.loadbalancer;
1818

19-
import java.util.HashMap;
2019
import java.util.List;
2120
import java.util.Map;
2221
import java.util.Optional;
@@ -27,10 +26,13 @@
2726
import io.fabric8.kubernetes.client.utils.Utils;
2827

2928
import org.springframework.cloud.kubernetes.commons.discovery.DefaultKubernetesServiceInstance;
29+
import org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils;
3030
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
3131
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesServiceInstance;
32+
import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadata;
3233
import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesLoadBalancerProperties;
3334
import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesServiceInstanceMapper;
35+
import org.springframework.cloud.kubernetes.fabric8.Fabric8Utils;
3436

3537
/**
3638
* Class for mapping Kubernetes Service object into {@link KubernetesServiceInstance}.
@@ -39,6 +41,11 @@
3941
*/
4042
public class Fabric8ServiceInstanceMapper implements KubernetesServiceInstanceMapper<Service> {
4143

44+
/**
45+
* empty on purpose, load balancer implementation does not need them.
46+
*/
47+
private static final Map<String, Integer> PORTS_DATA = Map.of();
48+
4249
private final KubernetesLoadBalancerProperties properties;
4350

4451
private final KubernetesDiscoveryProperties discoveryProperties;
@@ -57,7 +64,7 @@ public KubernetesServiceInstance map(Service service) {
5764
if (ports.size() == 1) {
5865
port = ports.get(0);
5966
}
60-
else if (ports.size() > 1 && Utils.isNotNullOrEmpty(this.properties.getPortName())) {
67+
else if (ports.size() > 1 && Utils.isNotNullOrEmpty(properties.getPortName())) {
6168
Optional<ServicePort> optPort = ports.stream().filter(it -> properties.getPortName().endsWith(it.getName()))
6269
.findAny();
6370
if (optPort.isPresent()) {
@@ -72,24 +79,12 @@ else if (ports.size() > 1 && Utils.isNotNullOrEmpty(this.properties.getPortName(
7279
boolean secure = KubernetesServiceInstanceMapper.isSecure(service.getMetadata().getLabels(),
7380
service.getMetadata().getAnnotations(), port.getName(), port.getPort());
7481
return new DefaultKubernetesServiceInstance(meta.getUid(), meta.getName(), host, port.getPort(),
75-
getServiceMetadata(service), secure);
82+
serviceMetadata(service), secure);
7683
}
7784

78-
private Map<String, String> getServiceMetadata(Service service) {
79-
Map<String, String> serviceMetadata = new HashMap<>();
80-
KubernetesDiscoveryProperties.Metadata metadataProps = this.discoveryProperties.metadata();
81-
if (metadataProps.addLabels()) {
82-
Map<String, String> labelMetadata = KubernetesServiceInstanceMapper
83-
.getMapWithPrefixedKeys(service.getMetadata().getLabels(), metadataProps.labelsPrefix());
84-
serviceMetadata.putAll(labelMetadata);
85-
}
86-
if (metadataProps.addAnnotations()) {
87-
Map<String, String> annotationMetadata = KubernetesServiceInstanceMapper
88-
.getMapWithPrefixedKeys(service.getMetadata().getAnnotations(), metadataProps.annotationsPrefix());
89-
serviceMetadata.putAll(annotationMetadata);
90-
}
91-
92-
return serviceMetadata;
85+
Map<String, String> serviceMetadata(Service service) {
86+
ServiceMetadata serviceMetadata = Fabric8Utils.serviceMetadata(service);
87+
return DiscoveryClientUtils.serviceInstanceMetadata(PORTS_DATA, serviceMetadata, discoveryProperties);
9388
}
9489

9590
}

‎spring-cloud-kubernetes-fabric8-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapperTests.java

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2023 the original author or authors.
2+
* Copyright 2013-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -38,7 +38,7 @@ class Fabric8ServiceInstanceMapperTests {
3838
@Test
3939
void testMapperSimple() {
4040
KubernetesLoadBalancerProperties properties = new KubernetesLoadBalancerProperties();
41-
Service service = buildService("test", "abc", 8080, null, Map.of());
41+
Service service = buildService("test", "test-namespace", "abc", 8080, null, Map.of());
4242
KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties,
4343
KubernetesDiscoveryProperties.DEFAULT).map(service);
4444
Assertions.assertNotNull(instance);
@@ -53,7 +53,7 @@ void testMapperMultiplePorts() {
5353
List<ServicePort> ports = new ArrayList<>();
5454
ports.add(new ServicePortBuilder().withPort(8080).withName("web").build());
5555
ports.add(new ServicePortBuilder().withPort(9000).withName("http").build());
56-
Service service = buildService("test", "abc", ports, Map.of());
56+
Service service = buildService("test", "test-namespace", "abc", ports, Map.of());
5757
KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties,
5858
KubernetesDiscoveryProperties.DEFAULT).map(service);
5959
Assertions.assertNotNull(instance);
@@ -65,7 +65,7 @@ void testMapperMultiplePorts() {
6565
@Test
6666
void testMapperSecure() {
6767
KubernetesLoadBalancerProperties properties = new KubernetesLoadBalancerProperties();
68-
Service service = buildService("test", "abc", 443, null, Map.of());
68+
Service service = buildService("test", "test-namespace", "abc", 443, null, Map.of());
6969
KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties,
7070
KubernetesDiscoveryProperties.DEFAULT).map(service);
7171
Assertions.assertNotNull(instance);
@@ -82,7 +82,7 @@ void testMapperSecureNullLabelsAndAnnotations() {
8282
false);
8383
List<ServicePort> ports = new ArrayList<>();
8484
ports.add(new ServicePortBuilder().withPort(443).build());
85-
Service service = buildService("test", "abc", ports, null, null);
85+
Service service = buildService("test", "test-namespace", "abc", ports, null, null);
8686
KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties, discoveryProperties)
8787
.map(service);
8888
Assertions.assertNotNull(instance);
@@ -95,29 +95,56 @@ void testMapperSecureNullLabelsAndAnnotations() {
9595
void testMapperSecureWithLabels() {
9696
KubernetesLoadBalancerProperties properties = new KubernetesLoadBalancerProperties();
9797
Map<String, String> labels = Map.of("secured", "true", "label1", "123");
98-
Service service = buildService("test", "abc", 8080, null, labels);
98+
Service service = buildService("test", "test-namespace", "abc", 8080, null, labels);
9999
KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties,
100100
KubernetesDiscoveryProperties.DEFAULT).map(service);
101101
Assertions.assertNotNull(instance);
102102
Assertions.assertEquals("test", instance.getServiceId());
103103
Assertions.assertEquals("abc", instance.getInstanceId());
104104
Assertions.assertTrue(instance.isSecure());
105-
Assertions.assertEquals(2, instance.getMetadata().keySet().size());
105+
Assertions.assertEquals(4, instance.getMetadata().keySet().size());
106106
}
107107

108-
private Service buildService(String name, String uid, int port, String portName, Map<String, String> labels) {
108+
@Test
109+
void serviceMetadataTest() {
110+
111+
KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties();
112+
KubernetesDiscoveryProperties discoveryProperties = new KubernetesDiscoveryProperties(true, false, Set.of(),
113+
true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0,
114+
true);
115+
116+
List<ServicePort> ports = new ArrayList<>();
117+
ports.add(new ServicePortBuilder().withPort(443).build());
118+
119+
Map<String, String> labels = Map.of("one", "1");
120+
Map<String, String> annotations = Map.of("two", "2");
121+
122+
Service service = buildService("test", "test-namespace", "abc", ports, labels, annotations);
123+
Map<String, String> result = new Fabric8ServiceInstanceMapper(loadBalancerProperties, discoveryProperties)
124+
.serviceMetadata(service);
125+
Assertions.assertEquals(result.size(), 4);
126+
Assertions.assertEquals(result.get("k8s_namespace"), "test-namespace");
127+
Assertions.assertEquals(result.get("type"), "ClusterIP");
128+
Assertions.assertEquals(result.get("one"), "1");
129+
Assertions.assertEquals(result.get("two"), "2");
130+
}
131+
132+
private Service buildService(String name, String namespace, String uid, int port, String portName,
133+
Map<String, String> labels) {
109134
ServicePort servicePort = new ServicePortBuilder().withPort(port).withName(portName).build();
110-
return buildService(name, uid, Collections.singletonList(servicePort), labels);
135+
return buildService(name, namespace, uid, Collections.singletonList(servicePort), labels);
111136
}
112137

113-
private Service buildService(String name, String uid, List<ServicePort> ports, Map<String, String> labels,
114-
Map<String, String> annotations) {
115-
return new ServiceBuilder().withNewMetadata().withName(name).withUid(uid).addToLabels(labels)
116-
.withAnnotations(annotations).endMetadata().withNewSpec().addAllToPorts(ports).endSpec().build();
138+
private Service buildService(String name, String namespace, String uid, List<ServicePort> ports,
139+
Map<String, String> labels, Map<String, String> annotations) {
140+
return new ServiceBuilder().withNewMetadata().withNamespace(namespace).withName(name).withUid(uid)
141+
.addToLabels(labels).withAnnotations(annotations).endMetadata().withNewSpec().addAllToPorts(ports)
142+
.withType("ClusterIP").endSpec().build();
117143
}
118144

119-
private Service buildService(String name, String uid, List<ServicePort> ports, Map<String, String> labels) {
120-
return buildService(name, uid, ports, labels, Map.of());
145+
private Service buildService(String name, String namespace, String uid, List<ServicePort> ports,
146+
Map<String, String> labels) {
147+
return buildService(name, namespace, uid, ports, labels, Map.of());
121148
}
122149

123150
}

0 commit comments

Comments
 (0)