-
Notifications
You must be signed in to change notification settings - Fork 0
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
Initial support for testing #9
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,7 @@ public class PodManager { | |
* @param keystorePath Path to the Java Keystore containing trusted certificates | ||
* @param maximumPodCount Maximum number of pods to ever have running at once | ||
* @param refreshIntervalInMs Interval with which to refresh the pods | ||
* @param podYamlPath the location of the yaml config for the application pod | ||
*/ | ||
public PodManager(KubernetesConfiguration k8sConfiguration, | ||
String keystorePath, | ||
|
@@ -343,7 +344,7 @@ else if (left.getLastRequest() < right.getLastRequest()) | |
* @throws IOException | ||
*/ | ||
private void refreshPods() throws IOException { | ||
HttpGet podsGet = new HttpGet("https://" + k8sConfiguration.getMasterIp() + ":443/api/v1/namespaces/" + k8sConfiguration.getNamespace() + "/pods"); | ||
HttpGet podsGet = new HttpGet(k8sConfiguration.getProtocol() + "://" + k8sConfiguration.getMasterIp() + "/api/v1/namespaces/" + k8sConfiguration.getNamespace() + "/pods"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could drop the protocol and master ip and just have a URL. master_url: https://k8s.example.com
# or
master_url: http://k8s.example.com I'm fairly certain I just went with master IP since that is what the GKE console displayed. It's fairly straightforward to reason out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like having an optional protocol for that reason. It's less to think about when you are configuring it. In almost all cases (except test) we can assume it's https. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to see the GKE dashboard again and verify they just surface up an IP. If we rename the parameter to |
||
|
||
try (CloseableHttpResponse response = httpClient.execute(podsGet, httpContext)) { | ||
HttpEntity entity = response.getEntity(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
package com.o19s.grandcentral.kubernetes; | ||
import com.github.tomakehurst.wiremock.junit.WireMockRule; | ||
import org.junit.After; | ||
import org.junit.Before; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
|
||
import static com.github.tomakehurst.wiremock.client.WireMock.*; | ||
|
||
; | ||
/** | ||
* Created by Omnifroodle on 5/17/16. | ||
*/ | ||
public class PodManagerTest { | ||
private KubernetesConfiguration kubecfg; | ||
@Rule | ||
public WireMockRule wireMockRule = new WireMockRule(8888); | ||
|
||
@Before | ||
public void setUp() throws Exception { | ||
this.kubecfg = new KubernetesConfiguration(); | ||
kubecfg.setMasterIp("127.0.0.1:8888"); | ||
kubecfg.setProtocol("http"); | ||
kubecfg.setNamespace("test"); | ||
kubecfg.setUsername("fred"); | ||
kubecfg.setPassword("flintstone"); | ||
} | ||
|
||
@After | ||
public void tearDown() throws Exception { | ||
} | ||
|
||
@Test | ||
public void testGet() throws Exception { | ||
|
||
} | ||
|
||
@Test | ||
public void testContains() throws Exception { | ||
|
||
} | ||
|
||
@Test | ||
public void testAdd() throws Exception { | ||
|
||
} | ||
|
||
@Test | ||
public void testRefreshPods() throws Exception { | ||
stubFor(get(urlEqualTo("/api/v1/namespaces/test/pods")) | ||
.willReturn(aResponse() | ||
.withStatus(200) | ||
.withHeader("Content-Type", "text/json") | ||
.withBody("{\"items\":[" + | ||
" {" + | ||
" \"metadata\":{" + | ||
" \"name\": \"abc\"" + | ||
" }," + | ||
" \"status\": {" + | ||
" \"phase\": \"Running\"," + | ||
" \"podIP\": \"1.1.1.1\"" + | ||
" }" + | ||
" }," + | ||
" {" + | ||
" \"metadata\":{" + | ||
" \"name\": \"abcd\"" + | ||
" }," + | ||
" \"status\": {" + | ||
" \"phase\": \"Running\"," + | ||
" \"podIP\": \"1.1.1.2\"" + | ||
" }" + | ||
" }" + | ||
" ]}"))); | ||
|
||
PodManager manager = new PodManager(this.kubecfg, "config/grandcentral.jks", 100, 1, "./config/configuration.yml"); | ||
} | ||
} |
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.
Why not put this version number with the others in
properties
?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.
no reason, I'll fix that.
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 go back and forth. If it’s version used by a number of jar files that are related, then I like using the interpolation. If it’s a single jar, then it’s yet another level of indirection.
Regardless, good to see…
Eric Pugh | Principal | OpenSource Connections, LLC | 434.466.1467 | http://www.opensourceconnections.com http://www.opensourceconnections.com/ | My Free/Busy http://tinyurl.com/eric-cal
Co-Author: Apache Solr Enterprise Search Server, 3rd Ed https://www.packtpub.com/big-data-and-business-intelligence/apache-solr-enterprise-search-server-third-edition-raw
This e-mail and all contents, including attachments, is considered to be Company Confidential unless explicitly stated otherwise, regardless of whether attachments are marked as such.
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 see your point on level of indirection, but it is convenient to have all version numbers together. If we include them with each dependency then you're crawling through each dependency's XML block a) looking for the right block and b) looking for the version element.
Fortunately our number of dependencies is small so it isn't that big of a task. I'm open for either approach. @epugh has a point.