From 9e6de827b128e54e7fa412573f34c126e52ecbdc Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Fri, 31 Mar 2023 16:00:52 -0400 Subject: [PATCH 01/21] enhancement(vdev): Load compose files and inject network block --- scripts/integration/amqp/compose.yaml | 4 --- scripts/integration/aws/compose.yaml | 4 --- scripts/integration/azure/compose.yaml | 4 --- scripts/integration/chronicle/compose.yaml | 4 --- scripts/integration/clickhouse/compose.yaml | 4 --- scripts/integration/databend/compose.yaml | 4 --- .../integration/datadog-agent/compose.yaml | 4 --- .../integration/datadog-traces/compose.yaml | 4 --- scripts/integration/dnstap/compose.yaml | 4 --- .../integration/elasticsearch/compose.yaml | 4 --- scripts/integration/eventstoredb/compose.yaml | 4 --- scripts/integration/gcp/compose.yaml | 4 --- scripts/integration/http-client/compose.yaml | 4 --- scripts/integration/humio/compose.yaml | 4 --- scripts/integration/influxdb/compose.yaml | 4 --- scripts/integration/kafka/compose.yaml | 4 --- scripts/integration/logstash/compose.yaml | 4 --- scripts/integration/loki/compose.yaml | 4 --- scripts/integration/mongodb/compose.yaml | 4 --- scripts/integration/nats/compose.yaml | 4 --- scripts/integration/nginx/compose.yaml | 5 --- .../integration/opentelemetry/compose.yaml | 4 --- scripts/integration/postgres/compose.yaml | 4 --- scripts/integration/prometheus/compose.yaml | 4 --- scripts/integration/pulsar/compose.yaml | 4 --- scripts/integration/redis/compose.yaml | 4 --- scripts/integration/shutdown/compose.yaml | 4 --- scripts/integration/splunk/compose.yaml | 4 --- scripts/integration/webhdfs/compose.yaml | 4 --- vdev/src/testing/config.rs | 14 +++++--- vdev/src/testing/integration.rs | 33 +++++++++++++++++-- 31 files changed, 40 insertions(+), 124 deletions(-) diff --git a/scripts/integration/amqp/compose.yaml b/scripts/integration/amqp/compose.yaml index aa11f17a43af9..9c268b7ede3ff 100644 --- a/scripts/integration/amqp/compose.yaml +++ b/scripts/integration/amqp/compose.yaml @@ -5,7 +5,3 @@ services: image: docker.io/rabbitmq:${CONFIG_VERSION} ports: - 5672:5672 - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/aws/compose.yaml b/scripts/integration/aws/compose.yaml index 41f72695c4a01..07ed1fde4311d 100644 --- a/scripts/integration/aws/compose.yaml +++ b/scripts/integration/aws/compose.yaml @@ -14,7 +14,3 @@ services: volumes: - $DOCKER_SOCKET:/var/run/docker.sock - $HOME/.aws/:/home/.aws/ - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/azure/compose.yaml b/scripts/integration/azure/compose.yaml index 9fdf8a76a1cd4..6c7b00e37d6e1 100644 --- a/scripts/integration/azure/compose.yaml +++ b/scripts/integration/azure/compose.yaml @@ -6,7 +6,3 @@ services: command: azurite --blobHost 0.0.0.0 --loose volumes: - /var/run:/var/run - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/chronicle/compose.yaml b/scripts/integration/chronicle/compose.yaml index 75ae40bdbb54d..09097e24a97de 100644 --- a/scripts/integration/chronicle/compose.yaml +++ b/scripts/integration/chronicle/compose.yaml @@ -10,7 +10,3 @@ services: command: - -p - /public.pem - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/clickhouse/compose.yaml b/scripts/integration/clickhouse/compose.yaml index 4ac410aa1e6ac..fe11611e9265d 100644 --- a/scripts/integration/clickhouse/compose.yaml +++ b/scripts/integration/clickhouse/compose.yaml @@ -3,7 +3,3 @@ version: '3' services: clickhouse: image: docker.io/yandex/clickhouse-server:${CONFIG_VERSION} - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/databend/compose.yaml b/scripts/integration/databend/compose.yaml index 15bad6fd9b9cc..65ed7a9c809d2 100644 --- a/scripts/integration/databend/compose.yaml +++ b/scripts/integration/databend/compose.yaml @@ -19,7 +19,3 @@ services: - minio healthcheck: test: "curl -f localhost:8080/v1/health || exit 1" - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/datadog-agent/compose.yaml b/scripts/integration/datadog-agent/compose.yaml index de8d93037915b..5a36d822ac6ca 100644 --- a/scripts/integration/datadog-agent/compose.yaml +++ b/scripts/integration/datadog-agent/compose.yaml @@ -26,7 +26,3 @@ services: - DD_CMD_PORT=5002 - DD_USE_DOGSTATSD=false - DD_HOSTNAME=datadog-trace-agent - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/datadog-traces/compose.yaml b/scripts/integration/datadog-traces/compose.yaml index 1390ce472ab17..8ae4c0dd6c026 100644 --- a/scripts/integration/datadog-traces/compose.yaml +++ b/scripts/integration/datadog-traces/compose.yaml @@ -29,7 +29,3 @@ services: - DD_APM_MAX_MEMORY=0 - DD_APM_MAX_CPU_PERCENT=0 - DD_HOSTNAME=datadog-trace-agent-to-vector - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/dnstap/compose.yaml b/scripts/integration/dnstap/compose.yaml index 2ed599a55cf4a..ae5f973aff541 100644 --- a/scripts/integration/dnstap/compose.yaml +++ b/scripts/integration/dnstap/compose.yaml @@ -11,9 +11,5 @@ services: - dnstap-sockets:/bind2/etc/bind/socket - dnstap-sockets:/bind3/etc/bind/socket -networks: - default: - name: ${VECTOR_NETWORK} - volumes: dnstap-sockets: {} diff --git a/scripts/integration/elasticsearch/compose.yaml b/scripts/integration/elasticsearch/compose.yaml index 338cff73f2fde..cdaf5e2b1d64c 100644 --- a/scripts/integration/elasticsearch/compose.yaml +++ b/scripts/integration/elasticsearch/compose.yaml @@ -25,7 +25,3 @@ services: - ES_JAVA_OPTS=-Xms400m -Xmx400m volumes: - ../../../tests/data/ca:/usr/share/elasticsearch/config/certs:ro - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/eventstoredb/compose.yaml b/scripts/integration/eventstoredb/compose.yaml index c29906f7c033d..324bfeeb84c54 100644 --- a/scripts/integration/eventstoredb/compose.yaml +++ b/scripts/integration/eventstoredb/compose.yaml @@ -6,7 +6,3 @@ services: command: --insecure --stats-period-sec=1 volumes: - ../../../tests/data:/etc/vector:ro - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/gcp/compose.yaml b/scripts/integration/gcp/compose.yaml index b067a1c622a87..643362a2bc76e 100644 --- a/scripts/integration/gcp/compose.yaml +++ b/scripts/integration/gcp/compose.yaml @@ -6,7 +6,3 @@ services: environment: - PUBSUB_PROJECT1=testproject,topic1:subscription1 - PUBSUB_PROJECT2=sourceproject,topic2:subscription2 - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/http-client/compose.yaml b/scripts/integration/http-client/compose.yaml index c3b06ecb0d6d0..69ad1c6ef3d94 100644 --- a/scripts/integration/http-client/compose.yaml +++ b/scripts/integration/http-client/compose.yaml @@ -29,7 +29,3 @@ services: - ../../../tests/data/http-client/serve:/data - ../../../tests/data/ca/intermediate_server/certs/dufs-https-chain.cert.pem:/certs/ca.cert.pem - ../../../tests/data/ca/intermediate_server/private/dufs-https.key.pem:/certs/ca.key.pem - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/humio/compose.yaml b/scripts/integration/humio/compose.yaml index bbef37c60c86c..fa95bec0826bd 100644 --- a/scripts/integration/humio/compose.yaml +++ b/scripts/integration/humio/compose.yaml @@ -3,7 +3,3 @@ version: '3' services: humio: image: docker.io/humio/humio:${CONFIG_VERSION} - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/influxdb/compose.yaml b/scripts/integration/influxdb/compose.yaml index d1972a3b5f6b2..9202280345c19 100644 --- a/scripts/integration/influxdb/compose.yaml +++ b/scripts/integration/influxdb/compose.yaml @@ -19,7 +19,3 @@ services: command: influxd --reporting-disabled environment: - INFLUXDB_REPORTING_DISABLED=true - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/kafka/compose.yaml b/scripts/integration/kafka/compose.yaml index 7287561df47ff..1f1ee04cd7baa 100644 --- a/scripts/integration/kafka/compose.yaml +++ b/scripts/integration/kafka/compose.yaml @@ -33,7 +33,3 @@ services: volumes: - ../../../tests/data/ca/intermediate_server/private/kafka.p12:/certs/kafka.p12:ro - ../../../tests/data/kafka_server_jaas.conf:/etc/kafka/kafka_server_jaas.conf - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/logstash/compose.yaml b/scripts/integration/logstash/compose.yaml index ae8fcdbf602f9..9d1e16f2bee19 100644 --- a/scripts/integration/logstash/compose.yaml +++ b/scripts/integration/logstash/compose.yaml @@ -12,7 +12,3 @@ services: - /dev/null:/usr/share/logstash/pipeline/logstash.yml - ../../../tests/data/host.docker.internal.crt:/tmp/logstash.crt - ../../../tests/data/logstash/logstash.conf:/usr/share/logstash/pipeline/logstash.conf - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/loki/compose.yaml b/scripts/integration/loki/compose.yaml index 102e3ea5e8837..bca8cd4a458e2 100644 --- a/scripts/integration/loki/compose.yaml +++ b/scripts/integration/loki/compose.yaml @@ -4,7 +4,3 @@ services: loki: image: docker.io/grafana/loki:${CONFIG_VERSION} command: -config.file=/etc/loki/local-config.yaml -auth.enabled=true - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/mongodb/compose.yaml b/scripts/integration/mongodb/compose.yaml index 45d01a6a1e3e5..8db5c198b8b36 100644 --- a/scripts/integration/mongodb/compose.yaml +++ b/scripts/integration/mongodb/compose.yaml @@ -30,7 +30,3 @@ services: - MONGODB_INITIAL_PRIMARY_PORT_NUMBER=27017 - MONGODB_INITIAL_PRIMARY_ROOT_PASSWORD=toor - MONGODB_REPLICA_SET_KEY=vector - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/nats/compose.yaml b/scripts/integration/nats/compose.yaml index f0a827111056f..fb36e78d819d4 100644 --- a/scripts/integration/nats/compose.yaml +++ b/scripts/integration/nats/compose.yaml @@ -43,7 +43,3 @@ services: - /usr/share/nats/config/nats-jwt.conf volumes: - ../../../tests/data/nats:/usr/share/nats/config - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/nginx/compose.yaml b/scripts/integration/nginx/compose.yaml index a702f7bb2c2e8..ea4d734d12e0b 100644 --- a/scripts/integration/nginx/compose.yaml +++ b/scripts/integration/nginx/compose.yaml @@ -20,8 +20,3 @@ services: - ../../../tests/data/nginx/:/etc/nginx:ro networks: - proxy - -networks: - default: - name: ${VECTOR_NETWORK} - proxy: {} diff --git a/scripts/integration/opentelemetry/compose.yaml b/scripts/integration/opentelemetry/compose.yaml index 1764eae606dce..e4b100750f085 100644 --- a/scripts/integration/opentelemetry/compose.yaml +++ b/scripts/integration/opentelemetry/compose.yaml @@ -5,7 +5,3 @@ services: image: docker.io/otel/opentelemetry-collector-contrib:${CONFIG_VERSION} volumes: - ../../../tests/data/opentelemetry/config.yaml:/etc/otelcol-contrib/config.yaml - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/postgres/compose.yaml b/scripts/integration/postgres/compose.yaml index 137715264442b..ab73d2bbc2303 100644 --- a/scripts/integration/postgres/compose.yaml +++ b/scripts/integration/postgres/compose.yaml @@ -12,9 +12,5 @@ services: - ../../../tests/data/postgres-init.sh:/postgres-init.sh:ro - ../../../tests/data/ca:/certs:ro -networks: - default: - name: ${VECTOR_NETWORK} - volumes: socket: {} diff --git a/scripts/integration/prometheus/compose.yaml b/scripts/integration/prometheus/compose.yaml index 1036ee17c163e..e41592c28eccf 100644 --- a/scripts/integration/prometheus/compose.yaml +++ b/scripts/integration/prometheus/compose.yaml @@ -21,7 +21,3 @@ services: command: --config.file=/etc/vector/prometheus.yaml volumes: - ../../../tests/data:/etc/vector:ro - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/pulsar/compose.yaml b/scripts/integration/pulsar/compose.yaml index a94a4021fe76d..b73d35909be9d 100644 --- a/scripts/integration/pulsar/compose.yaml +++ b/scripts/integration/pulsar/compose.yaml @@ -6,7 +6,3 @@ services: command: bin/pulsar standalone ports: - 6650:6650 - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/redis/compose.yaml b/scripts/integration/redis/compose.yaml index 1b08ef7c3ba2d..a5dd865e43579 100644 --- a/scripts/integration/redis/compose.yaml +++ b/scripts/integration/redis/compose.yaml @@ -3,7 +3,3 @@ version: '3' services: redis: image: docker.io/redis:${CONFIG_VERSION} - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/shutdown/compose.yaml b/scripts/integration/shutdown/compose.yaml index aa2ec34a0e4cb..a74d753e841bc 100644 --- a/scripts/integration/shutdown/compose.yaml +++ b/scripts/integration/shutdown/compose.yaml @@ -31,7 +31,3 @@ services: volumes: - ../../../tests/data/ca/intermediate_server/private/kafka.p12:/certs/kafka.p12:ro - ../../../tests/data/kafka_server_jaas.conf:/etc/kafka/kafka_server_jaas.conf - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/splunk/compose.yaml b/scripts/integration/splunk/compose.yaml index 8805ad726dcc0..f42364adb2418 100644 --- a/scripts/integration/splunk/compose.yaml +++ b/scripts/integration/splunk/compose.yaml @@ -13,7 +13,3 @@ services: - 8000:8000 - 8088:8088 - 8089:8089 - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/scripts/integration/webhdfs/compose.yaml b/scripts/integration/webhdfs/compose.yaml index acf094c0bb882..9585927e84a10 100644 --- a/scripts/integration/webhdfs/compose.yaml +++ b/scripts/integration/webhdfs/compose.yaml @@ -30,7 +30,3 @@ services: interval: 5s timeout: 5s retries: 3 - -networks: - default: - name: ${VECTOR_NETWORK} diff --git a/vdev/src/testing/config.rs b/vdev/src/testing/config.rs index 8a2d7e4e263fa..ce5dc390e4397 100644 --- a/vdev/src/testing/config.rs +++ b/vdev/src/testing/config.rs @@ -5,7 +5,7 @@ use std::{env, fs}; use anyhow::{bail, Context, Result}; use hashlink::LinkedHashMap; use itertools::{self, Itertools}; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use serde_yaml::Value; use crate::{app, util}; @@ -37,15 +37,21 @@ impl RustToolchainConfig { } } -#[derive(Debug, Deserialize)] +// Added the networks key, to match the structure of networks eg +// networks: default: name: value: +#[derive(Debug, Deserialize, Serialize)] pub struct ComposeConfig { pub services: BTreeMap, - #[serde(default)] + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub volumes: BTreeMap, + #[serde(default)] + pub networks: BTreeMap>, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] pub struct ComposeService { + pub image: String, + #[serde(default, skip_serializing_if = "Option::is_none")] pub volumes: Option>, } diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index 6cfe9e84a66ef..b0838cd62530b 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -1,4 +1,5 @@ -use std::{path::Path, path::PathBuf, process::Command}; +use std::collections::BTreeMap; +use std::{fs, path::Path, path::PathBuf, process::Command}; use anyhow::{bail, Context, Result}; @@ -28,11 +29,28 @@ impl IntegrationTest { let integration = integration.into(); let environment = environment.into(); let (test_dir, config) = IntegrationTestConfig::load(&integration)?; + // Load thee compose_config + let mut compose_config = ComposeConfig::parse(&test_dir.join("compose.yaml"))?; + let network_name = format!("vector-integration-tests-{integration}"); + // Inject the networks block + // TODO: Inject the labels block + compose_config.networks.insert("default".to_string(), { + let mut default_network = BTreeMap::new(); + default_network.insert("name".to_string(), network_name.clone()); + default_network + }); + + let temp_compose_path = test_dir.join("compose-temp.yaml"); + fs::write( + temp_compose_path, + serde_yaml::to_string(&compose_config) + .with_context(|| "Failed to serialize modified compose.yml".to_string())?, + )?; + let envs_dir = EnvsDir::new(&integration); let Some(env_config) = config.environments().get(&environment).map(Clone::clone) else { bail!("Could not find environment named {environment:?}"); }; - let network_name = format!("vector-integration-tests-{integration}"); let compose = Compose::new(test_dir, env_config.clone(), Some(network_name.clone()))?; let runner = IntegrationTestRunner::new( integration.clone(), @@ -134,6 +152,10 @@ impl IntegrationTest { } } +// Question: Should I inject network here or should I keep it in the compose config? +// I am leaning towards keeping it in the compose config. Since that seems to be the +// Compose file direct mapping, but I would like guidance on this. For now I can +// Add to network by reading the compose config. struct Compose { path: PathBuf, test_dir: PathBuf, @@ -145,13 +167,17 @@ struct Compose { impl Compose { fn new(test_dir: PathBuf, env: Environment, network: Option) -> Result> { - let path: PathBuf = [&test_dir, Path::new("compose.yaml")].iter().collect(); + let path: PathBuf = [&test_dir, Path::new("compose-temp.yaml")].iter().collect(); match path.try_exists() { Err(error) => Err(error).with_context(|| format!("Could not lookup {path:?}")), Ok(false) => Ok(None), Ok(true) => { #[cfg(unix)] let config = ComposeConfig::parse(&path)?; + println!( + "[jonathanpv:debugging]\nUsing compose file: {:?}, found in: {:?}", + config, path + ); Ok(Some(Self { path, test_dir, @@ -171,6 +197,7 @@ impl Compose { fn stop(&self) -> Result<()> { // The config settings are not needed when stopping a compose setup. + std::fs::remove_file(&self.path)?; self.run("Stopping", &["down", "--timeout", "0", "--volumes"], None) } From fc15fbddd0b7cc1c79983d47e17740b56555b4b4 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Fri, 31 Mar 2023 18:51:37 -0400 Subject: [PATCH 02/21] remove temp file after compose.stop() is called --- vdev/src/testing/integration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index b0838cd62530b..9e44677b1d703 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -146,6 +146,7 @@ impl IntegrationTest { self.runner.remove()?; compose.stop()?; self.envs_dir.remove()?; + std::fs::remove_file(&compose.path)?; } Ok(()) @@ -197,7 +198,6 @@ impl Compose { fn stop(&self) -> Result<()> { // The config settings are not needed when stopping a compose setup. - std::fs::remove_file(&self.path)?; self.run("Stopping", &["down", "--timeout", "0", "--volumes"], None) } From 537049b34eee73a305da3d61caad7dac6a4ceb96 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Sat, 1 Apr 2023 21:38:23 -0400 Subject: [PATCH 03/21] Make config service for serialization include more fields --- vdev/src/testing/config.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/vdev/src/testing/config.rs b/vdev/src/testing/config.rs index ce5dc390e4397..fba3794439736 100644 --- a/vdev/src/testing/config.rs +++ b/vdev/src/testing/config.rs @@ -52,7 +52,35 @@ pub struct ComposeConfig { pub struct ComposeService { pub image: String, #[serde(default, skip_serializing_if = "Option::is_none")] + pub command: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub volumes: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub environment: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub depends_on: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub healthcheck: Option, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct HealthCheck { + pub test: String, + #[serde(rename = "interval", skip_serializing_if = "Option::is_none")] + pub interval: Option, + #[serde(rename = "timeout", skip_serializing_if = "Option::is_none")] + pub timeout: Option, + #[serde(rename = "retries", skip_serializing_if = "Option::is_none")] + pub retries: Option, + #[serde(rename = "start_period", skip_serializing_if = "Option::is_none")] + pub start_period: Option, +} + +#[derive(Debug, Deserialize, Serialize)] +#[serde(untagged)] +pub enum Command { + Single(String), + Multiple(Vec), } impl ComposeConfig { From 06433abaf415d8eea4d64d8e489b7d4edae426e8 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Sat, 1 Apr 2023 22:12:16 -0400 Subject: [PATCH 04/21] Add compose support for webdhfs --- vdev/src/testing/config.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/vdev/src/testing/config.rs b/vdev/src/testing/config.rs index fba3794439736..357c0125d04c3 100644 --- a/vdev/src/testing/config.rs +++ b/vdev/src/testing/config.rs @@ -50,10 +50,21 @@ pub struct ComposeConfig { #[derive(Debug, Deserialize, Serialize)] pub struct ComposeService { - pub image: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub image: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub hostname: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub container_name: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub build: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub command: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub ports: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub env_file: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] pub volumes: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] pub environment: Option>, @@ -65,7 +76,8 @@ pub struct ComposeService { #[derive(Debug, Deserialize, Serialize)] pub struct HealthCheck { - pub test: String, + #[serde(rename = "test", skip_serializing_if = "Option::is_none")] + pub test: Option, #[serde(rename = "interval", skip_serializing_if = "Option::is_none")] pub interval: Option, #[serde(rename = "timeout", skip_serializing_if = "Option::is_none")] @@ -76,6 +88,12 @@ pub struct HealthCheck { pub start_period: Option, } +#[derive(Debug, Deserialize, Serialize)] +pub struct BuildConfig { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub context: Option +} + #[derive(Debug, Deserialize, Serialize)] #[serde(untagged)] pub enum Command { From 1209010079f85eb332cd012d1a37d1514a26eb14 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Sat, 1 Apr 2023 23:02:26 -0400 Subject: [PATCH 05/21] remove renaming setting from serde annotations --- vdev/src/testing/config.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/vdev/src/testing/config.rs b/vdev/src/testing/config.rs index 357c0125d04c3..8881cf67f1844 100644 --- a/vdev/src/testing/config.rs +++ b/vdev/src/testing/config.rs @@ -76,15 +76,15 @@ pub struct ComposeService { #[derive(Debug, Deserialize, Serialize)] pub struct HealthCheck { - #[serde(rename = "test", skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] pub test: Option, - #[serde(rename = "interval", skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] pub interval: Option, - #[serde(rename = "timeout", skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] pub timeout: Option, - #[serde(rename = "retries", skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] pub retries: Option, - #[serde(rename = "start_period", skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] pub start_period: Option, } From 7b7a34623d5d5c60ee3d435075f89165d8b1af0b Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Sat, 1 Apr 2023 23:20:53 -0400 Subject: [PATCH 06/21] Use from_iter() in network injection --- vdev/src/testing/integration.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index 9e44677b1d703..2ecb05a5939f1 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -35,9 +35,7 @@ impl IntegrationTest { // Inject the networks block // TODO: Inject the labels block compose_config.networks.insert("default".to_string(), { - let mut default_network = BTreeMap::new(); - default_network.insert("name".to_string(), network_name.clone()); - default_network + BTreeMap::from_iter([("name".to_string(), network_name.clone())]) }); let temp_compose_path = test_dir.join("compose-temp.yaml"); From 1562ad7575c5acf7e656e47ee534ed47b907b00c Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Mon, 3 Apr 2023 13:36:44 -0400 Subject: [PATCH 07/21] utilize tempfile for creation of the rewritten compose file --- Cargo.lock | 1 + vdev/Cargo.toml | 1 + vdev/src/testing/integration.rs | 59 ++++++++++++++++++--------------- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f76409037990c..b732da43a9c52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9320,6 +9320,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml 0.9.19", + "tempfile", "toml 0.7.3", ] diff --git a/vdev/Cargo.toml b/vdev/Cargo.toml index cedc06cd82e3b..8e83576b71993 100644 --- a/vdev/Cargo.toml +++ b/vdev/Cargo.toml @@ -32,4 +32,5 @@ regex = { version = "1.7.3", default-features = false, features = ["std", "unico serde = { version = "1.0", features = ["derive"] } serde_json = "1.0.95" serde_yaml = "0.9.19" +tempfile = "3.5.0" toml = { version = "0.7.3", default-features = false, features = ["parse"] } diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index 2ecb05a5939f1..e98844d5e07d4 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -1,6 +1,6 @@ use std::collections::BTreeMap; use std::{fs, path::Path, path::PathBuf, process::Command}; - +use tempfile::{Builder, NamedTempFile}; use anyhow::{bail, Context, Result}; #[cfg(unix)] @@ -29,22 +29,8 @@ impl IntegrationTest { let integration = integration.into(); let environment = environment.into(); let (test_dir, config) = IntegrationTestConfig::load(&integration)?; - // Load thee compose_config - let mut compose_config = ComposeConfig::parse(&test_dir.join("compose.yaml"))?; - let network_name = format!("vector-integration-tests-{integration}"); - // Inject the networks block - // TODO: Inject the labels block - compose_config.networks.insert("default".to_string(), { - BTreeMap::from_iter([("name".to_string(), network_name.clone())]) - }); - - let temp_compose_path = test_dir.join("compose-temp.yaml"); - fs::write( - temp_compose_path, - serde_yaml::to_string(&compose_config) - .with_context(|| "Failed to serialize modified compose.yml".to_string())?, - )?; + let network_name = format!("vector-integration-tests-{integration}"); let envs_dir = EnvsDir::new(&integration); let Some(env_config) = config.environments().get(&environment).map(Clone::clone) else { bail!("Could not find environment named {environment:?}"); @@ -144,41 +130,60 @@ impl IntegrationTest { self.runner.remove()?; compose.stop()?; self.envs_dir.remove()?; - std::fs::remove_file(&compose.path)?; } Ok(()) } } -// Question: Should I inject network here or should I keep it in the compose config? -// I am leaning towards keeping it in the compose config. Since that seems to be the -// Compose file direct mapping, but I would like guidance on this. For now I can -// Add to network by reading the compose config. struct Compose { - path: PathBuf, test_dir: PathBuf, env: Environment, #[cfg(unix)] config: ComposeConfig, network: Option, + temp_file: NamedTempFile } impl Compose { fn new(test_dir: PathBuf, env: Environment, network: Option) -> Result> { - let path: PathBuf = [&test_dir, Path::new("compose-temp.yaml")].iter().collect(); + let path: PathBuf = [&test_dir, Path::new("compose.yaml")].iter().collect(); + match path.try_exists() { Err(error) => Err(error).with_context(|| format!("Could not lookup {path:?}")), Ok(false) => Ok(None), Ok(true) => { #[cfg(unix)] - let config = ComposeConfig::parse(&path)?; + let mut config = ComposeConfig::parse(&path)?; + + // Inject the networks block + if let Some(network) = network.clone() { + config.networks.insert( + "default".to_string(), + BTreeMap::from_iter([("name".to_string(), network)]), + ); + } + + // Create a named tempfile, there may be resource leakage here in case of SIGINT + // Tried tempfile::tempfile() but this returns a File object without a usable path + // https://docs.rs/tempfile/latest/tempfile/#resource-leaking + let temp_file = Builder::new() + .prefix("compose-temp-") + .suffix(".yaml") + .tempfile() + .with_context(|| "Failed to create temporary compose file")?; + + fs::write( + temp_file.path(), + serde_yaml::to_string(&config) + .with_context(|| "Failed to serialize modified compose.yml".to_string())?, + )?; println!( "[jonathanpv:debugging]\nUsing compose file: {:?}, found in: {:?}", - config, path + config, temp_file.path() ); Ok(Some(Self { - path, + temp_file, test_dir, env, #[cfg(unix)] @@ -204,7 +209,7 @@ impl Compose { command.push("-compose"); let mut command = Command::new(command); command.arg("--file"); - command.arg(&self.path); + command.arg(&self.temp_file.path()); command.args(args); command.current_dir(&self.test_dir); From a9df5915c05bd979f8ca7f8e8f3b98e0b6039a01 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Mon, 3 Apr 2023 13:25:21 -0400 Subject: [PATCH 08/21] chore(vdev): Make network non optional in Compose struct --- vdev/src/testing/integration.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index e98844d5e07d4..5e3bcfd0d741b 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -35,7 +35,8 @@ impl IntegrationTest { let Some(env_config) = config.environments().get(&environment).map(Clone::clone) else { bail!("Could not find environment named {environment:?}"); }; - let compose = Compose::new(test_dir, env_config.clone(), Some(network_name.clone()))?; + let network_name = format!("vector-integration-tests-{integration}"); + let compose = Compose::new(test_dir, env_config.clone(), network_name.clone())?; let runner = IntegrationTestRunner::new( integration.clone(), &config.runner, @@ -141,12 +142,12 @@ struct Compose { env: Environment, #[cfg(unix)] config: ComposeConfig, - network: Option, + network: String, temp_file: NamedTempFile } impl Compose { - fn new(test_dir: PathBuf, env: Environment, network: Option) -> Result> { + fn new(test_dir: PathBuf, env: Environment, network: String) -> Result> { let path: PathBuf = [&test_dir, Path::new("compose.yaml")].iter().collect(); match path.try_exists() { @@ -215,9 +216,8 @@ impl Compose { command.current_dir(&self.test_dir); command.env("DOCKER_SOCKET", &*DOCKER_SOCKET); - if let Some(network_name) = &self.network { - command.env(NETWORK_ENV_VAR, network_name); - } + command.env(NETWORK_ENV_VAR, &self.network); + for (key, value) in &self.env { if let Some(value) = value { command.env(key, value); From e1bef448cb8cf0a856ed6f360b32f088f67e1789 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Mon, 3 Apr 2023 14:11:15 -0400 Subject: [PATCH 09/21] Merge via cherry-pick network name PR, cargo fmt --- vdev/src/testing/config.rs | 2 +- vdev/src/testing/integration.rs | 23 ++++++++++------------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/vdev/src/testing/config.rs b/vdev/src/testing/config.rs index 8881cf67f1844..f4f69c0e5eda5 100644 --- a/vdev/src/testing/config.rs +++ b/vdev/src/testing/config.rs @@ -91,7 +91,7 @@ pub struct HealthCheck { #[derive(Debug, Deserialize, Serialize)] pub struct BuildConfig { #[serde(default, skip_serializing_if = "Option::is_none")] - pub context: Option + pub context: Option, } #[derive(Debug, Deserialize, Serialize)] diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index 5e3bcfd0d741b..5e895c697e2ef 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -1,7 +1,7 @@ +use anyhow::{bail, Context, Result}; use std::collections::BTreeMap; use std::{fs, path::Path, path::PathBuf, process::Command}; use tempfile::{Builder, NamedTempFile}; -use anyhow::{bail, Context, Result}; #[cfg(unix)] use super::config::ComposeConfig; @@ -29,8 +29,6 @@ impl IntegrationTest { let integration = integration.into(); let environment = environment.into(); let (test_dir, config) = IntegrationTestConfig::load(&integration)?; - - let network_name = format!("vector-integration-tests-{integration}"); let envs_dir = EnvsDir::new(&integration); let Some(env_config) = config.environments().get(&environment).map(Clone::clone) else { bail!("Could not find environment named {environment:?}"); @@ -143,7 +141,7 @@ struct Compose { #[cfg(unix)] config: ComposeConfig, network: String, - temp_file: NamedTempFile + temp_file: NamedTempFile, } impl Compose { @@ -158,12 +156,10 @@ impl Compose { let mut config = ComposeConfig::parse(&path)?; // Inject the networks block - if let Some(network) = network.clone() { - config.networks.insert( - "default".to_string(), - BTreeMap::from_iter([("name".to_string(), network)]), - ); - } + config.networks.insert( + "default".to_string(), + BTreeMap::from_iter([("name".to_string(), network.clone())]), + ); // Create a named tempfile, there may be resource leakage here in case of SIGINT // Tried tempfile::tempfile() but this returns a File object without a usable path @@ -181,15 +177,16 @@ impl Compose { )?; println!( "[jonathanpv:debugging]\nUsing compose file: {:?}, found in: {:?}", - config, temp_file.path() + config, + temp_file.path() ); Ok(Some(Self { - temp_file, test_dir, env, #[cfg(unix)] config, network, + temp_file, })) } } @@ -210,7 +207,7 @@ impl Compose { command.push("-compose"); let mut command = Command::new(command); command.arg("--file"); - command.arg(&self.temp_file.path()); + command.arg(self.temp_file.path()); command.args(args); command.current_dir(&self.test_dir); From 317da52edc9881cc39e7c08061e248bac7606b13 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Mon, 3 Apr 2023 14:14:16 -0400 Subject: [PATCH 10/21] Remove unneeded comments --- vdev/src/testing/config.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/vdev/src/testing/config.rs b/vdev/src/testing/config.rs index f4f69c0e5eda5..b3e288ec63303 100644 --- a/vdev/src/testing/config.rs +++ b/vdev/src/testing/config.rs @@ -37,8 +37,6 @@ impl RustToolchainConfig { } } -// Added the networks key, to match the structure of networks eg -// networks: default: name: value: #[derive(Debug, Deserialize, Serialize)] pub struct ComposeConfig { pub services: BTreeMap, From 76a9b974c89b5235230a74a962f4bb9e4e4afdf6 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Mon, 3 Apr 2023 17:01:40 -0400 Subject: [PATCH 11/21] Reintroduce path in Compose struct, clean up comments --- vdev/src/testing/integration.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index 5e895c697e2ef..bf98c95db6d78 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -136,6 +136,7 @@ impl IntegrationTest { } struct Compose { + path: PathBuf, test_dir: PathBuf, env: Environment, #[cfg(unix)] @@ -175,12 +176,9 @@ impl Compose { serde_yaml::to_string(&config) .with_context(|| "Failed to serialize modified compose.yml".to_string())?, )?; - println!( - "[jonathanpv:debugging]\nUsing compose file: {:?}, found in: {:?}", - config, - temp_file.path() - ); + Ok(Some(Self { + path, test_dir, env, #[cfg(unix)] @@ -206,8 +204,19 @@ impl Compose { let mut command = CONTAINER_TOOL.clone(); command.push("-compose"); let mut command = Command::new(command); - command.arg("--file"); - command.arg(self.temp_file.path()); + // When the integration test environment is already active, the tempfile path does not + // exist because `Compose::new()` has not been called. In this case, the `stop` command + // needs to use the calculated path from the integration name instead of the nonexistent + // tempfile path. This is because `stop` doesn't go through the same logic as `start` + // and doesn't create a new tempfile before calling docker compose. + if config.is_none() { + command.arg("--file"); + command.arg(&self.path); + } else { + command.arg("--file"); + command.arg(self.temp_file.path()); + } + command.args(args); command.current_dir(&self.test_dir); From 54aacd01a8b922daf44467783793486d559934ff Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Tue, 4 Apr 2023 11:23:58 -0400 Subject: [PATCH 12/21] add cargo.toml --- Cargo.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 7dec5a7b23325..02643e8164c42 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9300,7 +9300,6 @@ dependencies = [ "serde_yaml 0.9.19", "sha2 0.10.6", "tempfile", - "tempfile", "toml 0.7.3", ] From 1a64739e6b6b7adbe481b88c657bcdd3af08026b Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Tue, 4 Apr 2023 12:54:27 -0400 Subject: [PATCH 13/21] Utilize tempfile_in() to fix relative file path resolution of compose files --- vdev/src/testing/integration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index bf98c95db6d78..912cc06ae9a67 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -168,7 +168,7 @@ impl Compose { let temp_file = Builder::new() .prefix("compose-temp-") .suffix(".yaml") - .tempfile() + .tempfile_in(&test_dir) .with_context(|| "Failed to create temporary compose file")?; fs::write( From cc03e3c640532133de58c2c4753292ff5f0766a5 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Tue, 4 Apr 2023 13:39:16 -0400 Subject: [PATCH 14/21] Restore proxy network on nginx integration test --- scripts/integration/nginx/compose.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/integration/nginx/compose.yaml b/scripts/integration/nginx/compose.yaml index ea4d734d12e0b..ddedd8eb2e05e 100644 --- a/scripts/integration/nginx/compose.yaml +++ b/scripts/integration/nginx/compose.yaml @@ -20,3 +20,6 @@ services: - ../../../tests/data/nginx/:/etc/nginx:ro networks: - proxy + +networks: + proxy: {} From ba1f01f0c0a28f01b233150e378f42db64ff9db4 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Tue, 4 Apr 2023 13:42:29 -0400 Subject: [PATCH 15/21] Make Compose run command command use the original file path if environment is active --- vdev/src/testing/integration.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index 912cc06ae9a67..af1f0698d0216 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -136,7 +136,7 @@ impl IntegrationTest { } struct Compose { - path: PathBuf, + original_path: PathBuf, test_dir: PathBuf, env: Environment, #[cfg(unix)] @@ -147,14 +147,14 @@ struct Compose { impl Compose { fn new(test_dir: PathBuf, env: Environment, network: String) -> Result> { - let path: PathBuf = [&test_dir, Path::new("compose.yaml")].iter().collect(); + let original_path: PathBuf = [&test_dir, Path::new("compose.yaml")].iter().collect(); - match path.try_exists() { - Err(error) => Err(error).with_context(|| format!("Could not lookup {path:?}")), + match original_path.try_exists() { + Err(error) => Err(error).with_context(|| format!("Could not lookup {original_path:?}")), Ok(false) => Ok(None), Ok(true) => { #[cfg(unix)] - let mut config = ComposeConfig::parse(&path)?; + let mut config = ComposeConfig::parse(&original_path)?; // Inject the networks block config.networks.insert( @@ -178,7 +178,7 @@ impl Compose { )?; Ok(Some(Self { - path, + original_path, test_dir, env, #[cfg(unix)] @@ -209,11 +209,11 @@ impl Compose { // needs to use the calculated path from the integration name instead of the nonexistent // tempfile path. This is because `stop` doesn't go through the same logic as `start` // and doesn't create a new tempfile before calling docker compose. + // If stop command needs to use some of the injected bits then we need to rebuild it + command.arg("--file"); if config.is_none() { - command.arg("--file"); - command.arg(&self.path); + command.arg(&self.original_path); } else { - command.arg("--file"); command.arg(self.temp_file.path()); } From e69623b88e665c528e31baf643188c6cd9325dc6 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Wed, 5 Apr 2023 01:20:42 -0400 Subject: [PATCH 16/21] Conditional compile for Unix, prevent windows CI failure --- vdev/src/testing/integration.rs | 46 +++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index af1f0698d0216..d6c6ff7afed1f 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -155,27 +155,32 @@ impl Compose { Ok(true) => { #[cfg(unix)] let mut config = ComposeConfig::parse(&original_path)?; + let temp_file: NamedTempFile; - // Inject the networks block - config.networks.insert( - "default".to_string(), - BTreeMap::from_iter([("name".to_string(), network.clone())]), - ); - - // Create a named tempfile, there may be resource leakage here in case of SIGINT - // Tried tempfile::tempfile() but this returns a File object without a usable path - // https://docs.rs/tempfile/latest/tempfile/#resource-leaking - let temp_file = Builder::new() - .prefix("compose-temp-") - .suffix(".yaml") - .tempfile_in(&test_dir) - .with_context(|| "Failed to create temporary compose file")?; - - fs::write( - temp_file.path(), - serde_yaml::to_string(&config) - .with_context(|| "Failed to serialize modified compose.yml".to_string())?, - )?; + #[cfg(unix)] + { + // Inject the networks block + config.networks.insert( + "default".to_string(), + BTreeMap::from_iter([("name".to_string(), network.clone())]), + ); + + // Create a named tempfile, there may be resource leakage here in case of SIGINT + // Tried tempfile::tempfile() but this returns a File object without a usable path + // https://docs.rs/tempfile/latest/tempfile/#resource-leaking + temp_file = Builder::new() + .prefix("compose-temp-") + .suffix(".yaml") + .tempfile_in(&test_dir) + .with_context(|| "Failed to create temporary compose file")?; + + fs::write( + temp_file.path(), + serde_yaml::to_string(&config).with_context(|| { + "Failed to serialize modified compose.yml".to_string() + })?, + )?; + } Ok(Some(Self { original_path, @@ -184,6 +189,7 @@ impl Compose { #[cfg(unix)] config, network, + #[cfg(unix)] temp_file, })) } From 6a7e226c95e90ccd1c1a1202cf74e588727cafda Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Wed, 5 Apr 2023 02:54:05 -0400 Subject: [PATCH 17/21] Condtional compilation fix --- vdev/src/testing/integration.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index d6c6ff7afed1f..045fa3e39434d 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -1,7 +1,12 @@ use anyhow::{bail, Context, Result}; -use std::collections::BTreeMap; -use std::{fs, path::Path, path::PathBuf, process::Command}; -use tempfile::{Builder, NamedTempFile}; +use std::{path::Path, path::PathBuf, process::Command}; + +#[cfg(unix)] +use { + std::collections::BTreeMap, + std::fs, + tempfile::{Builder, NamedTempFile}, +}; #[cfg(unix)] use super::config::ComposeConfig; @@ -142,6 +147,7 @@ struct Compose { #[cfg(unix)] config: ComposeConfig, network: String, + #[cfg(unix)] temp_file: NamedTempFile, } @@ -155,6 +161,8 @@ impl Compose { Ok(true) => { #[cfg(unix)] let mut config = ComposeConfig::parse(&original_path)?; + + #[cfg(unix)] let temp_file: NamedTempFile; #[cfg(unix)] @@ -220,6 +228,7 @@ impl Compose { if config.is_none() { command.arg(&self.original_path); } else { + #[cfg(unix)] command.arg(self.temp_file.path()); } From 612e1e04fd31d8e296b58016873930601f6674ce Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Wed, 5 Apr 2023 13:35:15 -0400 Subject: [PATCH 18/21] Using Value for fields not used --- vdev/src/testing/config.rs | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/vdev/src/testing/config.rs b/vdev/src/testing/config.rs index b3e288ec63303..e7bedd7093805 100644 --- a/vdev/src/testing/config.rs +++ b/vdev/src/testing/config.rs @@ -55,7 +55,7 @@ pub struct ComposeService { #[serde(default, skip_serializing_if = "Option::is_none")] pub container_name: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub build: Option, + pub build: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub command: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -69,27 +69,7 @@ pub struct ComposeService { #[serde(default, skip_serializing_if = "Option::is_none")] pub depends_on: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] - pub healthcheck: Option, -} - -#[derive(Debug, Deserialize, Serialize)] -pub struct HealthCheck { - #[serde(default, skip_serializing_if = "Option::is_none")] - pub test: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub interval: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub timeout: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub retries: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub start_period: Option, -} - -#[derive(Debug, Deserialize, Serialize)] -pub struct BuildConfig { - #[serde(default, skip_serializing_if = "Option::is_none")] - pub context: Option, + pub healthcheck: Option, } #[derive(Debug, Deserialize, Serialize)] @@ -100,7 +80,6 @@ pub enum Command { } impl ComposeConfig { - #[cfg(unix)] pub fn parse(path: &Path) -> Result { let contents = fs::read_to_string(path).with_context(|| format!("failed to read {path:?}"))?; From eacf0270fc84f8b3bcad4da8d66e863fa754fcbf Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Wed, 5 Apr 2023 13:38:42 -0400 Subject: [PATCH 19/21] Remove conditional compilation flags to ensure temp_file created on Windows --- vdev/src/testing/integration.rs | 66 ++++++++++++--------------------- 1 file changed, 23 insertions(+), 43 deletions(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index 045fa3e39434d..d7144f47acfa5 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -1,14 +1,8 @@ -use anyhow::{bail, Context, Result}; -use std::{path::Path, path::PathBuf, process::Command}; +use std::{collections::BTreeMap, fs, path::Path, path::PathBuf, process::Command}; -#[cfg(unix)] -use { - std::collections::BTreeMap, - std::fs, - tempfile::{Builder, NamedTempFile}, -}; +use anyhow::{bail, Context, Result}; +use tempfile::{Builder, NamedTempFile}; -#[cfg(unix)] use super::config::ComposeConfig; use super::config::{Environment, IntegrationTestConfig}; use super::runner::{ @@ -144,10 +138,8 @@ struct Compose { original_path: PathBuf, test_dir: PathBuf, env: Environment, - #[cfg(unix)] config: ComposeConfig, network: String, - #[cfg(unix)] temp_file: NamedTempFile, } @@ -159,45 +151,34 @@ impl Compose { Err(error) => Err(error).with_context(|| format!("Could not lookup {original_path:?}")), Ok(false) => Ok(None), Ok(true) => { - #[cfg(unix)] let mut config = ComposeConfig::parse(&original_path)?; - - #[cfg(unix)] - let temp_file: NamedTempFile; - - #[cfg(unix)] - { - // Inject the networks block - config.networks.insert( - "default".to_string(), - BTreeMap::from_iter([("name".to_string(), network.clone())]), - ); - - // Create a named tempfile, there may be resource leakage here in case of SIGINT - // Tried tempfile::tempfile() but this returns a File object without a usable path - // https://docs.rs/tempfile/latest/tempfile/#resource-leaking - temp_file = Builder::new() - .prefix("compose-temp-") - .suffix(".yaml") - .tempfile_in(&test_dir) - .with_context(|| "Failed to create temporary compose file")?; - - fs::write( - temp_file.path(), - serde_yaml::to_string(&config).with_context(|| { - "Failed to serialize modified compose.yml".to_string() - })?, - )?; - } + // Inject the networks block + config.networks.insert( + "default".to_string(), + BTreeMap::from_iter([("name".to_string(), network.clone())]), + ); + + // Create a named tempfile, there may be resource leakage here in case of SIGINT + // Tried tempfile::tempfile() but this returns a File object without a usable path + // https://docs.rs/tempfile/latest/tempfile/#resource-leaking + let temp_file = Builder::new() + .prefix("compose-temp-") + .suffix(".yaml") + .tempfile_in(&test_dir) + .with_context(|| "Failed to create temporary compose file")?; + + fs::write( + temp_file.path(), + serde_yaml::to_string(&config) + .with_context(|| "Failed to serialize modified compose.yml".to_string())?, + )?; Ok(Some(Self { original_path, test_dir, env, - #[cfg(unix)] config, network, - #[cfg(unix)] temp_file, })) } @@ -228,7 +209,6 @@ impl Compose { if config.is_none() { command.arg(&self.original_path); } else { - #[cfg(unix)] command.arg(self.temp_file.path()); } From cd1e26770c7f4205bbaa288dd5e8dcecc11fb4c9 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Thu, 6 Apr 2023 11:43:16 +0000 Subject: [PATCH 20/21] Allow deadcode for windows --- vdev/src/testing/integration.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index d7144f47acfa5..ea20a267de3fd 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -138,6 +138,7 @@ struct Compose { original_path: PathBuf, test_dir: PathBuf, env: Environment, + #[cfg_attr(target_family = "windows", allow(dead_code))] config: ComposeConfig, network: String, temp_file: NamedTempFile, From 4b0260efedabc1587805c293349756879d06a183 Mon Sep 17 00:00:00 2001 From: jonathanpv Date: Thu, 6 Apr 2023 12:25:27 -0400 Subject: [PATCH 21/21] Remove unneeded to_string() --- vdev/src/testing/integration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vdev/src/testing/integration.rs b/vdev/src/testing/integration.rs index ea20a267de3fd..ecac804a98842 100644 --- a/vdev/src/testing/integration.rs +++ b/vdev/src/testing/integration.rs @@ -171,7 +171,7 @@ impl Compose { fs::write( temp_file.path(), serde_yaml::to_string(&config) - .with_context(|| "Failed to serialize modified compose.yml".to_string())?, + .with_context(|| "Failed to serialize modified compose.yaml")?, )?; Ok(Some(Self {