Skip to content
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.

Sign up for GitHub

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

Set StopTimeout for compat API if not set by client #19704

Merged
merged 2 commits into from
Aug 25, 2023
Merged

Set StopTimeout for compat API if not set by client #19704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2ec127
Set StopTimeout for compat API if not set by client
rhatdan Aug 22, 2023
f32e954
Set StopTimeout for service-container started under podman kube play
rhatdan Aug 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/api/handlers/compat/containers_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ func cliOpts(cc handlers.CreateContainerConfig, rtc *config.Config) (*entities.C
Rm: cc.HostConfig.AutoRemove,
SecurityOpt: cc.HostConfig.SecurityOpt,
StopSignal: cc.Config.StopSignal,
StopTimeout: rtc.Engine.StopTimeout, // podman default
StorageOpts: stringMaptoArray(cc.HostConfig.StorageOpt),
Sysctl: stringMaptoArray(cc.HostConfig.Sysctls),
Systemd: "true", // podman default
Expand Down
7 changes: 6 additions & 1 deletion pkg/domain/infra/abi/play.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ func (ic *ContainerEngine) createServiceContainer(ctx context.Context, name stri
return nil, fmt.Errorf("image for service container: %w", err)
}

rtc, err := ic.Libpod.GetConfigNoCopy()
if err != nil {
return nil, err
}
ctrOpts := entities.ContainerCreateOptions{
// Inherited from infra containers
ImageVolume: define.TypeBind,
Expand All @@ -73,7 +77,8 @@ func (ic *ContainerEngine) createServiceContainer(ctx context.Context, name stri
ReadOnly: true,
ReadWriteTmpFS: false,
// No need to spin up slirp etc.
Net: &entities.NetOptions{Network: specgen.Namespace{NSMode: specgen.NoNetwork}},
Net: &entities.NetOptions{Network: specgen.Namespace{NSMode: specgen.NoNetwork}},
StopTimeout: rtc.Engine.StopTimeout,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reporter actually caught it.

Copy link
Member Author

@rhatdan rhatdan Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@InvalidInterrupt PTAL

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about it without a fried brain: I am not sure the stop timeout makes that much sense for a service container. There is no workload inside other than catatonit. But catatonit handles SIGTERM just fine so I don't expect any measurable negative performance impact on shut down.

}

// Create and fill out the runtime spec.
Expand Down
1 change: 1 addition & 0 deletions test/apiv2/20-containers.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ cid_top=$(jq -r '.Id' <<<"$output")
t GET containers/${cid_top}/json 200 \
.Config.Entrypoint[0]="top" \
.Config.Cmd='[]' \
.Config.StopTimeout="10" \
.Path="top" \
.NetworkSettings.Networks.podman.NetworkID=podman
t POST containers/${cid_top}/start 204
Expand Down
3 changes: 3 additions & 0 deletions test/system/700-play.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ EOF
run_podman container inspect $service_container --format "{{.State.Running}}"
is "$output" "true"

run_podman container inspect $service_container --format '{{.Config.StopTimeout}}'
is "$output" "10" "StopTimeout should be initialized to 10"

# Stop the *main* container and make sure that
# 1) The pod transitions to Exited
# 2) The service container is stopped
Expand Down