intel / cri-resource-manager

Kubernetes Container Runtime Interface proxy service with hardware resource aware workload placement policies
Apache License 2.0
172 stars 56 forks source link

pkg/cri: remove support for the obsolete v1alpha2 CRI protocol. #1081

Closed klihub closed 8 months ago

klihub commented 8 months ago

This patch set

Note that relaying for the streaming PLEG/GetContainerEvents interface is not implemented.

codecov-commenter commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (cfe4bc2) 32.12% compared to head (3ffff03) 32.12%. Report is 7 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1081 +/- ## ======================================= Coverage 32.12% 32.12% ======================================= Files 64 64 Lines 9937 9937 ======================================= Hits 3192 3192 Misses 6449 6449 Partials 296 296 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

marquiz commented 8 months ago

LGTM

marquiz commented 8 months ago

I suppose this is something to be emphasized in release notes... CRI-RM 0.9 requires Kubernetes 1.23+ and containerd 1.6+ or something.

Yes, definitely.

@klihub please rebase and let's see how it goes. I think this would be "Ready for review" 😊

marquiz commented 8 months ago

Or should we wait for #1079?

klihub commented 8 months ago

@marquiz, @askervin I think merging this requires/warrants rerunning the end-to-end tests.

marquiz commented 8 months ago

I think merging this requires/warrants rerunning the end-to-end tests.

Fair point

klihub commented 8 months ago

I think merging this requires/warrants rerunning the end-to-end tests.

Fair point

Okay, with the latest commit at the tip, all tests pass.

marquiz commented 8 months ago

Okay, with the latest commit at the tip, all tests pass.

Yeah, same here.

LGTM