-
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
feat(build): add clouddriver-integration module to exercise the just-built docker imageTest docker image #6206
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
b3dab39
fix(web): replace deprecated spring.profiles in configuration
dbyron-sf 995d61f
feat(docker): add HEALTHCHECK
dbyron-sf 769c526
feat(build): add clouddriver-integration module to exercise the just-…
dbyron-sf d9e0c12
feat(gha): run integration test in pr builds
dbyron-sf 9db127a
feat(gha): run integration test in branch builds
dbyron-sf ad6cf9a
fix(docker): reduce the chance for false positives in the health check
dbyron-sf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
dependencies { | ||
testImplementation "com.fasterxml.jackson.core:jackson-databind" | ||
testImplementation "org.assertj:assertj-core" | ||
testImplementation "org.junit.jupiter:junit-jupiter-api" | ||
testImplementation "org.slf4j:slf4j-api" | ||
testImplementation "org.testcontainers:testcontainers" | ||
testImplementation "org.testcontainers:junit-jupiter" | ||
testRuntimeOnly "ch.qos.logback:logback-classic" | ||
} | ||
|
||
test.configure { | ||
def fullDockerImageName = System.getenv('FULL_DOCKER_IMAGE_NAME') | ||
onlyIf("there is a docker image to test") { | ||
fullDockerImageName != null && fullDockerImageName.trim() != '' | ||
} | ||
} | ||
|
||
test { | ||
// So stdout and stderr from the just-built container are available in CI | ||
testLogging.showStandardStreams = true | ||
|
||
// Run the tests when the docker image changes | ||
inputs.property 'fullDockerImageName', System.getenv('FULL_DOCKER_IMAGE_NAME') | ||
} |
133 changes: 133 additions & 0 deletions
133
...-integration/src/test/java/com/netflix/spinnaker/clouddriver/StandaloneContainerTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
/* | ||
* Copyright 2024 Salesforce, Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.netflix.spinnaker.clouddriver; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.junit.jupiter.api.Assumptions.assumeTrue; | ||
|
||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import java.net.URI; | ||
import java.net.http.HttpClient; | ||
import java.net.http.HttpRequest; | ||
import java.net.http.HttpResponse; | ||
import java.time.Duration; | ||
import java.util.Map; | ||
import org.junit.jupiter.api.AfterAll; | ||
import org.junit.jupiter.api.BeforeAll; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.TestInfo; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.testcontainers.containers.GenericContainer; | ||
import org.testcontainers.containers.Network; | ||
import org.testcontainers.containers.output.Slf4jLogConsumer; | ||
import org.testcontainers.containers.wait.strategy.Wait; | ||
import org.testcontainers.junit.jupiter.Testcontainers; | ||
import org.testcontainers.utility.DockerImageName; | ||
|
||
@Testcontainers | ||
class StandaloneContainerTest { | ||
|
||
private static final String REDIS_NETWORK_ALIAS = "redisHost"; | ||
|
||
private static final int REDIS_PORT = 6379; | ||
|
||
private static final Logger logger = LoggerFactory.getLogger(StandaloneContainerTest.class); | ||
|
||
private static final Network network = Network.newNetwork(); | ||
|
||
private static final GenericContainer redis = | ||
new GenericContainer(DockerImageName.parse("library/redis:5-alpine")) | ||
.withNetwork(network) | ||
.withNetworkAliases(REDIS_NETWORK_ALIAS) | ||
.withExposedPorts(REDIS_PORT); | ||
|
||
private static GenericContainer clouddriverContainer; | ||
|
||
@BeforeAll | ||
static void setupOnce() throws Exception { | ||
String fullDockerImageName = System.getenv("FULL_DOCKER_IMAGE_NAME"); | ||
|
||
// Skip the tests if there's no docker image. This allows gradlew build to work. | ||
assumeTrue(fullDockerImageName != null); | ||
|
||
redis.start(); | ||
|
||
DockerImageName dockerImageName = DockerImageName.parse(fullDockerImageName); | ||
|
||
clouddriverContainer = | ||
new GenericContainer(dockerImageName) | ||
.withNetwork(network) | ||
.withExposedPorts(7002) | ||
.dependsOn(redis) | ||
.waitingFor(Wait.forHealthcheck().withStartupTimeout(Duration.ofSeconds(90))) | ||
.withEnv("SPRING_APPLICATION_JSON", getSpringApplicationJson()); | ||
|
||
Slf4jLogConsumer logConsumer = new Slf4jLogConsumer(logger); | ||
clouddriverContainer.start(); | ||
clouddriverContainer.followOutput(logConsumer); | ||
} | ||
|
||
private static String getSpringApplicationJson() throws JsonProcessingException { | ||
String redisUrl = "redis://" + REDIS_NETWORK_ALIAS + ":" + REDIS_PORT; | ||
logger.info("redisUrl: '{}'", redisUrl); | ||
Map<String, String> properties = | ||
Map.of("redis.connection", redisUrl, "services.fiat.baseUrl", "http://nowhere"); | ||
ObjectMapper mapper = new ObjectMapper(); | ||
return mapper.writeValueAsString(properties); | ||
} | ||
|
||
@AfterAll | ||
static void cleanupOnce() { | ||
if (clouddriverContainer != null) { | ||
clouddriverContainer.stop(); | ||
} | ||
|
||
if (redis != null) { | ||
redis.stop(); | ||
} | ||
} | ||
|
||
@BeforeEach | ||
void init(TestInfo testInfo) { | ||
System.out.println("--------------- Test " + testInfo.getDisplayName()); | ||
} | ||
|
||
@Test | ||
void testHealthCheck() throws Exception { | ||
// hit an arbitrary endpoint | ||
HttpRequest request = | ||
HttpRequest.newBuilder() | ||
.uri( | ||
new URI( | ||
"http://" | ||
+ clouddriverContainer.getHost() | ||
+ ":" | ||
+ clouddriverContainer.getFirstMappedPort() | ||
+ "/health")) | ||
.GET() | ||
.build(); | ||
|
||
HttpClient client = HttpClient.newHttpClient(); | ||
|
||
HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString()); | ||
assertThat(response).isNotNull(); | ||
logger.info("response: {}, {}", response.statusCode(), response.body()); | ||
assertThat(response.statusCode()).isEqualTo(200); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
<!-- | ||
Copyright 2024 Salesforce, Inc. | ||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
http://www.apache.org/licenses/LICENSE-2.0 | ||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
--> | ||
<configuration> | ||
|
||
<!-- see https://java.testcontainers.org/supported_docker_environment/logging_config/ --> | ||
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender"> | ||
<encoder> | ||
<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern> | ||
</encoder> | ||
</appender> | ||
|
||
<root level="DEBUG"> | ||
<appender-ref ref="STDOUT" /> | ||
</root> | ||
|
||
<logger name="org.testcontainers" level="INFO"/> | ||
<logger name="tc" level="INFO"/> | ||
<logger name="com.github.dockerjava" level="WARN"/> | ||
<logger name="com.github.dockerjava.zerodep.shaded.org.apache.hc.client5.http.wire" level="OFF"/> | ||
<logger name="wiremock.org.eclipse.jetty" level="INFO"/> | ||
</configuration> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hmm, this wouldn't work right? Because it's a complete object.
We need to check if the root status is
UP
and not a wide search, otherwise it could be a false positive.We could instead install
jq
if it doesn't already exist, and just query for it directly withjq '.status'
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.
MOST of the time it's only a complete object IF the "management" exposes ALL data. USUALLY it's a single line check.
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.
Understood, but better safe to assume that things may be configured differently or be subject to change, than having a pleasant surprise :p
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.
AKA this data is NORMALLY what's returned unless you use:
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.
(or similar)
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.
BUT... jq adds more deps... and this isn't used in k8s - only in say testiing via docker CLI. I'm not AGAINST JQ - particularly since can be useful... just a "which is the less evil"
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.
It's a small dep for preventing false positives. So I'd argue that is probably more satisfactory. We could also use something like perl, awk, etc to achieve this if those are included in our slim image, but the command is going to be much more unreadable if dependencies are that much of a concern. If we build our docker images intelligently, it should not be too bad to add a single binary.
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.
And we are already adding curl in this PR, so adding jq doesn't seem to be that bad since curl isn't used in k8s either
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.
How's ad6cf9a ?