Skip to content

OCPNODE-3062: Add container stop signal feature (KEP-4960) #9086

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

Merged
merged 2 commits into from
Apr 8, 2025
Merged

OCPNODE-3062: Add container stop signal feature (KEP-4960) #9086

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
7c4fbad
Add container stop signal feature (KEP-4960)
bitoku Mar 27, 2025
6a4a3ee
test
bitoku Mar 29, 2025
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
5 changes: 5 additions & 0 deletions contrib/test/ci/integration-main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
include_tasks: "build/kata.yml"
when: "build_kata | default(False) | bool"

Copy link
Member

@haircommander haircommander Apr 9, 2025

Choose a reason for hiding this comment

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

now that we've upated the cri-tools in the VM image we should undo this change so we continue to use the (new) cached version

Copy link
Contributor Author

@bitoku bitoku Apr 9, 2025

Choose a reason for hiding this comment

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

I'm just curious why do we want to remove the building cri-tools step?

Copy link
Member

@haircommander haircommander Apr 9, 2025

Choose a reason for hiding this comment

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

we do it as part of setup which runs as a periodic job that we then cache the images for. basically: we update the images daily instead of doing it each ci run. The upstream doesn't change enough to warrant rebuilding each PR and spending all that download/build time.

Copy link
Contributor Author

@bitoku bitoku Apr 10, 2025

Choose a reason for hiding this comment

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

@haircommander
aha, I should've waited for a while. thanks!

Copy link
Member

@haircommander haircommander Apr 10, 2025

Choose a reason for hiding this comment

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

no this is fine to force a rebuild temporarily, no worries 🙂

- name: clone build and install cri-tools
include_tasks: "build/cri-tools.yml"
vars:
force_clone: true

- name: build and install cri-o
include_tasks: "build/cri-o.yml"

Expand Down
13 changes: 7 additions & 6 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ require (
github.com/opencontainers/runtime-spec v1.2.1
github.com/opencontainers/runtime-tools v0.9.1-0.20241108202711-f7e3563b0271
github.com/opencontainers/selinux v1.12.0
github.com/prometheus/client_golang v1.21.1
github.com/prometheus/client_golang v1.22.0-rc.0
github.com/seccomp/libseccomp-golang v0.10.0
github.com/sirupsen/logrus v1.9.3
github.com/soheilhy/cmux v0.1.5
Expand All @@ -76,8 +76,8 @@ require (
k8s.io/api v0.32.3
k8s.io/apimachinery v0.32.3
k8s.io/client-go v0.32.3
k8s.io/cri-api v0.33.0-beta.0.0.20250313010358-ab383b81657e
k8s.io/cri-client v0.32.3
k8s.io/cri-api v0.33.0-beta.0.0.20250324233632-87ee4e17aba6
k8s.io/cri-client v0.31.0-alpha.0.0.20250321074236-bc20b67fdb8f
k8s.io/klog/v2 v2.130.1
k8s.io/kubelet v0.32.3
k8s.io/utils v0.0.0-20241210054802-24370beab758
Expand Down Expand Up @@ -144,7 +144,7 @@ require (
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 // indirect
github.com/google/gnostic-models v0.6.9 // indirect
github.com/google/go-containerregistry v0.20.3 // indirect
github.com/google/go-github/v60 v60.0.0 // indirect
github.com/google/go-intervals v0.0.2 // indirect
Expand Down Expand Up @@ -240,8 +240,9 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/apiserver v0.32.3 // indirect
k8s.io/component-base v0.32.3 // indirect
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f // indirect
k8s.io/kube-openapi v0.0.0-20250318190949-c8a335a9a2ff // indirect
sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.4.2 // indirect
sigs.k8s.io/randfill v1.0.0 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.6.0 // indirect
tags.cncf.io/container-device-interface/specs-go v1.0.0 // indirect
)
27 changes: 15 additions & 12 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw
github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 h1:0VpGH+cDhbDtdcweoyCVsF3fhN8kejK6rFe/2FFX2nU=
github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49/go.mod h1:BkkQ4L1KS1xMt2aWSPStnn55ChGC0DPOn2FQYj+f25M=
github.com/google/gnostic-models v0.6.9 h1:MU/8wDLif2qCXZmzncUQ/BOfxWfthHi63KqpoNbWqVw=
github.com/google/gnostic-models v0.6.9/go.mod h1:CiWsm0s6BSQd1hRn8/QmxqB6BesYcbSZxsz9b0KuDBw=
github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
Expand Down Expand Up @@ -412,8 +412,8 @@ github.com/proglottis/gpgme v0.1.4/go.mod h1:5LoXMgpE4bttgwwdv9bLs/vwqv3qV7F4glE
github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo=
github.com/prometheus/client_golang v1.1.0/go.mod h1:I1FGZT9+L76gKKOs5djB6ezCbFQP1xR9D75/vuwEF3g=
github.com/prometheus/client_golang v1.21.1 h1:DOvXXTqVzvkIewV/CDPFdejpMCGeMcbGCQ8YOmu+Ibk=
github.com/prometheus/client_golang v1.21.1/go.mod h1:U9NM32ykUErtVBxdvD3zfi+EuFkkaBvMb09mIfe0Zgg=
github.com/prometheus/client_golang v1.22.0-rc.0 h1:meoqLyZIVEIiQxZmyVTOnzk/bA+n2pN2MXN8pSzX2ws=
github.com/prometheus/client_golang v1.22.0-rc.0/go.mod h1:R7ljNsLXhuQXYZYtw6GAE9AZg8Y7vEW5scdCXrWRXC0=
github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
Expand Down Expand Up @@ -777,26 +777,29 @@ k8s.io/client-go v0.32.3 h1:RKPVltzopkSgHS7aS98QdscAgtgah/+zmpAogooIqVU=
k8s.io/client-go v0.32.3/go.mod h1:3v0+3k4IcT9bXTc4V2rt+d2ZPPG700Xy6Oi0Gdl2PaY=
k8s.io/component-base v0.32.3 h1:98WJvvMs3QZ2LYHBzvltFSeJjEx7t5+8s71P7M74u8k=
k8s.io/component-base v0.32.3/go.mod h1:LWi9cR+yPAv7cu2X9rZanTiFKB2kHA+JjmhkKjCZRpI=
k8s.io/cri-api v0.33.0-beta.0.0.20250313010358-ab383b81657e h1:E5kmNomg/jOO2v58iqYjUQSYsFSYpglEK5MjnxenJ08=
k8s.io/cri-api v0.33.0-beta.0.0.20250313010358-ab383b81657e/go.mod h1:AWeYLzfWgDAsuMDuL4Cdv4QN6w8I38Skhl7VL5Kt88Y=
k8s.io/cri-client v0.32.3 h1:+D2ajlFpXsUcr/9ofYcE5kVqVK4Q97wnZHeH80oDEzw=
k8s.io/cri-client v0.32.3/go.mod h1:W1+Z8QsVnLkoGqtJ41B5SRHfQn6/mqGORdfNDl2cEkw=
k8s.io/cri-api v0.33.0-beta.0.0.20250324233632-87ee4e17aba6 h1:91EJUQoR6rcMaSANiMMqvOdkkkC6vO0YPrUzD04dAe8=
k8s.io/cri-api v0.33.0-beta.0.0.20250324233632-87ee4e17aba6/go.mod h1:AWeYLzfWgDAsuMDuL4Cdv4QN6w8I38Skhl7VL5Kt88Y=
k8s.io/cri-client v0.31.0-alpha.0.0.20250321074236-bc20b67fdb8f h1:mAVvu6TLYvnIuJZXbsmb1ABX8TBrpJ0F5byH1wyjo/o=
k8s.io/cri-client v0.31.0-alpha.0.0.20250321074236-bc20b67fdb8f/go.mod h1:0BiO2DF4FAuvZwvu65LVv6OlcOCOGCGK4/XpSOR8OqI=
k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f h1:GA7//TjRY9yWGy1poLzYYJJ4JRdzg3+O6e8I+e+8T5Y=
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f/go.mod h1:R/HEjbvWI0qdfb8viZUeVZm0X6IZnxAydC7YU42CMw4=
k8s.io/kube-openapi v0.0.0-20250318190949-c8a335a9a2ff h1:/usPimJzUKKu+m+TE36gUyGcf03XZEP0ZIKgKj35LS4=
k8s.io/kube-openapi v0.0.0-20250318190949-c8a335a9a2ff/go.mod h1:5jIi+8yX4RIb8wk3XwBo5Pq2ccx4FP10ohkbSKCZoK8=
k8s.io/kubelet v0.32.3 h1:B9HzW4yB67flx8tN2FYuDwZvxnmK3v5EjxxFvOYjmc8=
k8s.io/kubelet v0.32.3/go.mod h1:yyAQSCKC+tjSlaFw4HQG7Jein+vo+GeKBGdXdQGvL1U=
k8s.io/utils v0.0.0-20241210054802-24370beab758 h1:sdbE21q2nlQtFh65saZY+rRM6x6aJJI8IUa1AmH/qa0=
k8s.io/utils v0.0.0-20241210054802-24370beab758/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 h1:/Rv+M11QRah1itp8VhT6HoVx1Ray9eB4DBr+K+/sCJ8=
sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3/go.mod h1:18nIHnGi6636UCz6m8i4DhaJ65T6EruyzmoQqI2BVDo=
sigs.k8s.io/randfill v0.0.0-20250304075658-069ef1bbf016/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY=
sigs.k8s.io/randfill v1.0.0 h1:JfjMILfT8A6RbawdsK2JXGBR5AQVfd+9TbzrlneTyrU=
sigs.k8s.io/randfill v1.0.0/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY=
sigs.k8s.io/release-sdk v0.12.2 h1:ncuHwUu8VWcZVVrNkjoUR8xGo6ibHg+AM6uMMD+IwuQ=
sigs.k8s.io/release-sdk v0.12.2/go.mod h1:tlJgWPJLeRbWOvcyq1XrCZmLe8Yfn3H5U/LNtmBa0Nc=
sigs.k8s.io/release-utils v0.11.1 h1:hzvXGpHgHJfLOJB6TRuu14bzWc3XEglHmXHJqwClSZE=
sigs.k8s.io/release-utils v0.11.1/go.mod h1:ybR2V/uQAOGxYfzYtBenSYeXWkBGNP2qnEiX77ACtpc=
sigs.k8s.io/structured-merge-diff/v4 v4.4.2 h1:MdmvkGuXi/8io6ixD5wud3vOLwc1rj0aNqRlpuvjmwA=
sigs.k8s.io/structured-merge-diff/v4 v4.4.2/go.mod h1:N8f93tFZh9U6vpxwRArLiikrE5/2tiu1w1AGfACIGE4=
sigs.k8s.io/structured-merge-diff/v4 v4.6.0 h1:IUA9nvMmnKWcj5jl84xn+T5MnlZKThmUW1TdblaLVAc=
sigs.k8s.io/structured-merge-diff/v4 v4.6.0/go.mod h1:dDy58f92j70zLsuZVuUX5Wp9vtxXpaZnkPGWeqDfCps=
sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY=
tags.cncf.io/container-device-interface v1.0.1 h1:KqQDr4vIlxwfYh0Ed/uJGVgX+CHAkahrgabg6Q8GYxc=
Expand Down
2 changes: 1 addition & 1 deletion internal/factory/container/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ func (c *container) SpecAddAnnotations(ctx context.Context, sb SandboxIFace, con

if configStopSignal != "" {
// this key is defined in image-spec conversion document at https://github.com/opencontainers/image-spec/pull/492/files#diff-8aafbe2c3690162540381b8cdb157112R57
c.spec.AddAnnotation("org.opencontainers.image.stopSignal", configStopSignal)
c.spec.AddAnnotation(annotations.StopSignalAnnotation, configStopSignal)
}

return nil
Expand Down
4 changes: 2 additions & 2 deletions internal/lib/container_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (c *ContainerServer) LoadSandbox(ctx context.Context, id string) (sb *sandb
}

if !wasSpoofed {
scontainer, err = oci.NewContainer(m.Annotations[annotations.ContainerID], cname, sandboxPath, m.Annotations[annotations.LogPath], labels, m.Annotations, kubeAnnotations, m.Annotations[annotations.UserRequestedImage], nil, nil, "", nil, id, false, false, false, sb.RuntimeHandler(), sandboxDir, created, m.Annotations["org.opencontainers.image.stopSignal"])
scontainer, err = oci.NewContainer(m.Annotations[annotations.ContainerID], cname, sandboxPath, m.Annotations[annotations.LogPath], labels, m.Annotations, kubeAnnotations, m.Annotations[annotations.UserRequestedImage], nil, nil, "", nil, id, false, false, false, sb.RuntimeHandler(), sandboxDir, created, m.Annotations[annotations.StopSignalAnnotation])
if err != nil {
return sb, err
}
Expand Down Expand Up @@ -524,7 +524,7 @@ func (c *ContainerServer) LoadContainer(ctx context.Context, id string) (retErr
return err
}

ctr, err := oci.NewContainer(id, name, containerPath, m.Annotations[annotations.LogPath], labels, m.Annotations, kubeAnnotations, userRequestedImage, someNameOfTheImage, imageID, "", &metadata, sb.ID(), tty, stdin, stdinOnce, sb.RuntimeHandler(), containerDir, created, m.Annotations["org.opencontainers.image.stopSignal"])
ctr, err := oci.NewContainer(id, name, containerPath, m.Annotations[annotations.LogPath], labels, m.Annotations, kubeAnnotations, userRequestedImage, someNameOfTheImage, imageID, "", &metadata, sb.ID(), tty, stdin, stdinOnce, sb.RuntimeHandler(), containerDir, created, m.Annotations[annotations.StopSignalAnnotation])
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/oci/runtime_oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ func (r *runtimeOCI) StopLoopForContainer(c *Container, bm kwait.BackoffManager)
}

case <-time.After(time.Until(targetTime)):
log.Warnf(ctx, "Stopping container %s with stop signal timed out. Killing...", c.ID())
log.Warnf(ctx, "Stopping container %s with stop signal(%s) timed out. Killing...", c.ID(), c.GetStopSignal())

goto killContainer

Expand Down
4 changes: 4 additions & 0 deletions pkg/annotations/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ const (

// DisableFIPSAnnotation is used to disable FIPS mode for a pod within a FIPS-enabled Kubernetes cluster.
DisableFIPSAnnotation = "io.kubernetes.cri-o.DisableFIPS"

// StopSignalAnnotation represents the stop signal used for the image
// this key is defined in image-spec conversion document at https://github.com/opencontainers/image-spec/pull/492/files#diff-8aafbe2c3690162540381b8cdb157112R57
StopSignalAnnotation = "org.opencontainers.image.stopSignal"
)

var AllAllowedAnnotations = []string{
Expand Down
15 changes: 13 additions & 2 deletions server/container_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,18 @@
return nil, err
}

err = ctr.SpecAddAnnotations(ctx, sb, containerVolumes, mountPoint, containerImageConfig.Config.StopSignal, imgResult, s.config.CgroupManager().IsSystemd(), seccompRef, runtimePath)
// Determine the stop signal for the container. If a custom stop signal is provided
// via CRI API, use it. Otherwise, fall back to the image's default stop signal as
// defined in its configuration.
// https://github.com/kubernetes/enhancements/issues/4960
stopSignal := containerImageConfig.Config.StopSignal

if signal := ctr.Config().GetStopSignal(); signal != types.Signal_RUNTIME_DEFAULT {
log.Debugf(ctx, "Override stop signal to %s", signal)
stopSignal = signal.String()
}

Check warning on line 1096 in server/container_create.go

View check run for this annotation

Codecov / codecov/patch

server/container_create.go#L1094-L1096 

Added lines #L1094 - L1096 were not covered by tests

err = ctr.SpecAddAnnotations(ctx, sb, containerVolumes, mountPoint, stopSignal, imgResult, s.config.CgroupManager().IsSystemd(), seccompRef, runtimePath)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1220,7 +1231,7 @@
Attempt: metadata.Attempt,
}

ociContainer, err := oci.NewContainer(containerID, containerName, containerInfo.RunDir, logPath, labels, crioAnnotations, ctr.Config().Annotations, userRequestedImage, someNameOfTheImage, &imageID, someRepoDigest, criMetadata, sb.ID(), containerConfig.Tty, containerConfig.Stdin, containerConfig.StdinOnce, sb.RuntimeHandler(), containerInfo.Dir, created, containerImageConfig.Config.StopSignal)
ociContainer, err := oci.NewContainer(containerID, containerName, containerInfo.RunDir, logPath, labels, crioAnnotations, ctr.Config().Annotations, userRequestedImage, someNameOfTheImage, &imageID, someRepoDigest, criMetadata, sb.ID(), containerConfig.Tty, containerConfig.Stdin, containerConfig.StdinOnce, sb.RuntimeHandler(), containerInfo.Dir, created, stopSignal)
if err != nil {
return nil, err
}
Expand Down
3 changes: 1 addition & 2 deletions server/sandbox_run_freebsd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,7 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ
g.AddAnnotation(annotations.HostNetwork, fmt.Sprintf("%v", hostNetwork))
g.AddAnnotation(annotations.ContainerManager, constants.ContainerManagerCRIO)
if podContainer.Config.Config.StopSignal != "" {
// this key is defined in image-spec conversion document at https://github.com/opencontainers/image-spec/pull/492/files#diff-8aafbe2c3690162540381b8cdb157112R57
g.AddAnnotation("org.opencontainers.image.stopSignal", podContainer.Config.Config.StopSignal)
g.AddAnnotation(annotations.StopSignalAnnotation, podContainer.Config.Config.StopSignal)
}

if s.config.CgroupManager().IsSystemd() && node.SystemdHasCollectMode() {
Expand Down
3 changes: 1 addition & 2 deletions server/sandbox_run_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,8 +771,7 @@
g.AddAnnotation(annotations.ContainerManager, constants.ContainerManagerCRIO)

if podContainer.Config.Config.StopSignal != "" {
// this key is defined in image-spec conversion document at https://github.com/opencontainers/image-spec/pull/492/files#diff-8aafbe2c3690162540381b8cdb157112R57
g.AddAnnotation("org.opencontainers.image.stopSignal", podContainer.Config.Config.StopSignal)
g.AddAnnotation(annotations.StopSignalAnnotation, podContainer.Config.Config.StopSignal)

Check warning on line 774 in server/sandbox_run_linux.go

View check run for this annotation

Codecov / codecov/patch

server/sandbox_run_linux.go#L774 

Added line #L774 was not covered by tests
}

created := time.Now()
Expand Down
15 changes: 15 additions & 0 deletions server/sandbox_update_resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package server

import (
"context"
"errors"

types "k8s.io/cri-api/pkg/apis/runtime/v1"
)

// UpdatePodSandboxResources updates Config of the pod sandbox.
func (s *Server) UpdatePodSandboxResources(ctx context.Context, req *types.UpdatePodSandboxResourcesRequest) (*types.UpdatePodSandboxResourcesResponse, error) {
// TODO: implement this function
// https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/1287-in-place-update-pod-resources/README.md#cri-changes
return nil, errors.New("not implemented yet")

Check warning on line 14 in server/sandbox_update_resources.go

View check run for this annotation

Codecov / codecov/patch

server/sandbox_update_resources.go#L11-L14 

Added lines #L11 - L14 were not covered by tests
Comment on lines +12 to +14
Copy link
Member

@saschagrunert saschagrunert Apr 7, 2025

Choose a reason for hiding this comment

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

Are you going to follow-up on that or how should we handle it?

Copy link
Contributor Author

@bitoku bitoku Apr 7, 2025

Choose a reason for hiding this comment

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

I can follow up.
Since the KEP is going to be beta in 1.33, we should implement this anyway, right?
I'll create a JIRA ticket about it.

Copy link
Contributor Author

@bitoku bitoku Apr 7, 2025

Choose a reason for hiding this comment

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

https://issues.redhat.com/browse/OCPNODE-3122

}
43 changes: 42 additions & 1 deletion test/ctr.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1504,7 +1504,7 @@ EOF
CRICTL_TIMEOUT=10m crictl stop -t 10 "$ctr_id"
crictl rmp -f "$pod_id"

grep -q "Stopping container ${ctr_id} with stop signal timed out." "$CRIO_LOG"
grep -q "Stopping container ${ctr_id} with stop signal(15) timed out." "$CRIO_LOG"

readarray -t attempts < "$FAKE_RUNTIME_ATTEMPTS_LOG"

Expand Down Expand Up @@ -1561,3 +1561,44 @@ EOF
# verify that at least a default masked path exists
crictl inspect "$ctr_id" | jq -e '.info.runtimeSpec.linux.maskedPaths | index("/proc/acpi")'
}

@test "container stops with default SIGTERM stop signal" {
start_crio

# Start a container that traps SIGTERM and writes to a file when received
jq '.command = ["sh", "-c", "trap '"'"'echo SIGTERM; exit 0'"'"' TERM; while true; do sleep 1; done"]' \
"$TESTDATA"/container_config.json > "$TESTDIR/container_config.json"
ctr_id=$(crictl run "$TESTDIR/container_config.json" "$TESTDATA/sandbox_config.json")
crictl inspect "$ctr_id"

# Stop the container
crictl stop -t 3 "$ctr_id"

# Verify container exited with status 0
output=$(crictl inspect "$ctr_id" | jq -r '.status.state')
[[ "$output" == "CONTAINER_EXITED" ]]
output=$(run crictl inspect "$ctr_id" | jq -r '.status.exitCode')
[[ "$output" == "0" ]]
}

@test "container stops with custom SIGINT stop signal" {
start_crio

crictl --version
# Start a container that traps SIGTERM and writes to a file when received
jq '.command = ["sh", "-c", "trap '"'"'echo SIGINT; exit 0'"'"' INT; while true; do sleep 1; done"] |
.stop_signal = 10' \
"$TESTDATA"/container_config.json > "$TESTDIR/container_config.json"
ctr_id=$(crictl run "$TESTDIR/container_config.json" "$TESTDATA/sandbox_config.json")
grep -q "Override stop signal to SIGINT" "$CRIO_LOG"

crictl inspect "$ctr_id"
# Stop the container
crictl stop -t 3 "$ctr_id"

# Verify container exited with status 0
output=$(crictl inspect "$ctr_id" | jq -r '.status.state')
[[ "$output" == "CONTAINER_EXITED" ]]
output=$(run crictl inspect "$ctr_id" | jq -r '.status.exitCode')
[[ "$output" == "0" ]]
}
Loading
Loading