Skip to content

Commit 911477a

Browse files
authored
enhancement(ci): combine build steps for integration test workflows (#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% 🚀
1 parent 205300b commit 911477a

17 files changed

+739
-234
lines changed

.github/workflows/integration-comment.yml

+288-79
Large diffs are not rendered by default.

.github/workflows/integration-test.yml

+3-13
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,12 @@
1-
# This workflow is used to run an integration test.
2-
# The most common use case is that it is triggered by another workflow,
3-
# such as the Master Merge Queue Suite, or the Integration Comment.
1+
# Integration Test
42
#
5-
# It can also be triggered on manual dispatch in CI however.
6-
# In that use case, an input for the test name needs to be provided.
3+
# This workflow is used to run an integration test on demand.
4+
# An input for the test name needs to be provided.
75
# TODO: check if the input is "all" , and run all, without a timeout?
86

97
name: Integration Test
108

119
on:
12-
workflow_call:
13-
inputs:
14-
if:
15-
required: false
16-
type: boolean
17-
test_name:
18-
required: true
19-
type: string
2010
workflow_dispatch:
2111
inputs:
2212
test_name:

.github/workflows/integration.yml

+328-83
Large diffs are not rendered by default.

scripts/ci-integration-test.sh

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#!/bin/bash
2+
3+
# Used in CI to run and stop an integration test and upload the results of it.
4+
# This is useful to allow retrying the integration test at a higher level than
5+
# the nextest and reduce code duplication in the workflow file.
6+
7+
set -u
8+
9+
if [[ -z "${CI:-}" ]]; then
10+
echo "Aborted: this script is for use in CI." >&2
11+
exit 1
12+
fi
13+
14+
if [ $# -ne 1 ]
15+
then
16+
echo "usage: $0 INTEGRATION"
17+
exit 1
18+
fi
19+
20+
set -x
21+
22+
INTEGRATION=$1
23+
24+
cargo vdev -v int start "${INTEGRATION}"
25+
sleep 30
26+
cargo vdev -v int test --retries 2 -a "${INTEGRATION}"
27+
RET=$?
28+
cargo vdev -v int stop -a "${INTEGRATION}"
29+
./scripts/upload-test-results.sh
30+
exit $RET

scripts/integration/chronicle/compose.yaml

-12
This file was deleted.

scripts/integration/chronicle/test.yaml

-10
This file was deleted.
File renamed without changes.

scripts/integration/gcp/compose.yaml

+9
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,12 @@ services:
66
environment:
77
- PUBSUB_PROJECT1=testproject,topic1:subscription1
88
- PUBSUB_PROJECT2=sourceproject,topic2:subscription2
9+
chronicle-emulator:
10+
image: docker.io/plork/chronicle-emulator:${CONFIG_VERSION}
11+
ports:
12+
- 3000:3000
13+
volumes:
14+
- ./public.pem:/public.pem:ro
15+
command:
16+
- -p
17+
- /public.pem
File renamed without changes.

scripts/integration/gcp/test.yaml

+2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
features:
22
- gcp-integration-tests
3+
- chronicle-integration-tests
34

45
test_filter: '::gcp::'
56

67
env:
78
EMULATOR_ADDRESS: http://gcloud-pubsub:8681
9+
CHRONICLE_ADDRESS: http://chronicle-emulator:3000
810

911
matrix:
1012
version: [latest]

src/sinks/gcp/chronicle_unstructured.rs

+9-13
Original file line numberDiff line numberDiff line change
@@ -550,12 +550,10 @@ mod integration_tests {
550550
trace_init();
551551

552552
let log_type = random_string(10);
553-
let (sink, healthcheck) = config_build(
554-
&log_type,
555-
"/home/vector/scripts/integration/chronicle/auth.json",
556-
)
557-
.await
558-
.expect("Building sink failed");
553+
let (sink, healthcheck) =
554+
config_build(&log_type, "/home/vector/scripts/integration/gcp/auth.json")
555+
.await
556+
.expect("Building sink failed");
559557

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

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

@@ -599,12 +597,10 @@ mod integration_tests {
599597
// The chronicle-emulator we are testing against is setup so a `log_type` of "INVALID"
600598
// will return a `400 BAD_REQUEST`.
601599
let log_type = "INVALID";
602-
let (sink, healthcheck) = config_build(
603-
log_type,
604-
"/home/vector/scripts/integration/chronicle/auth.json",
605-
)
606-
.await
607-
.expect("Building sink failed");
600+
let (sink, healthcheck) =
601+
config_build(log_type, "/home/vector/scripts/integration/gcp/auth.json")
602+
.await
603+
.expect("Building sink failed");
608604

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

vdev/src/commands/integration/start.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@ impl Cli {
2424
let env = envs.keys().next().expect("Integration has no environments");
2525
env.clone()
2626
};
27-
IntegrationTest::new(self.integration, environment)?.start()
27+
IntegrationTest::new(self.integration, environment, false, 0)?.start()
2828
}
2929
}

vdev/src/commands/integration/stop.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,16 @@ use crate::testing::{integration::IntegrationTest, state::EnvsDir};
99
pub struct Cli {
1010
/// The integration name to stop
1111
integration: String,
12+
13+
/// If true, remove the runner container compiled with all integration test features
14+
#[arg(short = 'a', long)]
15+
all_features: bool,
1216
}
1317

1418
impl Cli {
1519
pub fn exec(self) -> Result<()> {
1620
if let Some(active) = EnvsDir::new(&self.integration).active()? {
17-
IntegrationTest::new(self.integration, active)?.stop()
21+
IntegrationTest::new(self.integration, active, self.all_features, 0)?.stop()
1822
} else {
1923
println!("No environment for {:?} is active.", self.integration);
2024
Ok(())

vdev/src/commands/integration/test.rs

+19-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ pub struct Cli {
2020
/// The desired environment (optional)
2121
environment: Option<String>,
2222

23+
/// Whether to compile the test runner with all integration test features
24+
#[arg(short = 'a', long)]
25+
build_all: bool,
26+
27+
/// Number of retries to allow on each integration test case.
28+
#[arg(short = 'r', long)]
29+
retries: Option<u8>,
30+
2331
/// Extra test command arguments
2432
args: Vec<String>,
2533
}
@@ -30,17 +38,25 @@ impl Cli {
3038
let envs = config.environments();
3139

3240
let active = EnvsDir::new(&self.integration).active()?;
41+
42+
let retries = self.retries.unwrap_or_default();
43+
3344
match (self.environment, active) {
3445
(Some(environment), Some(active)) if environment != active => {
3546
bail!("Requested environment {environment:?} does not match active one {active:?}")
3647
}
3748
(Some(environment), _) => {
38-
IntegrationTest::new(self.integration, environment)?.test(self.args)
49+
IntegrationTest::new(self.integration, environment, self.build_all, retries)?
50+
.test(self.args)
51+
}
52+
(None, Some(active)) => {
53+
IntegrationTest::new(self.integration, active, self.build_all, retries)?
54+
.test(self.args)
3955
}
40-
(None, Some(active)) => IntegrationTest::new(self.integration, active)?.test(self.args),
4156
(None, None) => {
4257
for env_name in envs.keys() {
43-
IntegrationTest::new(&self.integration, env_name)?.test(self.args.clone())?;
58+
IntegrationTest::new(&self.integration, env_name, self.build_all, retries)?
59+
.test(self.args.clone())?;
4460
}
4561
Ok(())
4662
}

vdev/src/testing/integration.rs

+26-3
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,17 @@ pub struct IntegrationTest {
2121
runner: IntegrationTestRunner,
2222
compose: Option<Compose>,
2323
env_config: Environment,
24+
build_all: bool,
25+
retries: u8,
2426
}
2527

2628
impl IntegrationTest {
27-
pub fn new(integration: impl Into<String>, environment: impl Into<String>) -> Result<Self> {
29+
pub fn new(
30+
integration: impl Into<String>,
31+
environment: impl Into<String>,
32+
build_all: bool,
33+
retries: u8,
34+
) -> Result<Self> {
2835
let integration = integration.into();
2936
let environment = environment.into();
3037
let (test_dir, config) = IntegrationTestConfig::load(&integration)?;
@@ -34,8 +41,12 @@ impl IntegrationTest {
3441
};
3542
let network_name = format!("vector-integration-tests-{integration}");
3643
let compose = Compose::new(test_dir, env_config.clone(), network_name.clone())?;
44+
45+
// None if compiling with all integration test feature flag.
46+
let runner_name = (!build_all).then(|| integration.clone());
47+
3748
let runner = IntegrationTestRunner::new(
38-
integration.clone(),
49+
runner_name,
3950
&config.runner,
4051
compose.is_some().then_some(network_name),
4152
)?;
@@ -48,6 +59,8 @@ impl IntegrationTest {
4859
runner,
4960
compose,
5061
env_config,
62+
build_all,
63+
retries,
5164
})
5265
}
5366

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

7184
args.push("--features".to_string());
72-
args.push(self.config.features.join(","));
85+
86+
args.push(if self.build_all {
87+
"all-integration-tests".to_string()
88+
} else {
89+
self.config.features.join(",")
90+
});
7391

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

112+
if self.retries > 0 {
113+
args.push("--retries".to_string());
114+
args.push(self.retries.to_string());
115+
}
116+
94117
self.runner
95118
.test(&env_vars, &self.config.runner.env, &args)?;
96119

vdev/src/testing/runner.rs

+19-16
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ fn detect_container_tool() -> OsString {
4747
fatal!("No container tool could be detected.");
4848
}
4949

50+
fn get_rust_version() -> String {
51+
match RustToolchainConfig::parse() {
52+
Ok(config) => config.channel,
53+
Err(error) => fatal!("Could not read `rust-toolchain.toml` file: {error}"),
54+
}
55+
}
56+
5057
fn dockercmd<I: AsRef<OsStr>>(args: impl IntoIterator<Item = I>) -> Command {
5158
let mut command = Command::new(&*CONTAINER_TOOL);
5259
command.args(args);
@@ -93,13 +100,6 @@ pub trait ContainerTestRunner: TestRunner {
93100
.wait(format!("Stopping container {}", self.container_name()))
94101
}
95102

96-
fn get_rust_version(&self) -> String {
97-
match RustToolchainConfig::parse() {
98-
Ok(config) => config.channel,
99-
Err(error) => fatal!("Could not read `rust-toolchain.toml` file: {error}"),
100-
}
101-
}
102-
103103
fn state(&self) -> Result<RunnerState> {
104104
let mut command = dockercmd(["ps", "-a", "--format", "{{.Names}} {{.State}}"]);
105105
let container_name = self.container_name();
@@ -183,8 +183,10 @@ pub trait ContainerTestRunner: TestRunner {
183183
&self.image_name(),
184184
"--file",
185185
dockerfile.to_str().unwrap(),
186+
"--label",
187+
"vector-test-runner=true",
186188
"--build-arg",
187-
&format!("RUST_VERSION={}", self.get_rust_version()),
189+
&format!("RUST_VERSION={}", get_rust_version()),
188190
".",
189191
]);
190192

@@ -295,15 +297,16 @@ where
295297
}
296298

297299
pub(super) struct IntegrationTestRunner {
298-
integration: String,
300+
// The integration is None when compiling the runner image with the `all-integration-tests` feature.
301+
integration: Option<String>,
299302
needs_docker_socket: bool,
300303
network: Option<String>,
301304
volumes: Vec<String>,
302305
}
303306

304307
impl IntegrationTestRunner {
305308
pub(super) fn new(
306-
integration: String,
309+
integration: Option<String>,
307310
config: &IntegrationRunnerConfig,
308311
network: Option<String>,
309312
) -> Result<Self> {
@@ -344,11 +347,11 @@ impl ContainerTestRunner for IntegrationTestRunner {
344347
}
345348

346349
fn container_name(&self) -> String {
347-
format!(
348-
"vector-test-runner-{}-{}",
349-
self.integration,
350-
self.get_rust_version()
351-
)
350+
if let Some(integration) = self.integration.as_ref() {
351+
format!("vector-test-runner-{}-{}", integration, get_rust_version())
352+
} else {
353+
format!("vector-test-runner-{}", get_rust_version())
354+
}
352355
}
353356

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

374377
fn container_name(&self) -> String {
375-
format!("vector-test-runner-{}", self.get_rust_version())
378+
format!("vector-test-runner-{}", get_rust_version())
376379
}
377380

378381
fn image_name(&self) -> String {

0 commit comments

Comments
 (0)