Skip to content

[release/1.7] Ensure the CRIAPIV1Alpha2 warning's lastOccurrence is accurate#10571

Merged
samuelkarp merged 1 commit intocontainerd:release/1.7from
chrishenzie:v1alpha2-cri-api-warnings
Aug 12, 2024
Merged

[release/1.7] Ensure the CRIAPIV1Alpha2 warning's lastOccurrence is accurate#10571
samuelkarp merged 1 commit intocontainerd:release/1.7from
chrishenzie:v1alpha2-cri-api-warnings

Conversation

@chrishenzie
Copy link
Contributor

@chrishenzie chrishenzie commented Aug 9, 2024 •
edited
Loading

This helps gather more accurate API usage data that informs efforts on how to safely migrate containerd clients to 2.0.

/cc @samuelkarp @tallclair

Tested manually on workstation.

# No output.
15:28:17 $ sudo bin/ctr -a /tmp/containerd.sock deprecations list --format=json

# Test client using v1alpha2 CRI API.
15:29:13 $ go run main.go

15:29:52 $ sudo bin/ctr -a /tmp/containerd.sock deprecations list --format=json
[
    {
        "id": "io.containerd.deprecation/cri-api-v1alpha2",
        "message": "CRI API v1alpha2 is deprecated since containerd v1.7 and removed in containerd v2.0. Use CRI API v1 instead.",
        "lastOccurrence": "2024-08-09T22:29:15.605544912Z"
    }
]

# Issued another v1alpha2 CRI API call.
15:30:38 $ go run main.go

# Last occurrence is updated.
15:31:02 $ sudo bin/ctr -a /tmp/containerd.sock deprecations list --format=json
[
    {
        "id": "io.containerd.deprecation/cri-api-v1alpha2",
        "message": "CRI API v1alpha2 is deprecated since containerd v1.7 and removed in containerd v2.0. Use CRI API v1 instead.",
        "lastOccurrence": "2024-08-09T22:30:40.515385572Z"
    }
]

@k8s-ci-robot
Copy link

k8s-ci-robot commented Aug 9, 2024

@chrishenzie: GitHub didn't allow me to request PR reviews from the following users: tallclair.

Note that only containerd members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

This helps gather more accurate API usage data that informs efforts on how to safely migrate containerd clients to 2.0.

/cc @samuelkarp @tallclair

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot requested a review from samuelkarp August 9, 2024 22:45
@k8s-ci-robot
Copy link

k8s-ci-robot commented Aug 9, 2024

Hi @chrishenzie. Thanks for your PR.

I'm waiting for a containerd 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.

Details

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-sigs/prow repository.

@dosubot dosubot bot added the area/cri Container Runtime Interface (CRI) label Aug 9, 2024
@chrishenzie chrishenzie force-pushed the v1alpha2-cri-api-warnings branch from b47123c to f65664a Compare August 9, 2024 22:53
@samuelkarp
Copy link
Member

samuelkarp commented Aug 9, 2024

/ok-to-test

@samuelkarp samuelkarp changed the title Update CRIAPIV1Alpha2 warning lastOccurrence every call [release/1.7] Update CRIAPIV1Alpha2 warning lastOccurrence every call Aug 9, 2024
@chrishenzie
Update CRIAPIV1Alpha2 warning lastOccurrence every call
52b79f3 
This helps gather more accurate API usage data that informs efforts on how to safely migrate containerd clients to 2.0.

Signed-off-by: Chris Henzie <[email protected]>
@chrishenzie chrishenzie force-pushed the v1alpha2-cri-api-warnings branch from f65664a to 52b79f3 Compare August 9, 2024 23:21
@chrishenzie
Copy link
Contributor Author

chrishenzie commented Aug 10, 2024

/retest

1 similar comment
@chrishenzie
Copy link
Contributor Author

chrishenzie commented Aug 10, 2024

/retest

@chrishenzie
Copy link
Contributor Author

chrishenzie commented Aug 10, 2024

/retest

I don't suspect this change is related to the failing test

@chrishenzie chrishenzie mentioned this pull request Aug 12, 2024
Merged
@samuelkarp
Copy link
Member

samuelkarp commented Aug 12, 2024

Some additional context: having the CRIAPIV1Alpha2 warning only update the very first time the v1alpha2 API is used makes it really hard to detect whether usage of that API version has subsided. With an accurate lastOccurrence time, a user can examine ctr deprecations list and see whether the v1alpha2 API was recent or not, and can use this to figure out whether workload changes they make are effective in eliminating that feature use. The additional lock scope involved in updating the map inside the deprecation service should be minimal, and is only in play for the v1alpha2 API anyway; I think this is acceptable given that v1alpha2 is deprecated and we want users to migrate away from it.

@samuelkarp samuelkarp requested a review from mikebrow August 12, 2024 18:30
@samuelkarp samuelkarp changed the title [release/1.7] Update CRIAPIV1Alpha2 warning lastOccurrence every call [release/1.7] Ensure the CRIAPIV1Alpha2 warning's lastOccurrence is accurate Aug 12, 2024
mikebrow
mikebrow approved these changes Aug 12, 2024
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM with a TODO for us to discuss/ evaluate some use cases for enhancing the process

E.g. import http://google.golang.org/grpc/peer ... peer.FromContext(ctx) use the peer (if set) to map peer+apiname for the warning map and keep a count, times first/last ..

@samuelkarp
Copy link
Member

samuelkarp commented Aug 12, 2024

peer.FromContext(ctx) use the peer (if set) to map peer+apiname for the warning map and keep a count, times first/last ..

Deprecation service doesn't have the ability to store that information, but we could write a log line that has the client name.

@samuelkarp samuelkarp merged commit e242c6a into containerd:release/1.7 Aug 12, 2024
@chrishenzie chrishenzie deleted the v1alpha2-cri-api-warnings branch August 13, 2024 16:42
@samuelkarp samuelkarp added cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch and removed cherry-pick/1.6.x labels Aug 19, 2024
@chrishenzie chrishenzie mentioned this pull request Aug 23, 2024
Merged
Mengkzhaoyun pushed a commit to open-beagle/containerd that referenced this pull request Sep 4, 2024
Merge tag 'v1.7.21' into release-v1.7
ec63e14 
containerd 1.7.21

Welcome to the v1.7.21 release of containerd!

The twenty-first patch release for containerd 1.7 contains various fixes
and updates.

* Regenerate introspection UUID if state is empty ([#10510](containerd/containerd#10510))
* Set stderr to empty string when using terminal on Windows ([#10499](containerd/containerd#10499))

* Move builds to Go 1.22 and add support for testing with 1.23 ([#10596](containerd/containerd#10596))

* Borrow latest wsstream from k8s v1.31.x to 1.7 ([#10575](containerd/containerd#10575))
* Ensure the CRIAPIV1Alpha2 warning's lastOccurrence is accurate ([#10571](containerd/containerd#10571))
* Make `StopContainer` idempotent ([#10528](containerd/containerd#10528))
* Make `StopPodSandbox` idempotent ([#10527](containerd/containerd#10527))

* Fix failed force deletion for tasks with PID 0 ([#10523](containerd/containerd#10523))

* Fix packaged runc reporting incorrect version ([#10559](containerd/containerd#10559))
* Ensure `/run/containerd` gets created with correct perms ([#10534](containerd/containerd#10534))

* Ensure the CRIAPIV1Alpha2 warning's lastOccurrence is accurate ([#10571](containerd/containerd#10571))
* Update warnings for deprecated CRI config fields ([#10512](containerd/containerd#10512))

Please try out the release binaries and report any issues at
https://github.com/containerd/containerd/issues.

* Davanum Srinivas
* Samuel Karp
* Sebastiaan van Stijn
* Phil Estes
* Maksym Pavlenko
* Akhil Mohan
* Chris Henzie
* Derek McGowan
* Kazuyoshi Kato
* Sascha Grunert
* Akihiro Suda
* Erikson Tung
* Iceber Gu
* Mauri de Souza Meneguzzo
* Mike Brown
* Shengjing Zhu
* TinaMor
* rongfu.leng
<details><summary>45 commits</summary>
<p>

* Prepare release notes for v1.7.21 ([#10632](containerd/containerd#10632))
  * [`975f279ee`](containerd/containerd@975f279) Prepare release notes for v1.7.21
* go.mod: keep minimum go version at go1.21 ([#10633](containerd/containerd#10633))
  * [`d63bd8464`](containerd/containerd@d63bd84) go.mod: keep minimum go version at go1.21
* Move builds to Go 1.22 and add support for testing with 1.23 ([#10596](containerd/containerd#10596))
  * [`c76028088`](containerd/containerd@c760280) update golangci-lint to 1.60.1
  * [`3b263d082`](containerd/containerd@3b263d0) add go1.23.0, drop go1.21.x
* Fix TestNewBinaryIOCleanup on Go 1.23 and Linux 5.4 ([#10590](containerd/containerd#10590))
  * [`09ca004de`](containerd/containerd@09ca004) Fix TestNewBinaryIOCleanup on Go 1.23 and Linux 5.4
* Borrow latest wsstream from k8s v1.31.x to 1.7 ([#10575](containerd/containerd#10575))
  * [`9269d97b1`](containerd/containerd@9269d97) hide wsstream under internal/ to prevent external use
  * [`59815fa44`](containerd/containerd@59815fa) golangci-lint should only look for problems in new code
  * [`1c431dc6f`](containerd/containerd@1c431dc) Run go mod tidy
  * [`226f93d92`](containerd/containerd@226f93d) Add copyright headers
  * [`6f3252733`](containerd/containerd@6f32527) switch over references to the new package
  * [`0a85d476a`](containerd/containerd@0a85d47) Fix up some constant references
  * [`82bfa44d0`](containerd/containerd@82bfa44) Copy over wsstream from k8s v1.31.0-rc.1 release
* Ensure the CRIAPIV1Alpha2 warning's lastOccurrence is accurate ([#10571](containerd/containerd#10571))
  * [`52b79f337`](containerd/containerd@52b79f3) Update CRIAPIV1Alpha2 warning lastOccurrence every call
* pkg/userns: deprecate and migrate to github.com/moby/sys/userns ([#10564](containerd/containerd#10564))
  * [`dce0b5a6d`](containerd/containerd@dce0b5a) migrate to github.com/moby/sys/userns
  * [`65f7d0740`](containerd/containerd@65f7d07) pkg/userns: deprecate and migrate to github.com/moby/sys/user/userns
  * [`f21675c27`](containerd/containerd@f21675c) vendor: github.com/moby/sys/user v0.2.0
* update to go1.21.13 / go1.22.6 ([#10570](containerd/containerd#10570))
  * [`228914a5e`](containerd/containerd@228914a) update to go1.21.13 / go1.22.6
* Fix TestNewBinaryIOCleanup failing with gotip ([#10554](containerd/containerd#10554))
  * [`3ff82ba0f`](containerd/containerd@3ff82ba) Fix TestNewBinaryIOCleanup failing with gotip
* Fix packaged runc reporting incorrect version ([#10559](containerd/containerd#10559))
  * [`d51143f6f`](containerd/containerd@d51143f) script/setup/install-runc: fix runc using incorrect version
* update auths code comment ([#10536](containerd/containerd#10536))
  * [`7bb1455d8`](containerd/containerd@7bb1455) update auths code comment
* Ensure `/run/containerd` gets created with correct perms ([#10534](containerd/containerd#10534))
  * [`16c5fc768`](containerd/containerd@16c5fc7) Ensure /run/containerd is created with correct perms
* Make `StopContainer` idempotent ([#10528](containerd/containerd#10528))
  * [`6da4e40b2`](containerd/containerd@6da4e40) Make `StopContainer` RPC idempotent
* Make `StopPodSandbox` idempotent ([#10527](containerd/containerd#10527))
  * [`b3b6f1507`](containerd/containerd@b3b6f15) Make `StopPodSandbox` RPC idempotent
* Fix failed force deletion for tasks with PID 0 ([#10523](containerd/containerd#10523))
  * [`0db46f664`](containerd/containerd@0db46f6) client: fix tasks with PID 0 cannot be forced to delete
* Update warnings for deprecated CRI config fields ([#10512](containerd/containerd#10512))
  * [`9afb8dcdf`](containerd/containerd@9afb8dc) deprecation: update warnings for CRI config fields
* Regenerate introspection UUID if state is empty ([#10510](containerd/containerd#10510))
  * [`b140792e4`](containerd/containerd@b140792) introspection: regenerate UUID if state is empty
* Set stderr to empty string when using terminal on Windows ([#10499](containerd/containerd#10499))
  * [`f9beac3db`](containerd/containerd@f9beac3) Set stderr to empty string when using terminal on Windows.
</p>
</details>

* **github.com/moby/sys/userns**  v0.1.0 **_new_**

Previous release can be found at [v1.7.20](https://github.com/containerd/containerd/releases/tag/v1.7.20)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samuelkarp samuelkarp samuelkarp approved these changes

@mikebrow mikebrow mikebrow approved these changes

Assignees

No one assigned

Labels

area/cri Container Runtime Interface (CRI) cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch impact/changelog impact/deprecation ok-to-test size/S

Projects

Pull Request Review
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@chrishenzie @k8s-ci-robot @samuelkarp @mikebrow