-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move service instance to commons #1399
Move service instance to commons #1399
Conversation
@@ -150,116 +142,4 @@ void testBothAddressesTaken() { | |||
Assertions.assertEquals(hostNames, List.of("one", "three", "two")); | |||
} | |||
|
|||
@Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests were moved to commons, not deleted
@@ -130,6 +135,32 @@ public static ServicePortNameAndNumber endpointsPort(LinkedHashMap<String, Integ | |||
} | |||
} | |||
|
|||
public static ServiceInstance serviceInstance(@Nullable ServicePortSecureResolver servicePortSecureResolver, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method has changed from the way it used to exist in fabric8 implementation. For example it takes as input a Supplier<InstanceIdHostPodName>
, where:
/**
* computes instanceId, host and podName. All needed when calculating ServiceInstance.
* @author wind57
*/
public record InstanceIdHostPodName(String instanceId, String host, String podName) {
}
instanceId, host and podName
are computed based on specific fabric8 EndpointAddress/Service
:
final class Fabric8InstanceIdHostPodNameSupplier implements Supplier<InstanceIdHostPodName> {
private final EndpointAddress endpointAddress;
private final Service service;
Fabric8InstanceIdHostPodNameSupplier(EndpointAddress endpointAddress, Service service) {
this.endpointAddress = endpointAddress;
this.service = service;
}
@Override
public InstanceIdHostPodName get() {
return new InstanceIdHostPodName(instanceId(), host(), podName());
}
// instanceId is usually the pod-uid as seen in the .metadata.uid
private String instanceId() {
return Optional.ofNullable(endpointAddress).map(EndpointAddress::getTargetRef).map(ObjectReference::getUid)
.orElseGet(() -> service.getMetadata().getUid());
}
private String host() {
return Optional.ofNullable(endpointAddress).map(EndpointAddress::getIp)
.orElseGet(() -> service.getSpec().getExternalName());
}
private String podName() {
return Optional.ofNullable(endpointAddress).map(EndpointAddress::getTargetRef)
.filter(objectReference -> "Pod".equals(objectReference.getKind())).map(ObjectReference::getName)
.orElse(null);
}
}
The logic is exactly the same as it used to be, but a Supplier/Function
is needed so that I can pass into this method from commons both fabric8 and in the nearest future: k8s code.
Last PR before I start working on k8s integration with all these moved methods |
@@ -154,6 +185,32 @@ static String primaryPortName(KubernetesDiscoveryProperties properties, Map<Stri | |||
return primaryPortName; | |||
} | |||
|
|||
static Map<String, Map<String, String>> podMetadata(String podName, Map<String, String> serviceMetadata, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exactly the same logic is preserved, tests ensure this
@ryanjbaxter ready to be looked at. This is a bit more involved, but in essence it does not change anything "business related". It just makes sure that instead of passing fabric8 specific stuff to
it now expects a Another example would be that
from a
This is needed so that commons does not care what client we use, thus I can delegate to the same common code. |
No description provided.