Skip to content

Commit

Permalink
enhancement(ci): combine build steps for integration test workflows (v…
Browse files Browse the repository at this point in the history
…ectordotdev#17724)

- vdev integration test logic has ability to build with all integration
features flag
- CI workflows use the new vdev flag, and are run within the same job,
thus each step leverages the cached runner image
- Adds retries for integration tests both at nextest and between
bringup/teardown of the container services
- Reduces billable time for Integration Test Suite workflow by 90% 🚀
  • Loading branch information
neuronull authored Jul 3, 2023
1 parent 205300b commit 911477a
Show file tree
Hide file tree
Showing 17 changed files with 739 additions and 234 deletions.
367 changes: 288 additions & 79 deletions .github/workflows/integration-comment.yml

Large diffs are not rendered by default.

16 changes: 3 additions & 13 deletions .github/workflows/integration-test.yml
Original file line number Diff line number Diff line change
@@ -1,22 +1,12 @@
# This workflow is used to run an integration test.
# The most common use case is that it is triggered by another workflow,
# such as the Master Merge Queue Suite, or the Integration Comment.
# Integration Test
#
# It can also be triggered on manual dispatch in CI however.
# In that use case, an input for the test name needs to be provided.
# This workflow is used to run an integration test on demand.
# An input for the test name needs to be provided.
# TODO: check if the input is "all" , and run all, without a timeout?

name: Integration Test

on:
workflow_call:
inputs:
if:
required: false
type: boolean
test_name:
required: true
type: string
workflow_dispatch:
inputs:
test_name:
Expand Down
411 changes: 328 additions & 83 deletions .github/workflows/integration.yml

Large diffs are not rendered by default.

30 changes: 30 additions & 0 deletions scripts/ci-integration-test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/bin/bash

# Used in CI to run and stop an integration test and upload the results of it.
# This is useful to allow retrying the integration test at a higher level than
# the nextest and reduce code duplication in the workflow file.

set -u

if [[ -z "${CI:-}" ]]; then
echo "Aborted: this script is for use in CI." >&2
exit 1
fi

if [ $# -ne 1 ]
then
echo "usage: $0 INTEGRATION"
exit 1
fi

set -x

INTEGRATION=$1

cargo vdev -v int start "${INTEGRATION}"
sleep 30
cargo vdev -v int test --retries 2 -a "${INTEGRATION}"
RET=$?
cargo vdev -v int stop -a "${INTEGRATION}"
./scripts/upload-test-results.sh
exit $RET
12 changes: 0 additions & 12 deletions scripts/integration/chronicle/compose.yaml

This file was deleted.

10 changes: 0 additions & 10 deletions scripts/integration/chronicle/test.yaml

This file was deleted.

File renamed without changes.
9 changes: 9 additions & 0 deletions scripts/integration/gcp/compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,12 @@ services:
environment:
- PUBSUB_PROJECT1=testproject,topic1:subscription1
- PUBSUB_PROJECT2=sourceproject,topic2:subscription2
chronicle-emulator:
image: docker.io/plork/chronicle-emulator:${CONFIG_VERSION}
ports:
- 3000:3000
volumes:
- ./public.pem:/public.pem:ro
command:
- -p
- /public.pem
File renamed without changes.
File renamed without changes.
2 changes: 2 additions & 0 deletions scripts/integration/gcp/test.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
features:
- gcp-integration-tests
- chronicle-integration-tests

test_filter: '::gcp::'

env:
EMULATOR_ADDRESS: http://gcloud-pubsub:8681
CHRONICLE_ADDRESS: http://chronicle-emulator:3000

matrix:
version: [latest]
Expand Down
22 changes: 9 additions & 13 deletions src/sinks/gcp/chronicle_unstructured.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,12 +550,10 @@ mod integration_tests {
trace_init();

let log_type = random_string(10);
let (sink, healthcheck) = config_build(
&log_type,
"/home/vector/scripts/integration/chronicle/auth.json",
)
.await
.expect("Building sink failed");
let (sink, healthcheck) =
config_build(&log_type, "/home/vector/scripts/integration/gcp/auth.json")
.await
.expect("Building sink failed");

healthcheck.await.expect("Health check failed");

Expand Down Expand Up @@ -585,7 +583,7 @@ mod integration_tests {
// Test with an auth file that doesnt match the public key sent to the dummy chronicle server.
let sink = config_build(
&log_type,
"/home/vector/scripts/integration/chronicle/invalidauth.json",
"/home/vector/scripts/integration/gcp/invalidauth.json",
)
.await;

Expand All @@ -599,12 +597,10 @@ mod integration_tests {
// The chronicle-emulator we are testing against is setup so a `log_type` of "INVALID"
// will return a `400 BAD_REQUEST`.
let log_type = "INVALID";
let (sink, healthcheck) = config_build(
log_type,
"/home/vector/scripts/integration/chronicle/auth.json",
)
.await
.expect("Building sink failed");
let (sink, healthcheck) =
config_build(log_type, "/home/vector/scripts/integration/gcp/auth.json")
.await
.expect("Building sink failed");

healthcheck.await.expect("Health check failed");

Expand Down
2 changes: 1 addition & 1 deletion vdev/src/commands/integration/start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ impl Cli {
let env = envs.keys().next().expect("Integration has no environments");
env.clone()
};
IntegrationTest::new(self.integration, environment)?.start()
IntegrationTest::new(self.integration, environment, false, 0)?.start()
}
}
6 changes: 5 additions & 1 deletion vdev/src/commands/integration/stop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@ use crate::testing::{integration::IntegrationTest, state::EnvsDir};
pub struct Cli {
/// The integration name to stop
integration: String,

/// If true, remove the runner container compiled with all integration test features
#[arg(short = 'a', long)]
all_features: bool,
}

impl Cli {
pub fn exec(self) -> Result<()> {
if let Some(active) = EnvsDir::new(&self.integration).active()? {
IntegrationTest::new(self.integration, active)?.stop()
IntegrationTest::new(self.integration, active, self.all_features, 0)?.stop()
} else {
println!("No environment for {:?} is active.", self.integration);
Ok(())
Expand Down
22 changes: 19 additions & 3 deletions vdev/src/commands/integration/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ pub struct Cli {
/// The desired environment (optional)
environment: Option<String>,

/// Whether to compile the test runner with all integration test features
#[arg(short = 'a', long)]
build_all: bool,

/// Number of retries to allow on each integration test case.
#[arg(short = 'r', long)]
retries: Option<u8>,

/// Extra test command arguments
args: Vec<String>,
}
Expand All @@ -30,17 +38,25 @@ impl Cli {
let envs = config.environments();

let active = EnvsDir::new(&self.integration).active()?;

let retries = self.retries.unwrap_or_default();

match (self.environment, active) {
(Some(environment), Some(active)) if environment != active => {
bail!("Requested environment {environment:?} does not match active one {active:?}")
}
(Some(environment), _) => {
IntegrationTest::new(self.integration, environment)?.test(self.args)
IntegrationTest::new(self.integration, environment, self.build_all, retries)?
.test(self.args)
}
(None, Some(active)) => {
IntegrationTest::new(self.integration, active, self.build_all, retries)?
.test(self.args)
}
(None, Some(active)) => IntegrationTest::new(self.integration, active)?.test(self.args),
(None, None) => {
for env_name in envs.keys() {
IntegrationTest::new(&self.integration, env_name)?.test(self.args.clone())?;
IntegrationTest::new(&self.integration, env_name, self.build_all, retries)?
.test(self.args.clone())?;
}
Ok(())
}
Expand Down
29 changes: 26 additions & 3 deletions vdev/src/testing/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,17 @@ pub struct IntegrationTest {
runner: IntegrationTestRunner,
compose: Option<Compose>,
env_config: Environment,
build_all: bool,
retries: u8,
}

impl IntegrationTest {
pub fn new(integration: impl Into<String>, environment: impl Into<String>) -> Result<Self> {
pub fn new(
integration: impl Into<String>,
environment: impl Into<String>,
build_all: bool,
retries: u8,
) -> Result<Self> {
let integration = integration.into();
let environment = environment.into();
let (test_dir, config) = IntegrationTestConfig::load(&integration)?;
Expand All @@ -34,8 +41,12 @@ impl IntegrationTest {
};
let network_name = format!("vector-integration-tests-{integration}");
let compose = Compose::new(test_dir, env_config.clone(), network_name.clone())?;

// None if compiling with all integration test feature flag.
let runner_name = (!build_all).then(|| integration.clone());

let runner = IntegrationTestRunner::new(
integration.clone(),
runner_name,
&config.runner,
compose.is_some().then_some(network_name),
)?;
Expand All @@ -48,6 +59,8 @@ impl IntegrationTest {
runner,
compose,
env_config,
build_all,
retries,
})
}

Expand All @@ -69,7 +82,12 @@ impl IntegrationTest {
let mut args = self.config.args.clone().unwrap_or_default();

args.push("--features".to_string());
args.push(self.config.features.join(","));

args.push(if self.build_all {
"all-integration-tests".to_string()
} else {
self.config.features.join(",")
});

// If the test field is not present then use the --lib flag
match self.config.test {
Expand All @@ -91,6 +109,11 @@ impl IntegrationTest {
args.push("--no-capture".to_string());
}

if self.retries > 0 {
args.push("--retries".to_string());
args.push(self.retries.to_string());
}

self.runner
.test(&env_vars, &self.config.runner.env, &args)?;

Expand Down
35 changes: 19 additions & 16 deletions vdev/src/testing/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ fn detect_container_tool() -> OsString {
fatal!("No container tool could be detected.");
}

fn get_rust_version() -> String {
match RustToolchainConfig::parse() {
Ok(config) => config.channel,
Err(error) => fatal!("Could not read `rust-toolchain.toml` file: {error}"),
}
}

fn dockercmd<I: AsRef<OsStr>>(args: impl IntoIterator<Item = I>) -> Command {
let mut command = Command::new(&*CONTAINER_TOOL);
command.args(args);
Expand Down Expand Up @@ -93,13 +100,6 @@ pub trait ContainerTestRunner: TestRunner {
.wait(format!("Stopping container {}", self.container_name()))
}

fn get_rust_version(&self) -> String {
match RustToolchainConfig::parse() {
Ok(config) => config.channel,
Err(error) => fatal!("Could not read `rust-toolchain.toml` file: {error}"),
}
}

fn state(&self) -> Result<RunnerState> {
let mut command = dockercmd(["ps", "-a", "--format", "{{.Names}} {{.State}}"]);
let container_name = self.container_name();
Expand Down Expand Up @@ -183,8 +183,10 @@ pub trait ContainerTestRunner: TestRunner {
&self.image_name(),
"--file",
dockerfile.to_str().unwrap(),
"--label",
"vector-test-runner=true",
"--build-arg",
&format!("RUST_VERSION={}", self.get_rust_version()),
&format!("RUST_VERSION={}", get_rust_version()),
".",
]);

Expand Down Expand Up @@ -295,15 +297,16 @@ where
}

pub(super) struct IntegrationTestRunner {
integration: String,
// The integration is None when compiling the runner image with the `all-integration-tests` feature.
integration: Option<String>,
needs_docker_socket: bool,
network: Option<String>,
volumes: Vec<String>,
}

impl IntegrationTestRunner {
pub(super) fn new(
integration: String,
integration: Option<String>,
config: &IntegrationRunnerConfig,
network: Option<String>,
) -> Result<Self> {
Expand Down Expand Up @@ -344,11 +347,11 @@ impl ContainerTestRunner for IntegrationTestRunner {
}

fn container_name(&self) -> String {
format!(
"vector-test-runner-{}-{}",
self.integration,
self.get_rust_version()
)
if let Some(integration) = self.integration.as_ref() {
format!("vector-test-runner-{}-{}", integration, get_rust_version())
} else {
format!("vector-test-runner-{}", get_rust_version())
}
}

fn image_name(&self) -> String {
Expand All @@ -372,7 +375,7 @@ impl ContainerTestRunner for DockerTestRunner {
}

fn container_name(&self) -> String {
format!("vector-test-runner-{}", self.get_rust_version())
format!("vector-test-runner-{}", get_rust_version())
}

fn image_name(&self) -> String {
Expand Down

0 comments on commit 911477a

Please sign in to comment.