Skip to content

Commit d9bad61

Browse files
HaybuHaybu
authored
Merge pull request spring-cloud#466 from Haybu/bug/issue-465
service (for its metadata) should be queried per namespace. fixes spring-cloudgh-465
2 parents 88768aa + 987222e commit d9bad61

File tree

4 files changed

+149
-53
lines changed

4 files changed

+149
-53
lines changed

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright 2012-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cloud.kubernetes.discovery;
18+
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
22+
import io.fabric8.kubernetes.api.model.EndpointSubset;
23+
24+
/**
25+
* @author Haytham Mohamed
26+
**/
27+
public class EndpointSubsetNS {
28+
29+
private String namespace;
30+
31+
private List<EndpointSubset> endpointSubset;
32+
33+
public EndpointSubsetNS() {
34+
endpointSubset = new ArrayList<>();
35+
}
36+
37+
public String getNamespace() {
38+
return namespace;
39+
}
40+
41+
public void setNamespace(String namespace) {
42+
this.namespace = namespace;
43+
}
44+
45+
public List<EndpointSubset> getEndpointSubset() {
46+
return endpointSubset;
47+
}
48+
49+
public void setEndpointSubset(List<EndpointSubset> endpointSubset) {
50+
this.endpointSubset = endpointSubset;
51+
}
52+
53+
public boolean equals(Object o) {
54+
return this.endpointSubset.equals(o);
55+
}
56+
57+
public int hashCode() {
58+
return this.endpointSubset.hashCode();
59+
}
60+
61+
}

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

Lines changed: 55 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -101,40 +101,38 @@ public String description() {
101101
public List<ServiceInstance> getInstances(String serviceId) {
102102
Assert.notNull(serviceId,
103103
"[Assertion failed] - the object argument must not be null");
104+
104105
List<Endpoints> endpointsList = this.properties.isAllNamespaces()
105106
? this.client.endpoints().inAnyNamespace()
106107
.withField("metadata.name", serviceId).list().getItems()
107108
: Collections
108109
.singletonList(this.client.endpoints().withName(serviceId).get());
109-
List<EndpointSubset> subsets = endpointsList.stream()
110-
.flatMap(endpoints -> getSubsetsFromEndpoints(endpoints).stream())
110+
111+
List<EndpointSubsetNS> subsetsNS = endpointsList.stream()
112+
.map(endpoints -> getSubsetsFromEndpoints(endpoints))
111113
.collect(Collectors.toList());
114+
112115
List<ServiceInstance> instances = new ArrayList<>();
113-
if (!subsets.isEmpty()) {
116+
if (!subsetsNS.isEmpty()) {
117+
for (EndpointSubsetNS es : subsetsNS) {
118+
instances.addAll(this.getNamespaceServiceInstances(es, serviceId));
119+
}
120+
}
114121

115-
final Service service = this.client.services().withName(serviceId).get();
122+
return instances;
123+
}
116124

117-
final Map<String, String> serviceMetadata = new HashMap<>();
125+
private List<ServiceInstance> getNamespaceServiceInstances(EndpointSubsetNS es,
126+
String serviceId) {
127+
String namespace = es.getNamespace();
128+
List<EndpointSubset> subsets = es.getEndpointSubset();
129+
List<ServiceInstance> instances = new ArrayList<>();
130+
if (!subsets.isEmpty()) {
131+
final Service service = this.client.services().inNamespace(namespace)
132+
.withName(serviceId).get();
133+
final Map<String, String> serviceMetadata = this.getServiceMetadata(service);
118134
KubernetesDiscoveryProperties.Metadata metadataProps = this.properties
119135
.getMetadata();
120-
if (metadataProps.isAddLabels()) {
121-
Map<String, String> labelMetadata = getMapWithPrefixedKeys(
122-
service.getMetadata().getLabels(),
123-
metadataProps.getLabelsPrefix());
124-
if (log.isDebugEnabled()) {
125-
log.debug("Adding label metadata: " + labelMetadata);
126-
}
127-
serviceMetadata.putAll(labelMetadata);
128-
}
129-
if (metadataProps.isAddAnnotations()) {
130-
Map<String, String> annotationMetadata = getMapWithPrefixedKeys(
131-
service.getMetadata().getAnnotations(),
132-
metadataProps.getAnnotationsPrefix());
133-
if (log.isDebugEnabled()) {
134-
log.debug("Adding annotation metadata: " + annotationMetadata);
135-
}
136-
serviceMetadata.putAll(annotationMetadata);
137-
}
138136

139137
for (EndpointSubset s : subsets) {
140138
// Extend the service metadata map with per-endpoint port information (if
@@ -176,6 +174,31 @@ public List<ServiceInstance> getInstances(String serviceId) {
176174
return instances;
177175
}
178176

177+
private Map<String, String> getServiceMetadata(Service service) {
178+
final Map<String, String> serviceMetadata = new HashMap<>();
179+
KubernetesDiscoveryProperties.Metadata metadataProps = this.properties
180+
.getMetadata();
181+
if (metadataProps.isAddLabels()) {
182+
Map<String, String> labelMetadata = getMapWithPrefixedKeys(
183+
service.getMetadata().getLabels(), metadataProps.getLabelsPrefix());
184+
if (log.isDebugEnabled()) {
185+
log.debug("Adding label metadata: " + labelMetadata);
186+
}
187+
serviceMetadata.putAll(labelMetadata);
188+
}
189+
if (metadataProps.isAddAnnotations()) {
190+
Map<String, String> annotationMetadata = getMapWithPrefixedKeys(
191+
service.getMetadata().getAnnotations(),
192+
metadataProps.getAnnotationsPrefix());
193+
if (log.isDebugEnabled()) {
194+
log.debug("Adding annotation metadata: " + annotationMetadata);
195+
}
196+
serviceMetadata.putAll(annotationMetadata);
197+
}
198+
199+
return serviceMetadata;
200+
}
201+
179202
private EndpointPort findEndpointPort(EndpointSubset s) {
180203
List<EndpointPort> ports = s.getPorts();
181204
EndpointPort endpointPort;
@@ -197,15 +220,17 @@ private EndpointPort findEndpointPort(EndpointSubset s) {
197220
return endpointPort;
198221
}
199222

200-
private List<EndpointSubset> getSubsetsFromEndpoints(Endpoints endpoints) {
201-
if (endpoints == null) {
202-
return new ArrayList<>();
203-
}
204-
if (endpoints.getSubsets() == null) {
205-
return new ArrayList<>();
223+
private EndpointSubsetNS getSubsetsFromEndpoints(Endpoints endpoints) {
224+
EndpointSubsetNS es = new EndpointSubsetNS();
225+
es.setNamespace(this.client.getNamespace()); // start with the default that comes
226+
// with the client
227+
228+
if (endpoints != null && endpoints.getSubsets() != null) {
229+
es.setNamespace(endpoints.getMetadata().getNamespace());
230+
es.setEndpointSubset(endpoints.getSubsets());
206231
}
207232

208-
return endpoints.getSubsets();
233+
return es;
209234
}
210235

211236
// returns a new map that contain all the entries of the original map

‎spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import io.fabric8.kubernetes.api.model.Endpoints;
2828
import io.fabric8.kubernetes.api.model.EndpointsBuilder;
2929
import io.fabric8.kubernetes.api.model.EndpointsList;
30+
import io.fabric8.kubernetes.api.model.ObjectMeta;
3031
import io.fabric8.kubernetes.api.model.Service;
3132
import io.fabric8.kubernetes.api.model.ServiceBuilder;
3233
import io.fabric8.kubernetes.api.model.ServiceList;
@@ -48,6 +49,7 @@
4849
import static java.util.stream.Collectors.toList;
4950
import static org.assertj.core.api.Assertions.assertThat;
5051
import static org.assertj.core.api.Assertions.entry;
52+
import static org.mockito.ArgumentMatchers.anyString;
5153
import static org.mockito.Mockito.when;
5254

5355
@RunWith(MockitoJUnitRunner.class)
@@ -89,7 +91,7 @@ public void testAllExtraMetadataDisabled() {
8991
when(this.metadata.isAddAnnotations()).thenReturn(false);
9092
when(this.metadata.isAddPorts()).thenReturn(false);
9193

92-
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId,
94+
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId, "ns",
9395
new HashMap<String, String>() {
9496
{
9597
put("l1", "lab");
@@ -119,7 +121,7 @@ public void testLabelsEnabled() {
119121
when(this.metadata.isAddAnnotations()).thenReturn(false);
120122
when(this.metadata.isAddPorts()).thenReturn(false);
121123

122-
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId,
124+
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId, "ns",
123125
new HashMap<String, String>() {
124126
{
125127
put("l1", "v1");
@@ -152,7 +154,7 @@ public void testLabelsEnabledWithPrefix() {
152154
when(this.metadata.isAddAnnotations()).thenReturn(false);
153155
when(this.metadata.isAddPorts()).thenReturn(false);
154156

155-
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId,
157+
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId, "ns",
156158
new HashMap<String, String>() {
157159
{
158160
put("l1", "v1");
@@ -184,7 +186,7 @@ public void testAnnotationsEnabled() {
184186
when(this.metadata.isAddAnnotations()).thenReturn(true);
185187
when(this.metadata.isAddPorts()).thenReturn(false);
186188

187-
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId,
189+
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId, "ns",
188190
new HashMap<String, String>() {
189191
{
190192
put("l1", "v1");
@@ -217,7 +219,7 @@ public void testAnnotationsEnabledWithPrefix() {
217219
when(this.metadata.getAnnotationsPrefix()).thenReturn("a_");
218220
when(this.metadata.isAddPorts()).thenReturn(false);
219221

220-
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId,
222+
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId, "ns",
221223
new HashMap<String, String>() {
222224
{
223225
put("l1", "v1");
@@ -249,7 +251,7 @@ public void testPortsEnabled() {
249251
when(this.metadata.isAddAnnotations()).thenReturn(false);
250252
when(this.metadata.isAddPorts()).thenReturn(true);
251253

252-
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId,
254+
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId, "ns",
253255
new HashMap<String, String>() {
254256
{
255257
put("l1", "v1");
@@ -281,7 +283,7 @@ public void testPortsEnabledWithPrefix() {
281283
when(this.metadata.isAddPorts()).thenReturn(true);
282284
when(this.metadata.getPortsPrefix()).thenReturn("p_");
283285

284-
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId,
286+
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId, "ns",
285287
new HashMap<String, String>() {
286288
{
287289
put("l1", "v1");
@@ -315,7 +317,7 @@ public void testLabelsAndAnnotationsAndPortsEnabledWithPrefix() {
315317
when(this.metadata.isAddPorts()).thenReturn(true);
316318
when(this.metadata.getPortsPrefix()).thenReturn("p_");
317319

318-
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId,
320+
setupServiceWithLabelsAndAnnotationsAndPorts(serviceId, "ns",
319321
new HashMap<String, String>() {
320322
{
321323
put("l1", "la1");
@@ -339,18 +341,24 @@ public void testLabelsAndAnnotationsAndPortsEnabledWithPrefix() {
339341
}
340342

341343
private void setupServiceWithLabelsAndAnnotationsAndPorts(String serviceId,
342-
Map<String, String> labels, Map<String, String> annotations,
344+
String namespace, Map<String, String> labels, Map<String, String> annotations,
343345
Map<Integer, String> ports) {
344-
final Service service = new ServiceBuilder().withNewMetadata().withLabels(labels)
345-
.withAnnotations(annotations).endMetadata().withNewSpec()
346-
.withPorts(getServicePorts(ports)).endSpec().build();
346+
final Service service = new ServiceBuilder().withNewMetadata()
347+
.withNamespace(namespace).withLabels(labels).withAnnotations(annotations)
348+
.endMetadata().withNewSpec().withPorts(getServicePorts(ports)).endSpec()
349+
.build();
347350
when(this.serviceOperation.withName(serviceId)).thenReturn(this.serviceResource);
348351
when(this.serviceResource.get()).thenReturn(service);
349352
when(this.kubernetesClient.services()).thenReturn(this.serviceOperation);
353+
when(this.kubernetesClient.services().inNamespace(anyString()))
354+
.thenReturn(this.serviceOperation);
350355

351-
final Endpoints endpoints = new EndpointsBuilder().addNewSubset()
352-
.addAllToPorts(getEndpointPorts(ports)).addNewAddress().endAddress()
353-
.endSubset().build();
356+
ObjectMeta objectMeta = new ObjectMeta();
357+
objectMeta.setNamespace(namespace);
358+
359+
final Endpoints endpoints = new EndpointsBuilder().withMetadata(objectMeta)
360+
.addNewSubset().addAllToPorts(getEndpointPorts(ports)).addNewAddress()
361+
.endAddress().endSubset().build();
354362

355363
when(this.endpointsResource.get()).thenReturn(endpoints);
356364
when(this.endpointsOperation.withName(serviceId))

‎spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientTest.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,14 @@ public void getInstancesShouldBeAbleToHandleEndpointsFromMultipleNamespaces() {
116116
.andReturn(200, services).once();
117117

118118
mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint")
119-
.andReturn(200, service1).once();
119+
.andReturn(200, service1).always();
120120

121121
mockServer.expect().get().withPath("/api/v1/namespaces/test2/services/endpoint")
122-
.andReturn(200, service2).once();
122+
.andReturn(200, service2).always();
123123

124124
final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties();
125125
properties.setAllNamespaces(true);
126+
126127
final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient,
127128
properties, KubernetesClient::services,
128129
new DefaultIsServicePortSecureResolver(properties));
@@ -149,16 +150,17 @@ public void getInstancesShouldBeAbleToHandleEndpointsSingleAddress() {
149150
.endAddress().addNewPort("http", 80, "TCP").endSubset().build())
150151
.once();
151152

152-
mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint")
153+
mockServer.expect().get().withPath("/api/v1/services/endpoint")
153154
.andReturn(200, new ServiceBuilder().withNewMetadata()
154155
.withName("endpoint").withLabels(new HashMap<String, String>() {
155156
{
156157
put("l", "v");
157158
}
158159
}).endMetadata().build())
159-
.once();
160+
.always();
160161

161162
final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties();
163+
162164
final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient,
163165
properties, KubernetesClient::services,
164166
new DefaultIsServicePortSecureResolver(properties));
@@ -180,14 +182,14 @@ public void getInstancesShouldBeAbleToHandleEndpointsSingleAddressAndMultiplePor
180182
.addNewPort("http", 80, "TCP").endSubset().build())
181183
.once();
182184

183-
mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint")
185+
mockServer.expect().get().withPath("/api/v1/services/endpoint")
184186
.andReturn(200, new ServiceBuilder().withNewMetadata()
185187
.withName("endpoint").withLabels(new HashMap<String, String>() {
186188
{
187189
put("l", "v");
188190
}
189191
}).endMetadata().build())
190-
.once();
192+
.always();
191193

192194
final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties();
193195
properties.setPrimaryPortName("http");
@@ -212,14 +214,14 @@ public void getInstancesShouldBeAbleToHandleEndpointsMultipleAddresses() {
212214
.endAddress().addNewPort("https", 443, "TCP").endSubset().build())
213215
.once();
214216

215-
mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint")
217+
mockServer.expect().get().withPath("/api/v1/services/endpoint")
216218
.andReturn(200, new ServiceBuilder().withNewMetadata()
217219
.withName("endpoint").withLabels(new HashMap<String, String>() {
218220
{
219221
put("l", "v");
220222
}
221223
}).endMetadata().build())
222-
.once();
224+
.always();
223225

224226
final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties();
225227
final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient,

0 commit comments

Comments
 (0)