mirror of
https://github.com/zed-industries/zed.git
synced 2026-08-21 06:54:45 +00:00
dev_containers: Preserve Compose entrypoints by default (#61897)
# Objective Docker Compose-based dev containers do not run their image or service entrypoint when `overrideCommand` is omitted. The Dev Container specification defaults `overrideCommand` to `true` for image and Dockerfile configurations, but to `false` for Docker Compose configurations. Zed treated an omitted value as `true` for every build type and generated a Compose entrypoint override, preventing the configured entrypoint from running. Fixes #60456 ## Solution Resolve the effective value of `overrideCommand` based on the dev container build type: - Preserve the existing default of `true` for image and Dockerfile configurations. - Use the specification default of `false` for Docker Compose configurations. - Continue to give an explicitly configured `overrideCommand` precedence over the default. Use the resolved value when deciding whether to generate Zed's entrypoint script. Add regression tests covering the defaults for image, Dockerfile, and Docker Compose configurations, along with explicit overrides in both directions. ## Testing Automated testing: - `cargo fmt --check` - `cargo test -p dev_container override_command --lib` - `./script/clippy -p dev_container --lib` - `cargo build -p zed` All three focused tests pass, and Clippy passes with warnings denied. Manually tested on Linux using the Docker Compose reproduction from #60456: 1. Built an Alpine image with an `ENTRYPOINT` that writes `/tmp/entrypoint-marker.log`. 2. Configured the Compose service with `command: sleep infinity`. 3. Omitted `overrideCommand` from `devcontainer.json`. 4. Opened the project in the current stable release and confirmed that the marker file was not created. 5. Opened a fresh container using this branch's development build and confirmed that: - `/tmp/entrypoint-marker.log` was created. - The Compose `sleep infinity` command remained active. Testing was performed with Docker Compose on Linux. I did not manually test this change on macOS or Windows, but the default resolution is platform-independent. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase ### Before Current stable Zed replaces the Compose entrypoint when `overrideCommand` is omitted, so `/tmp/entrypoint-marker.log` is not created. Zed 1.12.0 <img width="2827" height="1709" alt="image" src="https://github.com/user-attachments/assets/04dadd66-5e95-4ea8-8183-6156793f0654" /> ### After The development build preserves the Compose entrypoint by default. The marker file is created and the Compose service command remains active. <img width="2827" height="1709" alt="image" src="https://github.com/user-attachments/assets/cdd9808b-7867-42d8-bec7-97af9c170b0a" /> --- Release Notes: - Fixed Docker Compose dev containers not running their configured entrypoints by default.
This commit is contained in:
parent
e99616cdd4
commit
b7de76402c
2 changed files with 44 additions and 4 deletions
|
|
@ -278,6 +278,13 @@ impl DevContainer {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn override_command(&self) -> bool {
|
||||
self.override_command.unwrap_or(!matches!(
|
||||
self.build_type(),
|
||||
DevContainerBuildType::DockerCompose
|
||||
))
|
||||
}
|
||||
|
||||
pub(crate) fn validate_devcontainer_contents(&self) -> Result<(), DevContainerError> {
|
||||
match self.build_type() {
|
||||
DevContainerBuildType::Image(_) => Ok(()),
|
||||
|
|
@ -634,6 +641,39 @@ mod test {
|
|||
},
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn override_command_defaults_depend_on_build_type() {
|
||||
let image = deserialize_devcontainer_json(r#"{"image":"ubuntu"}"#).expect("image config");
|
||||
assert!(image.override_command());
|
||||
|
||||
let dockerfile = deserialize_devcontainer_json(r#"{"build":{"dockerfile":"Dockerfile"}}"#)
|
||||
.expect("Dockerfile config");
|
||||
assert!(dockerfile.override_command());
|
||||
|
||||
let compose = deserialize_devcontainer_json(
|
||||
r#"{"dockerComposeFile":"compose.yaml","service":"app"}"#,
|
||||
)
|
||||
.expect("Compose config");
|
||||
assert!(!compose.override_command());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_override_command_takes_precedence_over_build_type_default() {
|
||||
let image = deserialize_devcontainer_json(r#"{"image":"ubuntu","overrideCommand":false}"#)
|
||||
.expect("image config");
|
||||
assert!(!image.override_command());
|
||||
|
||||
let compose = deserialize_devcontainer_json(
|
||||
r#"{
|
||||
"dockerComposeFile":"compose.yaml",
|
||||
"service":"app",
|
||||
"overrideCommand":true
|
||||
}"#,
|
||||
)
|
||||
.expect("Compose config");
|
||||
assert!(compose.override_command());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn should_deserialize_customizations_with_unknown_keys() {
|
||||
let json_with_other_customizations = r#"
|
||||
|
|
|
|||
|
|
@ -877,9 +877,7 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${{PATH:-\3}}/g' /etc/profile || true
|
|||
push_unique_string(&mut security_opt, &opt);
|
||||
}
|
||||
|
||||
let entrypoint_script = if dev_container.override_command == Some(false) {
|
||||
None
|
||||
} else {
|
||||
let entrypoint_script = if dev_container.override_command() {
|
||||
let mut entrypoint_script_lines = vec![
|
||||
"echo Container started".to_string(),
|
||||
"trap \"exit 0\" 15".to_string(),
|
||||
|
|
@ -894,6 +892,8 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${{PATH:-\3}}/g' /etc/profile || true
|
|||
]);
|
||||
|
||||
Some(entrypoint_script_lines.join("\n").trim().to_string())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let mut container_env = HashMap::new();
|
||||
|
|
@ -5150,7 +5150,7 @@ ENV DOCKER_BUILDKIT=1
|
|||
Some(vec!["seccomp=unconfined".to_string()])
|
||||
);
|
||||
assert_eq!(app_service.privileged, Some(true));
|
||||
assert!(app_service.entrypoint.is_some());
|
||||
assert!(app_service.entrypoint.is_none());
|
||||
|
||||
let labels = app_service.labels.as_ref().unwrap();
|
||||
assert_eq!(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue