node: devicemgr: remove obsolete pre-1.20 checkpoint file support #123398

ffromani · 2024-02-20T16:43:51Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

In commit 2f426fd we added compatibility (and tests) to deal with pre-1.20 checkpoint files. We can now remove this code, including the brittle (but still the best we had available for the issue at hand) e2e tests and simplify again the device manager code

Which issue(s) this PR fixes:

Fixes #N/A

Special notes for your reviewer:

see PR #102882 for details about the original issue

Does this PR introduce a user-facing change?

the kubelet is no longer able to recover from device manager state file older than 1.20. If the proper recommended upgrade flow is followed, there should be no issue.

ffromani · 2024-02-20T16:43:59Z

/sig node

ffromani · 2024-02-20T16:44:27Z

/test

k8s-ci-robot · 2024-02-20T16:44:32Z

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

/test pull-cadvisor-e2e-kubernetes
/test pull-cos-containerd-e2e-ubuntu-gce
/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
/test pull-kubernetes-verify-lint

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-publishing-bot-validate

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

pull-kubernetes-conformance-kind-ga-only-parallel
pull-kubernetes-conformance-kind-ipv6-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-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.

ffromani · 2024-02-20T16:46:18Z

/test pull-crio-cgroupv1-node-e2e-resource-managers
/test pull-kubernetes-e2e-capz-windows-serial-slow
/test pull-kubernetes-node-e2e-containerd-serial-ec2
/test pull-kubernetes-node-kubelet-serial-containerd
/test pull-kubernetes-node-kubelet-serial-cpu-manager
/test pull-kubernetes-node-kubelet-serial-memory-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager

ffromani · 2024-02-20T18:02:27Z

@kannon92 FYI

ffromani · 2024-02-20T18:22:17Z

/test pull-crio-cgroupv1-node-e2e-resource-managers
/test pull-kubernetes-e2e-capz-windows-serial-slow
/test pull-kubernetes-node-e2e-containerd-serial-ec2
/test pull-kubernetes-node-kubelet-serial-containerd
/test pull-kubernetes-node-kubelet-serial-cpu-manager
/test pull-kubernetes-node-kubelet-serial-memory-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager

ffromani · 2024-02-21T06:50:39Z

/test pull-kubernetes-node-kubelet-serial-containerd

ffromani · 2024-02-21T08:46:09Z

failed because (unrelated)

ERROR: (gcloud.compute.instances.create) Could not fetch resource:
 - Quota 'NVIDIA_K80_GPUS' exceeded.  Limit: 0.0 in region us-west1.
	metric name = compute.googleapis.com/nvidia_k80_gpus
	limit name = NVIDIA-K80-GPUS-per-project-region
	limit = 0.0
	dimensions = region: us-west1

I think this is non-blocking

ffromani · 2024-02-21T10:58:25Z

/area kubelet
/area test

ffromani · 2024-02-21T10:59:13Z

/label area/kubelet

k8s-ci-robot · 2024-02-21T10:59:16Z

@ffromani: The label(s) /label area/kubelet cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor, official-cve-feed. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label area/kubelet

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.

ffromani · 2024-03-08T09:12:25Z

/test pull-kubernetes-node-kubelet-serial-containerd

still looks like infra issue to me

ffromani · 2024-04-09T11:06:01Z

/cc @swatisehgal

pkg/kubelet/cm/devicemanager/manager.go

test/e2e_node/device_manager_test.go

In commit 2f426fd we added compatibility (and tests) to deal with pre-1.20 checkpoint files. We are now well past the end of support for pre-1.20 kubelets, so we can get rid of this code. Signed-off-by: Francesco Romani <fromani@redhat.com>

swatisehgal

/lgtm

/assign @klueska
for approval

k8s-ci-robot · 2024-04-15T12:50:34Z

LGTM label has been added.

Git tree hash: 084e038732b079f6eda6c9f398d72e1e9280961b

SergeyKanzhelev · 2024-04-22T23:24:20Z

Does this PR introduce a user-facing change?

Yes it does. Since we are breaking this compatibility we should mention it in relnotes.

I don't think it is critical though. I cannot imagine people upgrading from 1.20 to 1.31 and expecting scheduled pods to keep running as normal - they will need to drain the node.

SergeyKanzhelev

/lgtm
/approve

ffromani · 2024-04-23T05:42:35Z

/hold

Does this PR introduce a user-facing change?

Yes it does. Since we are breaking this compatibility we should mention it in relnotes.

I don't think it is critical though. I cannot imagine people upgrading from 1.20 to 1.31 and expecting scheduled pods to keep running as normal - they will need to drain the node.

I see your point but we are breaking compatibility against direct upgrade from a version long unsupported. IOW, users upgrading using the suggested path should never see this issue.
That said I'm fine fixing the release notes.

ffromani · 2024-04-29T07:32:59Z

/hold cancel

release note updated

SergeyKanzhelev

/lgtm
/approve

k8s-ci-robot · 2024-04-29T20:45:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ffromani, mrunalp, SergeyKanzhelev, swatisehgal

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/kubelet/cm/OWNERS~~ [mrunalp]
~~test/e2e_node/OWNERS~~ [SergeyKanzhelev,mrunalp]

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

k8s-ci-robot added area/kubelet area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Feb 20, 2024

k8s-ci-robot requested review from klueska and mtaufen February 20, 2024 16:45

ffromani force-pushed the remove-legacy-checkpoint branch from 8eec588 to 7708bc9 Compare February 20, 2024 18:21

kannon92 mentioned this pull request Feb 20, 2024

mark flaky jobs as flaky and move them to a different job #123386

Merged

ffromani added this to PRs - Needs Reviewer in SIG Node CI/Test Board Feb 21, 2024

ffromani moved this from PRs - Needs Reviewer to Issues - To do in SIG Node CI/Test Board Feb 21, 2024

bart0sh added this to Triage in SIG Node PR Triage Feb 21, 2024

k8s-ci-robot requested a review from swatisehgal April 9, 2024 11:06

swatisehgal reviewed Apr 12, 2024

View reviewed changes

pkg/kubelet/cm/devicemanager/manager.go Show resolved Hide resolved

test/e2e_node/device_manager_test.go Show resolved Hide resolved

ffromani force-pushed the remove-legacy-checkpoint branch from 7708bc9 to 181fb0d Compare April 15, 2024 12:02

swatisehgal approved these changes Apr 15, 2024

View reviewed changes

k8s-ci-robot assigned klueska and swatisehgal Apr 15, 2024

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

swatisehgal moved this from Needs Reviewer to Needs Approver in SIG Node PR Triage Apr 15, 2024

ffromani moved this from Issues - To do to PRs - Needs Approver in SIG Node CI/Test Board Apr 15, 2024

SergeyKanzhelev approved these changes Apr 22, 2024

View reviewed changes

k8s-ci-robot assigned SergeyKanzhelev Apr 22, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 29, 2024

SergeyKanzhelev approved these changes Apr 29, 2024

View reviewed changes

mrunalp approved these changes Apr 29, 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 Apr 29, 2024

k8s-ci-robot merged commit 1fd835c into kubernetes:master Apr 29, 2024
15 checks passed

SIG Node CI/Test Board automation moved this from PRs - Needs Approver to Done Apr 29, 2024

SIG Node PR Triage automation moved this from Needs Approver to Done Apr 29, 2024

k8s-ci-robot added this to the v1.31 milestone Apr 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

node: devicemgr: remove obsolete pre-1.20 checkpoint file support #123398

node: devicemgr: remove obsolete pre-1.20 checkpoint file support #123398

ffromani commented Feb 20, 2024 •

edited

ffromani commented Feb 20, 2024

ffromani commented Feb 20, 2024

k8s-ci-robot commented Feb 20, 2024

ffromani commented Feb 20, 2024

ffromani commented Feb 20, 2024

ffromani commented Feb 20, 2024

ffromani commented Feb 21, 2024

ffromani commented Feb 21, 2024

ffromani commented Feb 21, 2024

ffromani commented Feb 21, 2024

k8s-ci-robot commented Feb 21, 2024

ffromani commented Mar 8, 2024

ffromani commented Apr 9, 2024

swatisehgal left a comment

k8s-ci-robot commented Apr 15, 2024

SergeyKanzhelev commented Apr 22, 2024

SergeyKanzhelev left a comment

ffromani commented Apr 23, 2024

ffromani commented Apr 29, 2024

SergeyKanzhelev left a comment

k8s-ci-robot commented Apr 29, 2024

node: devicemgr: remove obsolete pre-1.20 checkpoint file support #123398

node: devicemgr: remove obsolete pre-1.20 checkpoint file support #123398

Conversation

ffromani commented Feb 20, 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?

ffromani commented Feb 20, 2024

ffromani commented Feb 20, 2024

k8s-ci-robot commented Feb 20, 2024

ffromani commented Feb 20, 2024

ffromani commented Feb 20, 2024

ffromani commented Feb 20, 2024

ffromani commented Feb 21, 2024

ffromani commented Feb 21, 2024

ffromani commented Feb 21, 2024

ffromani commented Feb 21, 2024

k8s-ci-robot commented Feb 21, 2024

ffromani commented Mar 8, 2024

ffromani commented Apr 9, 2024

swatisehgal left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 15, 2024

SergeyKanzhelev commented Apr 22, 2024

SergeyKanzhelev left a comment

Choose a reason for hiding this comment

ffromani commented Apr 23, 2024

ffromani commented Apr 29, 2024

SergeyKanzhelev left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 29, 2024

ffromani commented Feb 20, 2024 •

edited