Switch 'ContainerCheckpoint' from Alpha to Beta #123215

adrianreber · 2024-02-09T11:05:46Z

Forensic Container Checkpointing as described in KEP 2008 moves from Alpha to Beta. This is the corresponding code change.

In the KEP following changes for Alpha to Beta graduation were described:

Test to ensure the feature does not work with the feature gate disabled.
Test to ensure the feature does work if enabled.
Test CRI metrics related to ContainerCheckpoint CRI RPC.
Test kubelet metrics related to kubelet checkpoint API endpoint.
Enable the feature per default
Add separate sub-resource permission to control permissions
at https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/server/auth.go#L101-L108
Add necessary metrics as described in the PRR sections and update the KEP with the metrics
names once they exist
- Add CRI metrics (this already exists via kubelet_runtime_operations_errors_total)
- Add kubelet metrics (this already exists under the name checkpoint)

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR does the code changes as discussed in kubernetes/enhancements#4288. Graduating the KEP 2008 "Forensic Container Checkpointing" from Alpha to Beta. All changes have been discussed in the corresponding KEP.

Which issue(s) this PR fixes:

Special notes for your reviewer:

This PR contains the code changes for graduating the KEP 2008 "Forensic Container Checkpointing" from Alpha to Beta.

Does this PR introduce a user-facing change?

Graduated _forensic container checkpointing_ [KEP #2008](https://kep.k8s.io/2008) from Alpha to Beta.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/issues/2008
- [KEP]: https://github.com/kubernetes/enhancements/pull/4288
- [Usage]: https://kubernetes.io/blog/2022/12/05/forensic-container-checkpointing-alpha/
- [Usage]: https://kubernetes.io/blog/2023/03/10/forensic-container-analysis/
- [Other doc]: https://kubernetes.io/docs/reference/node/kubelet-checkpoint-api/

k8s-ci-robot · 2024-02-09T11:13:07Z

Hi @adrianreber. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

kannon92 · 2024-02-09T14:24:22Z

/ok-to-test

kannon92 · 2024-02-09T14:24:38Z

/test

k8s-ci-robot · 2024-02-09T14:24:41Z

@kannon92: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

/test pull-cadvisor-e2e-kubernetes
/test pull-kubernetes-conformance-kind-ga-only-parallel
/test pull-kubernetes-coverage-unit
/test pull-kubernetes-dependencies
/test pull-kubernetes-dependencies-go-canary
/test pull-kubernetes-e2e-gce
/test pull-kubernetes-e2e-gce-100-performance
/test pull-kubernetes-e2e-gce-big-performance
/test pull-kubernetes-e2e-gce-cos
/test pull-kubernetes-e2e-gce-cos-canary
/test pull-kubernetes-e2e-gce-cos-no-stage
/test pull-kubernetes-e2e-gce-network-proxy-http-connect
/test pull-kubernetes-e2e-gce-scale-performance-manual
/test pull-kubernetes-e2e-kind
/test pull-kubernetes-e2e-kind-ipv6
/test pull-kubernetes-integration
/test pull-kubernetes-integration-go-canary
/test pull-kubernetes-kubemark-e2e-gce-scale
/test pull-kubernetes-node-e2e-containerd
/test pull-kubernetes-typecheck
/test pull-kubernetes-unit
/test pull-kubernetes-unit-go-canary
/test pull-kubernetes-update
/test pull-kubernetes-verify
/test pull-kubernetes-verify-go-canary

The following commands are available to trigger optional jobs:

/test check-dependency-stats
/test pull-ci-kubernetes-unit-windows
/test pull-crio-cgroupv1-node-e2e-eviction
/test pull-crio-cgroupv1-node-e2e-features
/test pull-crio-cgroupv1-node-e2e-hugepages
/test pull-crio-cgroupv1-node-e2e-resource-managers
/test pull-crio-cgroupv2-imagefs-e2e-diskpressure
/test pull-crio-cgroupv2-imagefs-e2e-separatedisktest
/test pull-e2e-gce-cloud-provider-disabled
/test pull-kubernetes-conformance-image-test
/test pull-kubernetes-conformance-kind-ga-only
/test pull-kubernetes-conformance-kind-ipv6-parallel
/test pull-kubernetes-cos-cgroupv1-containerd-node-e2e
/test pull-kubernetes-cos-cgroupv1-containerd-node-e2e-features
/test pull-kubernetes-cos-cgroupv2-containerd-node-e2e
/test pull-kubernetes-cos-cgroupv2-containerd-node-e2e-eviction
/test pull-kubernetes-cos-cgroupv2-containerd-node-e2e-features
/test pull-kubernetes-cos-cgroupv2-containerd-node-e2e-serial
/test pull-kubernetes-crio-node-memoryqos-cgrpv2
/test pull-kubernetes-cross
/test pull-kubernetes-e2e-autoscaling-hpa-cm
/test pull-kubernetes-e2e-autoscaling-hpa-cpu
/test pull-kubernetes-e2e-capz-azure-disk
/test pull-kubernetes-e2e-capz-azure-disk-vmss
/test pull-kubernetes-e2e-capz-azure-file
/test pull-kubernetes-e2e-capz-azure-file-vmss
/test pull-kubernetes-e2e-capz-conformance
/test pull-kubernetes-e2e-capz-windows-alpha-feature-vpa
/test pull-kubernetes-e2e-capz-windows-alpha-features
/test pull-kubernetes-e2e-capz-windows-master
/test pull-kubernetes-e2e-capz-windows-serial-slow
/test pull-kubernetes-e2e-capz-windows-serial-slow-hpa
/test pull-kubernetes-e2e-containerd-gce
/test pull-kubernetes-e2e-ec2
/test pull-kubernetes-e2e-ec2-arm64
/test pull-kubernetes-e2e-ec2-conformance
/test pull-kubernetes-e2e-ec2-conformance-arm64
/test pull-kubernetes-e2e-gce-canary
/test pull-kubernetes-e2e-gce-correctness
/test pull-kubernetes-e2e-gce-cos-alpha-features
/test pull-kubernetes-e2e-gce-csi-serial
/test pull-kubernetes-e2e-gce-device-plugin-gpu
/test pull-kubernetes-e2e-gce-disruptive-canary
/test pull-kubernetes-e2e-gce-kubelet-credential-provider
/test pull-kubernetes-e2e-gce-network-proxy-grpc
/test pull-kubernetes-e2e-gce-providerless
/test pull-kubernetes-e2e-gce-serial
/test pull-kubernetes-e2e-gce-serial-canary
/test pull-kubernetes-e2e-gce-storage-disruptive
/test pull-kubernetes-e2e-gce-storage-slow
/test pull-kubernetes-e2e-gce-storage-snapshot
/test pull-kubernetes-e2e-gci-gce-autoscaling
/test pull-kubernetes-e2e-gci-gce-ingress
/test pull-kubernetes-e2e-gci-gce-ipvs
/test pull-kubernetes-e2e-inplace-pod-resize-containerd-main-v2
/test pull-kubernetes-e2e-kind-alpha-beta-features
/test pull-kubernetes-e2e-kind-alpha-features
/test pull-kubernetes-e2e-kind-beta-features
/test pull-kubernetes-e2e-kind-canary
/test pull-kubernetes-e2e-kind-dual-canary
/test pull-kubernetes-e2e-kind-evented-pleg
/test pull-kubernetes-e2e-kind-ipv6-canary
/test pull-kubernetes-e2e-kind-ipvs-dual-canary
/test pull-kubernetes-e2e-kind-kms
/test pull-kubernetes-e2e-kind-multizone
/test pull-kubernetes-e2e-kind-nftables
/test pull-kubernetes-e2e-storage-kind-disruptive
/test pull-kubernetes-e2e-ubuntu-gce-network-policies
/test pull-kubernetes-kind-dra
/test pull-kubernetes-kind-json-logging
/test pull-kubernetes-kind-text-logging
/test pull-kubernetes-kubemark-e2e-gce-big
/test pull-kubernetes-linter-hints
/test pull-kubernetes-local-e2e
/test pull-kubernetes-node-arm64-e2e-containerd-ec2
/test pull-kubernetes-node-arm64-e2e-containerd-serial-ec2
/test pull-kubernetes-node-arm64-ubuntu-serial-gce
/test pull-kubernetes-node-crio-cgrpv1-evented-pleg-e2e
/test pull-kubernetes-node-crio-cgrpv2-e2e
/test pull-kubernetes-node-crio-cgrpv2-e2e-kubetest2
/test pull-kubernetes-node-crio-cgrpv2-imagefs-e2e
/test pull-kubernetes-node-crio-e2e
/test pull-kubernetes-node-crio-e2e-kubetest2
/test pull-kubernetes-node-e2e-containerd-1-7-dra
/test pull-kubernetes-node-e2e-containerd-alpha-features
/test pull-kubernetes-node-e2e-containerd-ec2
/test pull-kubernetes-node-e2e-containerd-features
/test pull-kubernetes-node-e2e-containerd-features-kubetest2
/test pull-kubernetes-node-e2e-containerd-kubetest2
/test pull-kubernetes-node-e2e-containerd-serial-ec2
/test pull-kubernetes-node-e2e-containerd-standalone-mode
/test pull-kubernetes-node-e2e-containerd-standalone-mode-all-alpha
/test pull-kubernetes-node-e2e-crio-dra
/test pull-kubernetes-node-kubelet-credential-provider
/test pull-kubernetes-node-kubelet-serial-containerd
/test pull-kubernetes-node-kubelet-serial-containerd-alpha-features
/test pull-kubernetes-node-kubelet-serial-containerd-kubetest2
/test pull-kubernetes-node-kubelet-serial-containerd-sidecar-containers
/test pull-kubernetes-node-kubelet-serial-cpu-manager
/test pull-kubernetes-node-kubelet-serial-cpu-manager-kubetest2
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2
/test pull-kubernetes-node-kubelet-serial-hugepages
/test pull-kubernetes-node-kubelet-serial-memory-manager
/test pull-kubernetes-node-kubelet-serial-pod-disruption-conditions
/test pull-kubernetes-node-kubelet-serial-topology-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager-kubetest2
/test pull-kubernetes-node-swap-fedora
/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial
/test pull-kubernetes-unit-experimental
/test pull-kubernetes-verify-lint
/test pull-publishing-bot-validate

Use /test all to run the following jobs that were automatically triggered:

pull-kubernetes-conformance-kind-ga-only-parallel
pull-kubernetes-dependencies
pull-kubernetes-e2e-ec2
pull-kubernetes-e2e-ec2-conformance
pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-canary
pull-kubernetes-e2e-gce-cos-alpha-features
pull-kubernetes-e2e-kind
pull-kubernetes-e2e-kind-ipv6
pull-kubernetes-integration
pull-kubernetes-linter-hints
pull-kubernetes-node-e2e-containerd
pull-kubernetes-typecheck
pull-kubernetes-unit
pull-kubernetes-verify
pull-kubernetes-verify-lint

In response to this:

/test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

kannon92 · 2024-02-09T14:26:31Z

/test pull-crio-cgroupv1-node-e2e-features

kannon92 · 2024-02-09T15:09:50Z

Make sure you trigger pull-crio-cgroupv1-node-e2e-features as that seems to hit your test.

You get:

{ failed [FAILED] Unexpected status code (500) during 'CheckpointContainer'
In [It] at: k8s.io/kubernetes/test/e2e_node/checkpoint_container.go:156 @ 02/09/24 14:43:49.232
}

adrianreber · 2024-02-09T15:29:29Z

@kannon92 Thanks. That is a good starting point.

adrianreber · 2024-02-11T15:26:40Z

/test pull-crio-cgroupv1-node-e2e-features

adrianreber · 2024-02-11T16:06:48Z

/test pull-crio-cgroupv1-node-e2e-features

k8s-ci-robot · 2024-02-20T19:22:26Z

LGTM label has been added.

Git tree hash: 9bcc632b046445672359e82fb4ad2dc4a0a2d49f

kannon92 · 2024-02-20T19:36:36Z

/triage accepted
/priority important-soon

kannon92 · 2024-02-20T20:27:53Z

test/e2e_node/checkpoint_container.go

+			// if the container engine returns that it explicitly has disabled support for it.
+			// or
+			// '(rpc error: code = Unknown desc = checkpoint/restore support not available)'
+			// if the container engine explicitly disabled the checkpoint/restore support
 			if (int(statusError.ErrStatus.Code)) == http.StatusInternalServerError {


My preference would be to fail rather than skip.

We should only be running these tests when we know checkpointing is set up.

We've seen cases where this jobs get skipped and no one really looks at them since they are not failing.

But I'm happy to do this as a followup.

I am not sure how to handle this without skipping. As long as we test against CRI implementations that do not support that CheckpointContainer RPC, either because of not implementing or explicitly disabling it, we need a way to ignore the test result. Skipping seems the one thing which allows this.

Do you have another idea, how to distinguish between a failure and the not implemented state. That is the reasons I am checking for the error message. These error message are the known reasons if an engine has not implemented it or explicitly disables support. If another failure is returned it should appear as a test error.

So its not clear to me how you configured these jobs..

Are all crio jobs able to run this?

I think I'd expect (if crio, these tests should work). Currently if containerd, they are not working. I'd explictly add that you expect this to fail for containerd.

That way you are making sure you know the expected behavior. skip is not great imo because as I said we will not report that anything went wrong.

This can be done as a followup I think. But just want to call it out.

It definitely makes sense to think about this. Skipping is indeed not optimal as it might hide real errors. Let's find a better solution in a followup.

cc: @mikebrow

kannon92 · 2024-02-22T14:35:10Z

/assign @mrunalp @dchen1107

@deads2k if you could find some time, would you want to confirm that we are correctly securing the checkpoint endpoint?

k8s-ci-robot · 2024-03-01T19:04:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianreber, mrunalp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/features/OWNERS~~ [mrunalp]
~~pkg/kubelet/server/OWNERS~~ [mrunalp]
~~test/e2e_node/OWNERS~~ [mrunalp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

adrianreber · 2024-03-01T20:42:23Z

/test pull-kubernetes-verify

k8s-triage-robot · 2024-03-01T22:53:18Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

sftim · 2024-03-12T17:13:42Z

Changelog suggestion

-Graduate "Forensic Container Checkpointing" (KEP #2008) from Alpha to Beta.
+Graduated _forensic container checkpointing_ [KEP #2008](https://kep.k8s.io/2008) from Alpha to Beta.

adrianreber · 2024-03-12T19:03:19Z

@sftim I updated the PR description with your suggestion. Is that enough?

sftim · 2024-03-12T19:28:30Z

Yep, that's fine.

Tobeabellwether · 2024-03-17T01:00:00Z

Hi @adrianreber , so will it be available as a beta feature in v1.30?

adrianreber · 2024-03-17T09:40:05Z

Hi @adrianreber , so will it be available as a beta feature in v1.30?

Yes. KEP and code has been merged.

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Feb 9, 2024

k8s-ci-robot requested review from dchen1107 and wojtek-t February 9, 2024 11:13

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 9, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 9, 2024

adrianreber force-pushed the 2024-02-09-forensic-container-checkpointing-beta branch from 280797f to 0fdb396 Compare February 11, 2024 15:26

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 11, 2024

k8s-ci-robot added area/test sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 11, 2024

adrianreber force-pushed the 2024-02-09-forensic-container-checkpointing-beta branch from 0fdb396 to f897249 Compare February 11, 2024 16:05

k8s-ci-robot assigned rphillips Feb 20, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2024

kannon92 moved this from WIP to Needs Approver in SIG Node PR Triage Feb 20, 2024

kannon92 reviewed Feb 20, 2024

View reviewed changes

kannon92 moved this from Triage to Archive-it in SIG Node CI/Test Board Feb 22, 2024

k8s-ci-robot assigned dchen1107 and mrunalp Feb 22, 2024

mrunalp approved these changes Mar 1, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 1, 2024

k8s-ci-robot merged commit cde4788 into kubernetes:master Mar 1, 2024
17 checks passed

SIG Node CI/Test Board automation moved this from Archive-it to Done Mar 1, 2024

SIG Node PR Triage automation moved this from Needs Approver to Done Mar 1, 2024

k8s-ci-robot added this to the v1.30 milestone Mar 1, 2024

adrianreber mentioned this pull request Mar 8, 2024

Forensic Container Checkpointing kubernetes/enhancements#2008

Open

21 tasks

adrianreber deleted the 2024-02-09-forensic-container-checkpointing-beta branch March 17, 2024 09:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch 'ContainerCheckpoint' from Alpha to Beta #123215

Switch 'ContainerCheckpoint' from Alpha to Beta #123215

adrianreber commented Feb 9, 2024 •

edited

k8s-ci-robot commented Feb 9, 2024

kannon92 commented Feb 9, 2024

kannon92 commented Feb 9, 2024

k8s-ci-robot commented Feb 9, 2024

kannon92 commented Feb 9, 2024

kannon92 commented Feb 9, 2024

adrianreber commented Feb 9, 2024

adrianreber commented Feb 11, 2024

adrianreber commented Feb 11, 2024

k8s-ci-robot commented Feb 20, 2024

kannon92 commented Feb 20, 2024

kannon92 Feb 20, 2024

kannon92 Feb 20, 2024

adrianreber Feb 21, 2024

kannon92 Feb 21, 2024

kannon92 Feb 21, 2024

adrianreber Feb 22, 2024

mrunalp Mar 1, 2024

kannon92 commented Feb 22, 2024

k8s-ci-robot commented Mar 1, 2024

adrianreber commented Mar 1, 2024

k8s-triage-robot commented Mar 1, 2024

sftim commented Mar 12, 2024

adrianreber commented Mar 12, 2024

sftim commented Mar 12, 2024

Tobeabellwether commented Mar 17, 2024

adrianreber commented Mar 17, 2024

Switch 'ContainerCheckpoint' from Alpha to Beta #123215

Switch 'ContainerCheckpoint' from Alpha to Beta #123215

Conversation

adrianreber commented Feb 9, 2024 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Feb 9, 2024

kannon92 commented Feb 9, 2024

kannon92 commented Feb 9, 2024

k8s-ci-robot commented Feb 9, 2024

kannon92 commented Feb 9, 2024

kannon92 commented Feb 9, 2024

adrianreber commented Feb 9, 2024

adrianreber commented Feb 11, 2024

adrianreber commented Feb 11, 2024

k8s-ci-robot commented Feb 20, 2024

kannon92 commented Feb 20, 2024

kannon92 Feb 20, 2024

Choose a reason for hiding this comment

kannon92 Feb 20, 2024

Choose a reason for hiding this comment

adrianreber Feb 21, 2024

Choose a reason for hiding this comment

kannon92 Feb 21, 2024

Choose a reason for hiding this comment

kannon92 Feb 21, 2024

Choose a reason for hiding this comment

adrianreber Feb 22, 2024

Choose a reason for hiding this comment

mrunalp Mar 1, 2024

Choose a reason for hiding this comment

kannon92 commented Feb 22, 2024

k8s-ci-robot commented Mar 1, 2024

adrianreber commented Mar 1, 2024

k8s-triage-robot commented Mar 1, 2024

sftim commented Mar 12, 2024

adrianreber commented Mar 12, 2024

sftim commented Mar 12, 2024

Tobeabellwether commented Mar 17, 2024

adrianreber commented Mar 17, 2024

adrianreber commented Feb 9, 2024 •

edited