Fix: Add explicit host entries to container configuration

[mazdak]

Type of Change

• Bug fix

Motivation and Context

While building a Docker Compose-like plugin for container and validating it against our real Docker Compose workload, we hit a core limitation: there was no way for callers to ask the runtime to append explicit entries to a container's /etc/hosts.

That showed up most clearly with Compose extra_hosts, especially the common host.docker.internal pattern. The plugin could parse those mappings, but there was no core field to carry them into the sandbox, so the runtime always generated only the default localhost/container-name entries.

In practice, containers that depended on host aliases still failed name resolution even though the compose file specified them.

Why this belongs in core

This is not something the plugin can fake safely. /etc/hosts is generated in the sandbox layer, so callers need a first-class way to provide additional host entries to the runtime.

What this changes

• add ContainerConfiguration.HostEntry
• add ContainerConfiguration.hosts
• preserve those entries through configuration encoding/decoding
• extend sandbox host generation to append caller-provided host entries after the default localhost and primary hostname entries
• add focused tests for round-tripping and host resolution behavior

Testing

• Tested locally
• Added/updated tests

[JaewonHur]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

[vxnkjyffmq-code]

We are splitting by / here because the value coming in as primaryAddress is not a plain IP address — it is a CIDR notation string (e.g. "192.168.1.45/24").
The /24 part is the subnet mask. We only want the actual IP address (192.168.1.45), so we split on the / and take the first part.
Cleaned-up & safer version of that function:

extension SandboxService {
static func resolvedHosts(
hostname: String,
primaryAddress: String?,
extraHosts: [ContainerConfiguration.HostEntry]
) -> [ContainerConfiguration.HostEntry] {

 var hosts: [ContainerConfiguration.HostEntry] = [
ContainerConfiguration.HostEntry(ipAddress: "127.0.0.1", hostnames: ["localhost"])
]
if let primaryAddress {
// Split off the CIDR suffix if present (e.g. "192.168.1.45/24" → "192.168.1.45")
let ipOnly = primaryAddress.split(separator: "/").first.map(String.init) ?? primaryAddress
hosts.append(
ContainerConfiguration.HostEntry(
ipAddress: ipOnly,
hostnames: [hostname]
)
)
}
// Add any extra hosts passed in
hosts.append(contentsOf: extraHosts)
return hosts
}

}

[vxnkjyffmq-code]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

Also with the way I am running low latency it must be a steady string no breaking if possible.! I can show you diagrams or math that proves it.

[JaewonHur]

It'd be good to have a follow up PR that wires this to CLI (e.g., --add-host [name:ip] in Docker).

[JaewonHur]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

[mazdak]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

Apologies for the late reply. Done.

[stephenlclarke]

Thanks for implementing this as a data-shape-only PR. I reviewed the patch from the downstream stephenlclarke/container-compose side, where the direct need is Compose depends_on.condition: service_healthy.

This looks like the right first slice to me:

• HealthStatus and optional ContainerSnapshot.health give API clients a stable type without forcing the observer design into this PR.
• Keeping the value nil until a runtime observer exists is a useful compatibility boundary for older servers/newer clients and for current daemon behavior.
• The split keeps Compose policy in stephenlclarke/container-compose, which is where service dependency waiting, selected services, and user-facing Compose errors should live.

I would suggest adding a small Codable compatibility test before merge. This PR is adding a new API field, even though the daemon does not populate it yet, so the important thing to protect is the wire/API shape:

• old ContainerSnapshot JSON without health should still decode successfully;
• all HealthStatus values should encode and decode correctly;
• existing code that creates a ContainerSnapshot should still default health to nil.

I would also make the meaning of nil versus .none very clear. nil should mean "health information is not available from this daemon/snapshot." .none should mean "health information is available, and the container has no healthcheck or no result yet." That difference will matter for downstream tools like stephenlclarke/container-compose.

For follow-up work, I would keep the healthcheck pieces split into small PRs:

1. add the health field and HealthStatus type (this PR);
2. add a way to configure a container healthcheck;
3. add the daemon code that runs the healthcheck and updates ContainerSnapshot.health;
4. add container create / container run flags for healthchecks;
5. add support for reading Dockerfile HEALTHCHECK settings from images.

I do not think those need to be included in this PR. This PR is easier to review if it only adds the shared API field. That gives stephenlclarke/container-compose and other clients something stable to build against, while keeping Compose-specific behaviour out of apple/container.

[mazdak]

Type of Change

• Bug fix

Motivation and Context

While building a Docker Compose-like plugin for container and validating it against our real Docker Compose workload, we hit a core limitation: there was no way for callers to ask the runtime to append explicit entries to a container's /etc/hosts.

That showed up most clearly with Compose extra_hosts, especially the common host.docker.internal pattern. The plugin could parse those mappings, but there was no core field to carry them into the sandbox, so the runtime always generated only the default localhost/container-name entries.

In practice, containers that depended on host aliases still failed name resolution even though the compose file specified them.

Why this belongs in core

This is not something the plugin can fake safely. /etc/hosts is generated in the sandbox layer, so callers need a first-class way to provide additional host entries to the runtime.

What this changes

• add ContainerConfiguration.HostEntry
• add ContainerConfiguration.hosts
• preserve those entries through configuration encoding/decoding
• extend sandbox host generation to append caller-provided host entries after the default localhost and primary hostname entries
• add focused tests for round-tripping and host resolution behavior

Testing

• Tested locally
• Added/updated tests

[JaewonHur]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

[vxnkjyffmq-code]

We are splitting by / here because the value coming in as primaryAddress is not a plain IP address — it is a CIDR notation string (e.g. "192.168.1.45/24").
The /24 part is the subnet mask. We only want the actual IP address (192.168.1.45), so we split on the / and take the first part.
Cleaned-up & safer version of that function:

extension SandboxService {
static func resolvedHosts(
hostname: String,
primaryAddress: String?,
extraHosts: [ContainerConfiguration.HostEntry]
) -> [ContainerConfiguration.HostEntry] {

 var hosts: [ContainerConfiguration.HostEntry] = [
ContainerConfiguration.HostEntry(ipAddress: "127.0.0.1", hostnames: ["localhost"])
]
if let primaryAddress {
// Split off the CIDR suffix if present (e.g. "192.168.1.45/24" → "192.168.1.45")
let ipOnly = primaryAddress.split(separator: "/").first.map(String.init) ?? primaryAddress
hosts.append(
ContainerConfiguration.HostEntry(
ipAddress: ipOnly,
hostnames: [hostname]
)
)
}
// Add any extra hosts passed in
hosts.append(contentsOf: extraHosts)
return hosts
}

}

[vxnkjyffmq-code]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

Also with the way I am running low latency it must be a steady string no breaking if possible.! I can show you diagrams or math that proves it.

[JaewonHur]

It'd be good to have a follow up PR that wires this to CLI (e.g., --add-host [name:ip] in Docker).

[JaewonHur]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

[mazdak]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

Apologies for the late reply. Done.

[stephenlclarke]

Thanks for implementing this as a data-shape-only PR. I reviewed the patch from the downstream stephenlclarke/container-compose side, where the direct need is Compose depends_on.condition: service_healthy.

This looks like the right first slice to me:

• HealthStatus and optional ContainerSnapshot.health give API clients a stable type without forcing the observer design into this PR.
• Keeping the value nil until a runtime observer exists is a useful compatibility boundary for older servers/newer clients and for current daemon behavior.
• The split keeps Compose policy in stephenlclarke/container-compose, which is where service dependency waiting, selected services, and user-facing Compose errors should live.

I would suggest adding a small Codable compatibility test before merge. This PR is adding a new API field, even though the daemon does not populate it yet, so the important thing to protect is the wire/API shape:

• old ContainerSnapshot JSON without health should still decode successfully;
• all HealthStatus values should encode and decode correctly;
• existing code that creates a ContainerSnapshot should still default health to nil.

I would also make the meaning of nil versus .none very clear. nil should mean "health information is not available from this daemon/snapshot." .none should mean "health information is available, and the container has no healthcheck or no result yet." That difference will matter for downstream tools like stephenlclarke/container-compose.

For follow-up work, I would keep the healthcheck pieces split into small PRs:

1. add the health field and HealthStatus type (this PR);
2. add a way to configure a container healthcheck;
3. add the daemon code that runs the healthcheck and updates ContainerSnapshot.health;
4. add container create / container run flags for healthchecks;
5. add support for reading Dockerfile HEALTHCHECK settings from images.

I do not think those need to be included in this PR. This PR is easier to review if it only adds the shared API field. That gives stephenlclarke/container-compose and other clients something stable to build against, while keeping Compose-specific behaviour out of apple/container.

[mazdak]

Type of Change

• Bug fix

Motivation and Context

While building a Docker Compose-like plugin for container and validating it against our real Docker Compose workload, we hit a core limitation: there was no way for callers to ask the runtime to append explicit entries to a container's /etc/hosts.

That showed up most clearly with Compose extra_hosts, especially the common host.docker.internal pattern. The plugin could parse those mappings, but there was no core field to carry them into the sandbox, so the runtime always generated only the default localhost/container-name entries.

In practice, containers that depended on host aliases still failed name resolution even though the compose file specified them.

Why this belongs in core

This is not something the plugin can fake safely. /etc/hosts is generated in the sandbox layer, so callers need a first-class way to provide additional host entries to the runtime.

What this changes

• add ContainerConfiguration.HostEntry
• add ContainerConfiguration.hosts
• preserve those entries through configuration encoding/decoding
• extend sandbox host generation to append caller-provided host entries after the default localhost and primary hostname entries
• add focused tests for round-tripping and host resolution behavior

Testing

• Tested locally
• Added/updated tests

[JaewonHur]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

[vxnkjyffmq-code]

We are splitting by / here because the value coming in as primaryAddress is not a plain IP address — it is a CIDR notation string (e.g. "192.168.1.45/24").
The /24 part is the subnet mask. We only want the actual IP address (192.168.1.45), so we split on the / and take the first part.
Cleaned-up & safer version of that function:

extension SandboxService {
static func resolvedHosts(
hostname: String,
primaryAddress: String?,
extraHosts: [ContainerConfiguration.HostEntry]
) -> [ContainerConfiguration.HostEntry] {

 var hosts: [ContainerConfiguration.HostEntry] = [
ContainerConfiguration.HostEntry(ipAddress: "127.0.0.1", hostnames: ["localhost"])
]
if let primaryAddress {
// Split off the CIDR suffix if present (e.g. "192.168.1.45/24" → "192.168.1.45")
let ipOnly = primaryAddress.split(separator: "/").first.map(String.init) ?? primaryAddress
hosts.append(
ContainerConfiguration.HostEntry(
ipAddress: ipOnly,
hostnames: [hostname]
)
)
}
// Add any extra hosts passed in
hosts.append(contentsOf: extraHosts)
return hosts
}

}

[vxnkjyffmq-code]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

Also with the way I am running low latency it must be a steady string no breaking if possible.! I can show you diagrams or math that proves it.

[JaewonHur]

It'd be good to have a follow up PR that wires this to CLI (e.g., --add-host [name:ip] in Docker).

[JaewonHur]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

[mazdak]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

Apologies for the late reply. Done.

[stephenlclarke]

Thanks for implementing this as a data-shape-only PR. I reviewed the patch from the downstream stephenlclarke/container-compose side, where the direct need is Compose depends_on.condition: service_healthy.

This looks like the right first slice to me:

• HealthStatus and optional ContainerSnapshot.health give API clients a stable type without forcing the observer design into this PR.
• Keeping the value nil until a runtime observer exists is a useful compatibility boundary for older servers/newer clients and for current daemon behavior.
• The split keeps Compose policy in stephenlclarke/container-compose, which is where service dependency waiting, selected services, and user-facing Compose errors should live.

I would suggest adding a small Codable compatibility test before merge. This PR is adding a new API field, even though the daemon does not populate it yet, so the important thing to protect is the wire/API shape:

• old ContainerSnapshot JSON without health should still decode successfully;
• all HealthStatus values should encode and decode correctly;
• existing code that creates a ContainerSnapshot should still default health to nil.

I would also make the meaning of nil versus .none very clear. nil should mean "health information is not available from this daemon/snapshot." .none should mean "health information is available, and the container has no healthcheck or no result yet." That difference will matter for downstream tools like stephenlclarke/container-compose.

For follow-up work, I would keep the healthcheck pieces split into small PRs:

1. add the health field and HealthStatus type (this PR);
2. add a way to configure a container healthcheck;
3. add the daemon code that runs the healthcheck and updates ContainerSnapshot.health;
4. add container create / container run flags for healthchecks;
5. add support for reading Dockerfile HEALTHCHECK settings from images.

I do not think those need to be included in this PR. This PR is easier to review if it only adds the shared API field. That gives stephenlclarke/container-compose and other clients something stable to build against, while keeping Compose-specific behaviour out of apple/container.

[mazdak]

Type of Change

• Bug fix

Motivation and Context

While building a Docker Compose-like plugin for container and validating it against our real Docker Compose workload, we hit a core limitation: there was no way for callers to ask the runtime to append explicit entries to a container's /etc/hosts.

That showed up most clearly with Compose extra_hosts, especially the common host.docker.internal pattern. The plugin could parse those mappings, but there was no core field to carry them into the sandbox, so the runtime always generated only the default localhost/container-name entries.

In practice, containers that depended on host aliases still failed name resolution even though the compose file specified them.

Why this belongs in core

This is not something the plugin can fake safely. /etc/hosts is generated in the sandbox layer, so callers need a first-class way to provide additional host entries to the runtime.

What this changes

• add ContainerConfiguration.HostEntry
• add ContainerConfiguration.hosts
• preserve those entries through configuration encoding/decoding
• extend sandbox host generation to append caller-provided host entries after the default localhost and primary hostname entries
• add focused tests for round-tripping and host resolution behavior

Testing

• Tested locally
• Added/updated tests

[JaewonHur]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

[vxnkjyffmq-code]

We are splitting by / here because the value coming in as primaryAddress is not a plain IP address — it is a CIDR notation string (e.g. "192.168.1.45/24").
The /24 part is the subnet mask. We only want the actual IP address (192.168.1.45), so we split on the / and take the first part.
Cleaned-up & safer version of that function:

extension SandboxService {
static func resolvedHosts(
hostname: String,
primaryAddress: String?,
extraHosts: [ContainerConfiguration.HostEntry]
) -> [ContainerConfiguration.HostEntry] {

 var hosts: [ContainerConfiguration.HostEntry] = [
ContainerConfiguration.HostEntry(ipAddress: "127.0.0.1", hostnames: ["localhost"])
]
if let primaryAddress {
// Split off the CIDR suffix if present (e.g. "192.168.1.45/24" → "192.168.1.45")
let ipOnly = primaryAddress.split(separator: "/").first.map(String.init) ?? primaryAddress
hosts.append(
ContainerConfiguration.HostEntry(
ipAddress: ipOnly,
hostnames: [hostname]
)
)
}
// Add any extra hosts passed in
hosts.append(contentsOf: extraHosts)
return hosts
}

}

[vxnkjyffmq-code]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

Also with the way I am running low latency it must be a steady string no breaking if possible.! I can show you diagrams or math that proves it.

[JaewonHur]

It'd be good to have a follow up PR that wires this to CLI (e.g., --add-host [name:ip] in Docker).

[JaewonHur]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

[mazdak]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

Apologies for the late reply. Done.

[stephenlclarke]

Thanks for implementing this as a data-shape-only PR. I reviewed the patch from the downstream stephenlclarke/container-compose side, where the direct need is Compose depends_on.condition: service_healthy.

This looks like the right first slice to me:

• HealthStatus and optional ContainerSnapshot.health give API clients a stable type without forcing the observer design into this PR.
• Keeping the value nil until a runtime observer exists is a useful compatibility boundary for older servers/newer clients and for current daemon behavior.
• The split keeps Compose policy in stephenlclarke/container-compose, which is where service dependency waiting, selected services, and user-facing Compose errors should live.

I would suggest adding a small Codable compatibility test before merge. This PR is adding a new API field, even though the daemon does not populate it yet, so the important thing to protect is the wire/API shape:

• old ContainerSnapshot JSON without health should still decode successfully;
• all HealthStatus values should encode and decode correctly;
• existing code that creates a ContainerSnapshot should still default health to nil.

I would also make the meaning of nil versus .none very clear. nil should mean "health information is not available from this daemon/snapshot." .none should mean "health information is available, and the container has no healthcheck or no result yet." That difference will matter for downstream tools like stephenlclarke/container-compose.

For follow-up work, I would keep the healthcheck pieces split into small PRs:

1. add the health field and HealthStatus type (this PR);
2. add a way to configure a container healthcheck;
3. add the daemon code that runs the healthcheck and updates ContainerSnapshot.health;
4. add container create / container run flags for healthchecks;
5. add support for reading Dockerfile HEALTHCHECK settings from images.

I do not think those need to be included in this PR. This PR is easier to review if it only adds the shared API field. That gives stephenlclarke/container-compose and other clients something stable to build against, while keeping Compose-specific behaviour out of apple/container.

[mazdak]

Type of Change

• Bug fix

Motivation and Context

While building a Docker Compose-like plugin for container and validating it against our real Docker Compose workload, we hit a core limitation: there was no way for callers to ask the runtime to append explicit entries to a container's /etc/hosts.

That showed up most clearly with Compose extra_hosts, especially the common host.docker.internal pattern. The plugin could parse those mappings, but there was no core field to carry them into the sandbox, so the runtime always generated only the default localhost/container-name entries.

In practice, containers that depended on host aliases still failed name resolution even though the compose file specified them.

Why this belongs in core

This is not something the plugin can fake safely. /etc/hosts is generated in the sandbox layer, so callers need a first-class way to provide additional host entries to the runtime.

What this changes

• add ContainerConfiguration.HostEntry
• add ContainerConfiguration.hosts
• preserve those entries through configuration encoding/decoding
• extend sandbox host generation to append caller-provided host entries after the default localhost and primary hostname entries
• add focused tests for round-tripping and host resolution behavior

Testing

• Tested locally
• Added/updated tests

[JaewonHur]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

[vxnkjyffmq-code]

We are splitting by / here because the value coming in as primaryAddress is not a plain IP address — it is a CIDR notation string (e.g. "192.168.1.45/24").
The /24 part is the subnet mask. We only want the actual IP address (192.168.1.45), so we split on the / and take the first part.
Cleaned-up & safer version of that function:

extension SandboxService {
static func resolvedHosts(
hostname: String,
primaryAddress: String?,
extraHosts: [ContainerConfiguration.HostEntry]
) -> [ContainerConfiguration.HostEntry] {

 var hosts: [ContainerConfiguration.HostEntry] = [
ContainerConfiguration.HostEntry(ipAddress: "127.0.0.1", hostnames: ["localhost"])
]
if let primaryAddress {
// Split off the CIDR suffix if present (e.g. "192.168.1.45/24" → "192.168.1.45")
let ipOnly = primaryAddress.split(separator: "/").first.map(String.init) ?? primaryAddress
hosts.append(
ContainerConfiguration.HostEntry(
ipAddress: ipOnly,
hostnames: [hostname]
)
)
}
// Add any extra hosts passed in
hosts.append(contentsOf: extraHosts)
return hosts
}

}

[vxnkjyffmq-code]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

Also with the way I am running low latency it must be a steady string no breaking if possible.! I can show you diagrams or math that proves it.

[JaewonHur]

It'd be good to have a follow up PR that wires this to CLI (e.g., --add-host [name:ip] in Docker).

[JaewonHur]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

[mazdak]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

Apologies for the late reply. Done.

[stephenlclarke]

Thanks for implementing this as a data-shape-only PR. I reviewed the patch from the downstream stephenlclarke/container-compose side, where the direct need is Compose depends_on.condition: service_healthy.

This looks like the right first slice to me:

• HealthStatus and optional ContainerSnapshot.health give API clients a stable type without forcing the observer design into this PR.
• Keeping the value nil until a runtime observer exists is a useful compatibility boundary for older servers/newer clients and for current daemon behavior.
• The split keeps Compose policy in stephenlclarke/container-compose, which is where service dependency waiting, selected services, and user-facing Compose errors should live.

I would suggest adding a small Codable compatibility test before merge. This PR is adding a new API field, even though the daemon does not populate it yet, so the important thing to protect is the wire/API shape:

• old ContainerSnapshot JSON without health should still decode successfully;
• all HealthStatus values should encode and decode correctly;
• existing code that creates a ContainerSnapshot should still default health to nil.

I would also make the meaning of nil versus .none very clear. nil should mean "health information is not available from this daemon/snapshot." .none should mean "health information is available, and the container has no healthcheck or no result yet." That difference will matter for downstream tools like stephenlclarke/container-compose.

For follow-up work, I would keep the healthcheck pieces split into small PRs:

1. add the health field and HealthStatus type (this PR);
2. add a way to configure a container healthcheck;
3. add the daemon code that runs the healthcheck and updates ContainerSnapshot.health;
4. add container create / container run flags for healthchecks;
5. add support for reading Dockerfile HEALTHCHECK settings from images.

I do not think those need to be included in this PR. This PR is easier to review if it only adds the shared API field. That gives stephenlclarke/container-compose and other clients something stable to build against, while keeping Compose-specific behaviour out of apple/container.

[mazdak]

Type of Change

• Bug fix

Motivation and Context

While building a Docker Compose-like plugin for container and validating it against our real Docker Compose workload, we hit a core limitation: there was no way for callers to ask the runtime to append explicit entries to a container's /etc/hosts.

That showed up most clearly with Compose extra_hosts, especially the common host.docker.internal pattern. The plugin could parse those mappings, but there was no core field to carry them into the sandbox, so the runtime always generated only the default localhost/container-name entries.

In practice, containers that depended on host aliases still failed name resolution even though the compose file specified them.

Why this belongs in core

This is not something the plugin can fake safely. /etc/hosts is generated in the sandbox layer, so callers need a first-class way to provide additional host entries to the runtime.

What this changes

• add ContainerConfiguration.HostEntry
• add ContainerConfiguration.hosts
• preserve those entries through configuration encoding/decoding
• extend sandbox host generation to append caller-provided host entries after the default localhost and primary hostname entries
• add focused tests for round-tripping and host resolution behavior

Testing

• Tested locally
• Added/updated tests

[JaewonHur]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

[vxnkjyffmq-code]

We are splitting by / here because the value coming in as primaryAddress is not a plain IP address — it is a CIDR notation string (e.g. "192.168.1.45/24").
The /24 part is the subnet mask. We only want the actual IP address (192.168.1.45), so we split on the / and take the first part.
Cleaned-up & safer version of that function:

extension SandboxService {
static func resolvedHosts(
hostname: String,
primaryAddress: String?,
extraHosts: [ContainerConfiguration.HostEntry]
) -> [ContainerConfiguration.HostEntry] {

 var hosts: [ContainerConfiguration.HostEntry] = [
ContainerConfiguration.HostEntry(ipAddress: "127.0.0.1", hostnames: ["localhost"])
]
if let primaryAddress {
// Split off the CIDR suffix if present (e.g. "192.168.1.45/24" → "192.168.1.45")
let ipOnly = primaryAddress.split(separator: "/").first.map(String.init) ?? primaryAddress
hosts.append(
ContainerConfiguration.HostEntry(
ipAddress: ipOnly,
hostnames: [hostname]
)
)
}
// Add any extra hosts passed in
hosts.append(contentsOf: extraHosts)
return hosts
}

}

[vxnkjyffmq-code]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

Also with the way I am running low latency it must be a steady string no breaking if possible.! I can show you diagrams or math that proves it.

[JaewonHur]

It'd be good to have a follow up PR that wires this to CLI (e.g., --add-host [name:ip] in Docker).

[JaewonHur]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

[mazdak]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

Apologies for the late reply. Done.

[stephenlclarke]

Thanks for implementing this as a data-shape-only PR. I reviewed the patch from the downstream stephenlclarke/container-compose side, where the direct need is Compose depends_on.condition: service_healthy.

This looks like the right first slice to me:

• HealthStatus and optional ContainerSnapshot.health give API clients a stable type without forcing the observer design into this PR.
• Keeping the value nil until a runtime observer exists is a useful compatibility boundary for older servers/newer clients and for current daemon behavior.
• The split keeps Compose policy in stephenlclarke/container-compose, which is where service dependency waiting, selected services, and user-facing Compose errors should live.

I would suggest adding a small Codable compatibility test before merge. This PR is adding a new API field, even though the daemon does not populate it yet, so the important thing to protect is the wire/API shape:

• old ContainerSnapshot JSON without health should still decode successfully;
• all HealthStatus values should encode and decode correctly;
• existing code that creates a ContainerSnapshot should still default health to nil.

I would also make the meaning of nil versus .none very clear. nil should mean "health information is not available from this daemon/snapshot." .none should mean "health information is available, and the container has no healthcheck or no result yet." That difference will matter for downstream tools like stephenlclarke/container-compose.

For follow-up work, I would keep the healthcheck pieces split into small PRs:

1. add the health field and HealthStatus type (this PR);
2. add a way to configure a container healthcheck;
3. add the daemon code that runs the healthcheck and updates ContainerSnapshot.health;
4. add container create / container run flags for healthchecks;
5. add support for reading Dockerfile HEALTHCHECK settings from images.

I do not think those need to be included in this PR. This PR is easier to review if it only adds the shared API field. That gives stephenlclarke/container-compose and other clients something stable to build against, while keeping Compose-specific behaviour out of apple/container.

[mazdak]

Type of Change

• Bug fix

Motivation and Context

While building a Docker Compose-like plugin for container and validating it against our real Docker Compose workload, we hit a core limitation: there was no way for callers to ask the runtime to append explicit entries to a container's /etc/hosts.

That showed up most clearly with Compose extra_hosts, especially the common host.docker.internal pattern. The plugin could parse those mappings, but there was no core field to carry them into the sandbox, so the runtime always generated only the default localhost/container-name entries.

In practice, containers that depended on host aliases still failed name resolution even though the compose file specified them.

Why this belongs in core

This is not something the plugin can fake safely. /etc/hosts is generated in the sandbox layer, so callers need a first-class way to provide additional host entries to the runtime.

What this changes

• add ContainerConfiguration.HostEntry
• add ContainerConfiguration.hosts
• preserve those entries through configuration encoding/decoding
• extend sandbox host generation to append caller-provided host entries after the default localhost and primary hostname entries
• add focused tests for round-tripping and host resolution behavior

Testing

• Tested locally
• Added/updated tests

[JaewonHur]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

[vxnkjyffmq-code]

We are splitting by / here because the value coming in as primaryAddress is not a plain IP address — it is a CIDR notation string (e.g. "192.168.1.45/24").
The /24 part is the subnet mask. We only want the actual IP address (192.168.1.45), so we split on the / and take the first part.
Cleaned-up & safer version of that function:

extension SandboxService {
static func resolvedHosts(
hostname: String,
primaryAddress: String?,
extraHosts: [ContainerConfiguration.HostEntry]
) -> [ContainerConfiguration.HostEntry] {

 var hosts: [ContainerConfiguration.HostEntry] = [
ContainerConfiguration.HostEntry(ipAddress: "127.0.0.1", hostnames: ["localhost"])
]
if let primaryAddress {
// Split off the CIDR suffix if present (e.g. "192.168.1.45/24" → "192.168.1.45")
let ipOnly = primaryAddress.split(separator: "/").first.map(String.init) ?? primaryAddress
hosts.append(
ContainerConfiguration.HostEntry(
ipAddress: ipOnly,
hostnames: [hostname]
)
)
}
// Add any extra hosts passed in
hosts.append(contentsOf: extraHosts)
return hosts
}

}

[vxnkjyffmq-code]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

Also with the way I am running low latency it must be a steady string no breaking if possible.! I can show you diagrams or math that proves it.

[JaewonHur]

It'd be good to have a follow up PR that wires this to CLI (e.g., --add-host [name:ip] in Docker).

[JaewonHur]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

[mazdak]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

Apologies for the late reply. Done.

[stephenlclarke]

Thanks for implementing this as a data-shape-only PR. I reviewed the patch from the downstream stephenlclarke/container-compose side, where the direct need is Compose depends_on.condition: service_healthy.

This looks like the right first slice to me:

• HealthStatus and optional ContainerSnapshot.health give API clients a stable type without forcing the observer design into this PR.
• Keeping the value nil until a runtime observer exists is a useful compatibility boundary for older servers/newer clients and for current daemon behavior.
• The split keeps Compose policy in stephenlclarke/container-compose, which is where service dependency waiting, selected services, and user-facing Compose errors should live.

I would suggest adding a small Codable compatibility test before merge. This PR is adding a new API field, even though the daemon does not populate it yet, so the important thing to protect is the wire/API shape:

• old ContainerSnapshot JSON without health should still decode successfully;
• all HealthStatus values should encode and decode correctly;
• existing code that creates a ContainerSnapshot should still default health to nil.

I would also make the meaning of nil versus .none very clear. nil should mean "health information is not available from this daemon/snapshot." .none should mean "health information is available, and the container has no healthcheck or no result yet." That difference will matter for downstream tools like stephenlclarke/container-compose.

For follow-up work, I would keep the healthcheck pieces split into small PRs:

1. add the health field and HealthStatus type (this PR);
2. add a way to configure a container healthcheck;
3. add the daemon code that runs the healthcheck and updates ContainerSnapshot.health;
4. add container create / container run flags for healthchecks;
5. add support for reading Dockerfile HEALTHCHECK settings from images.

I do not think those need to be included in this PR. This PR is easier to review if it only adds the shared API field. That gives stephenlclarke/container-compose and other clients something stable to build against, while keeping Compose-specific behaviour out of apple/container.

[mazdak]

Type of Change

• Bug fix

Motivation and Context

While building a Docker Compose-like plugin for container and validating it against our real Docker Compose workload, we hit a core limitation: there was no way for callers to ask the runtime to append explicit entries to a container's /etc/hosts.

That showed up most clearly with Compose extra_hosts, especially the common host.docker.internal pattern. The plugin could parse those mappings, but there was no core field to carry them into the sandbox, so the runtime always generated only the default localhost/container-name entries.

In practice, containers that depended on host aliases still failed name resolution even though the compose file specified them.

Why this belongs in core

This is not something the plugin can fake safely. /etc/hosts is generated in the sandbox layer, so callers need a first-class way to provide additional host entries to the runtime.

What this changes

• add ContainerConfiguration.HostEntry
• add ContainerConfiguration.hosts
• preserve those entries through configuration encoding/decoding
• extend sandbox host generation to append caller-provided host entries after the default localhost and primary hostname entries
• add focused tests for round-tripping and host resolution behavior

Testing

• Tested locally
• Added/updated tests

[JaewonHur]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

[vxnkjyffmq-code]

We are splitting by / here because the value coming in as primaryAddress is not a plain IP address — it is a CIDR notation string (e.g. "192.168.1.45/24").
The /24 part is the subnet mask. We only want the actual IP address (192.168.1.45), so we split on the / and take the first part.
Cleaned-up & safer version of that function:

extension SandboxService {
static func resolvedHosts(
hostname: String,
primaryAddress: String?,
extraHosts: [ContainerConfiguration.HostEntry]
) -> [ContainerConfiguration.HostEntry] {

 var hosts: [ContainerConfiguration.HostEntry] = [
ContainerConfiguration.HostEntry(ipAddress: "127.0.0.1", hostnames: ["localhost"])
]
if let primaryAddress {
// Split off the CIDR suffix if present (e.g. "192.168.1.45/24" → "192.168.1.45")
let ipOnly = primaryAddress.split(separator: "/").first.map(String.init) ?? primaryAddress
hosts.append(
ContainerConfiguration.HostEntry(
ipAddress: ipOnly,
hostnames: [hostname]
)
)
}
// Add any extra hosts passed in
hosts.append(contentsOf: extraHosts)
return hosts
}

}

[vxnkjyffmq-code]

Why are we splitting by / here?

public struct IPv4Address {
@inlinable
public var description: String {
"\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])"
}
}

Also with the way I am running low latency it must be a steady string no breaking if possible.! I can show you diagrams or math that proves it.

[JaewonHur]

It'd be good to have a follow up PR that wires this to CLI (e.g., --add-host [name:ip] in Docker).

[JaewonHur]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

[mazdak]

@mazdak Hi! Could you do make fmt and push again? Sorry for late! We are going to merge this.

Apologies for the late reply. Done.

[stephenlclarke]

Thanks for implementing this as a data-shape-only PR. I reviewed the patch from the downstream stephenlclarke/container-compose side, where the direct need is Compose depends_on.condition: service_healthy.

This looks like the right first slice to me:

• HealthStatus and optional ContainerSnapshot.health give API clients a stable type without forcing the observer design into this PR.
• Keeping the value nil until a runtime observer exists is a useful compatibility boundary for older servers/newer clients and for current daemon behavior.
• The split keeps Compose policy in stephenlclarke/container-compose, which is where service dependency waiting, selected services, and user-facing Compose errors should live.

I would suggest adding a small Codable compatibility test before merge. This PR is adding a new API field, even though the daemon does not populate it yet, so the important thing to protect is the wire/API shape:

• old ContainerSnapshot JSON without health should still decode successfully;
• all HealthStatus values should encode and decode correctly;
• existing code that creates a ContainerSnapshot should still default health to nil.

I would also make the meaning of nil versus .none very clear. nil should mean "health information is not available from this daemon/snapshot." .none should mean "health information is available, and the container has no healthcheck or no result yet." That difference will matter for downstream tools like stephenlclarke/container-compose.

For follow-up work, I would keep the healthcheck pieces split into small PRs:

1. add the health field and HealthStatus type (this PR);
2. add a way to configure a container healthcheck;
3. add the daemon code that runs the healthcheck and updates ContainerSnapshot.health;
4. add container create / container run flags for healthchecks;
5. add support for reading Dockerfile HEALTHCHECK settings from images.

I do not think those need to be included in this PR. This PR is easier to review if it only adds the shared API field. That gives stephenlclarke/container-compose and other clients something stable to build against, while keeping Compose-specific behaviour out of apple/container.