-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
driver: add support for configuring extra_hosts in podman tasks #255
Conversation
This PR adds support for setting extra_hosts on in podman task configuration, bringing the podman driver support up to par with the docker driver. Closes #373
if cleanupErr := d.podman.ContainerDelete(d.ctx, containerID, true, true); cleanupErr != nil { | ||
d.logger.Error("failed to clean up from an error in Start", "error", cleanupErr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fun fact - this would overwrite err
during the cleanup()
call, causing what is now startErr
to always be nil
at the time it was logged, which was beyond confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof..nice catch!
Spot check, job file job "sleep" {
group "group" {
count = 1
task "sleep" {
driver = "podman"
config {
image = "docker.io/bash:5"
args = ["sleep", "infinity"]
extra_hosts = ["test4.localhost:127.0.0.2", "test6.localhost:[::1]"]
}
resources {
cores = 2
memory = 64
}
}
}
} exec into the container and cat
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just needs a CHANGELOG entry 😄
if cleanupErr := d.podman.ContainerDelete(d.ctx, containerID, true, true); cleanupErr != nil { | ||
d.logger.Error("failed to clean up from an error in Start", "error", cleanupErr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof..nice catch!
This PR adds support for setting extra_hosts on in podman task configuration,
bringing the podman driver support up to par with the docker driver.
Closes #244