move ports metadata computation to the dedicated method #1231
Conversation
| @@ -39,14 +40,14 @@ private Fabric8Utils() { | |||
| private static final LogAccessor LOG = new LogAccessor(LogFactory.getLog(Fabric8Utils.class)); | |||
|
|
|||
| /** | |||
| * this method does the namespace resolution for both config map and secrets | |||
There was a problem hiding this comment.
I forgot to update this documentation a while ago, as it is re-used in other places as well, and will be used in discovery also at some point in time, its on my TODO list
| return findEndPointsFilteredByNamespaces(serviceId); | ||
| } | ||
|
|
||
| private List<Endpoints> findEndPointsFilteredByNamespaces(String serviceId) { |
There was a problem hiding this comment.
first, I inlined findEndPointsFilteredByNamespaces into getEndPointsList, so the code became a bit simpler to read:
public List<Endpoints> getEndPointsList(String serviceId) {
if (properties.allNamespaces()) {
LOG.debug(() -> "searching for endpoints in all namespaces");
return client.endpoints().inAnyNamespace().withField("metadata.name", serviceId)
.withLabels(properties.serviceLabels()).list().getItems();
}
else if (properties.namespaces().isEmpty()) {
LOG.debug(() -> "searching for endpoints in namespace : " + client.getNamespace());
return client.endpoints().withField("metadata.name", serviceId).withLabels(properties.serviceLabels())
.list().getItems();
}
else {
LOG.debug(() -> "searching for endpoints in namespaces : " + properties.namespaces());
List<Endpoints> endpoints = new ArrayList<>();
for (String namespace : properties.namespaces()) {
endpoints.addAll(client.endpoints().inNamespace(namespace).withField("metadata.name", serviceId)
.withLabels(properties.serviceLabels()).list().getItems());
}
return endpoints;
}
}
Then I noticed that we re-use this withField("metadata.name", serviceId) .withLabels(properties.serviceLabels()).list().getItems() in all 3 branches, so I had to find a way to extract it. I re-used an addition in the fabric8 client called FilterNested, basically you can do:
FilterNested<FilterWatchListDeletable<Endpoints, EndpointsList, Resource<Endpoints>>> filterNested = client.endpoints().inAnyNamespace().withNewFilter();
and then chain the filters you want. So I created a static method:
static List<Endpoints> endpoints(
FilterNested<FilterWatchListDeletable<Endpoints, EndpointsList, Resource<Endpoints>>> filterNested,
KubernetesDiscoveryProperties properties, String serviceId) {
return filterNested.withField("metadata.name", serviceId).withLabels(properties.serviceLabels()).endFilter()
.list().getItems();
}
in KubernetesDiscoveryClientUtils and refactored the code to:
public List<Endpoints> getEndPointsList(String serviceId) {
if (properties.allNamespaces()) {
LOG.debug(() -> "searching for endpoints in all namespaces");
return endpoints(client.endpoints().inAnyNamespace().withNewFilter(), properties, serviceId);
}
else if (properties.namespaces().isEmpty()) {
LOG.debug(() -> "searching for endpoints in namespace : " + client.getNamespace());
return endpoints(client.endpoints().withNewFilter(), properties, serviceId);
}
else {
LOG.debug(() -> "searching for endpoints in namespaces : " + properties.namespaces());
List<Endpoints> endpoints = new ArrayList<>();
for (String namespace : properties.namespaces()) {
endpoints.addAll(
endpoints(client.endpoints().inNamespace(namespace).withNewFilter(), properties, serviceId));
}
return endpoints;
}
}
I've also adjusted some failing tests and added some more.
| @@ -138,6 +142,16 @@ static Map<String, String> serviceMetadata(String serviceId, Service service, | |||
| serviceMetadata.putAll(annotationMetadata); | |||
| } | |||
|
|
|||
| if (metadataProps.addPorts()) { | |||
There was a problem hiding this comment.
add port metadata to be computed here. We have a single method now responsible for all service metadata.
| List<EndpointSubset> subsets = es.endpointSubset(); | ||
| if (subsets.isEmpty()) { |
There was a problem hiding this comment.
slightly invert the logic here, if subsets is empty, return early.
| for (EndpointSubset s : subsets) { | ||
| // Extend the service metadata map with per-endpoint port information (if | ||
| // requested) | ||
| Map<String, String> endpointMetadata = new HashMap<>(serviceMetadata); |
There was a problem hiding this comment.
port metadata has moved to the method that does all the service's metadata computation. imho, this is a lot cleaner, as there is a single entry point where metadata is computed. The rest of the changes is just formatting, because of the dropped if statement (the one that I inverted).
wind57
changed the title
|
@ryanjbaxter ready to be looked at. thank you. |
No description provided.