-
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
K8s client lb cleanup 1 #1625
K8s client lb cleanup 1 #1625
Conversation
Configure Renovate
@@ -122,7 +122,7 @@ Map<String, String> serviceMetadata(Service service) { | |||
return DiscoveryClientUtils.serviceInstanceMetadata(PORTS_DATA, serviceMetadata, discoveryProperties); | |||
} | |||
|
|||
boolean secure(ServicePort port, Service service) { | |||
private boolean secure(ServicePort port, Service service) { |
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.
make private
here as we do not use it outside anywhere else
@@ -59,136 +48,4 @@ void testCreateHostWithNullNamespace() { | |||
assertThat(host).isEqualTo("serviceName.default.svc.clusterDomain"); | |||
} | |||
|
|||
@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.
I am deleting these since we had code like this:
static Map<String, String> getMapWithPrefixedKeys(Map<String, String> map, String prefix) {
return ConfigUtils.keysWithPrefix(map, prefix);
}
so we simply delegated to ConfigUtils::keysWithPrefix
. I removed that method (it was not public
) and as such delete these tests also
@@ -44,41 +41,4 @@ static String createHost(String serviceName, String namespace, String clusterDom | |||
return new StringJoiner(".").add(serviceName).add(namespaceToUse).add("svc").add(clusterDomain).toString(); | |||
} | |||
|
|||
static boolean isSecure(Map<String, String> labels, Map<String, String> annotations, String servicePortName, |
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.
we do not need these methods anymore. isSecure
is not used, since we delegate the a common secure resolver now
@@ -16,12 +16,13 @@ | |||
|
|||
package org.springframework.cloud.kubernetes.client.loadbalancer; | |||
|
|||
import java.util.HashMap; |
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.
a little polish the tests and add one more
/* | ||
* Copyright 2013-2020 the original author or authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); |
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.
these two test classes removed are not needed anymore, since we have created an it
package, that already tests all these cases
serviceMetadata.putAll(annotationMetadata); | ||
} | ||
|
||
return serviceMetadata; | ||
} | ||
|
||
private boolean secure(V1ServicePort port, V1Service service) { |
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 like we do in fabric8, delegate to a common resolver to find if this service is secure or not
@ryanjbaxter I am replicating the changes we already made in fabric8 into k8s-native. A few PRs like this will follow. thank you |
No description provided.