Skip to content
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.

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

Update cri-api to v0.29.1 #9378

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Update cri-api to v0.29.1 #9378

merged 1 commit into from
Feb 13, 2024

Conversation

kiashok
Copy link
Contributor

@kiashok kiashok commented Nov 16, 2023

Update k8s.io/cri-api to v0.29.1

@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kiashok
Copy link
Contributor Author

kiashok commented Nov 16, 2023

/test all

@kiashok
Copy link
Contributor Author

kiashok commented Nov 20, 2023

image

Hi @dmcgowan @mikebrow , the v1.29 alpha3 tag of upstream k8s requires 1.21.3 golang version. But it looks like we don't allow 1.21.x version in containerd golang? Is there a way I could work around this?
I would like to update to the newer patch of k8s cri-api to send out PRs for the image pull per runtime class changes.

@estesp
Copy link
Member

estesp commented Nov 20, 2023

We default to 1.21.3 in our CI GH Actions script, but we verify that containerd still builds with Go 1.20.x as that release is still supported (see here).

I guess if upstream Kubernetes 1.29 is going to require Go 1.21 we will have to discuss how to handle that requirement in our project.

@k8s-ci-robot
Copy link

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

  • /test pull-containerd-build
  • /test pull-containerd-node-e2e

Use /test all to run all jobs.

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.

@kiashok
Copy link
Contributor Author

kiashok commented Nov 27, 2023

/test all

@kiashok kiashok force-pushed the updateCriApi branch 2 times, most recently from ba941b8 to 170dc8a Compare November 29, 2023 20:38
@kiashok kiashok changed the title Update cri-api to v0.29.0-alpha.3 Update cri-api to v0.29.0-rc.1 Nov 29, 2023
@kiashok kiashok marked this pull request as ready for review November 29, 2023 22:08
@kiashok
Copy link
Contributor Author

kiashok commented Nov 29, 2023

We default to 1.21.3 in our CI GH Actions script, but we verify that containerd still builds with Go 1.20.x as that release is still supported (see here).

I guess if upstream Kubernetes 1.29 is going to require Go 1.21 we will have to discuss how to handle that requirement in our project.

kubernetes/cri-api@5a34c2b this commit was made to fix this kubernetes/kubernetes#121338 . Started a thread for this discussion on sig-release. Not sure if its a hard requirement for cri-api v1.29 to stay on golang 1.21

cc @dims @dmcgowan @thaJeztah

@dims
Copy link
Member

dims commented Nov 30, 2023

@kiashok i don't think the error means that... it just objects that the format is A.B.C and not just A.B.

If you see latest https://github.com/kubernetes/cri-api/blob/master/go.mod#L5 we fixed it already, if you wait for rc2 you should be all set i think. you can check this theory by editing the file in your PR by hand

@thaJeztah
Copy link
Member

@kiashok i don't think the error means that... it just objects that the format is A.B.C and not just A.B.

If you see latest https://github.com/kubernetes/cri-api/blob/master/go.mod#L5 we fixed it already, if you wait for rc2 you should be all set i think. you can check this theory by editing the file in your PR by hand

Currently, the error is about the SemVer format (which is not supported by go1.20), but unfortunately, the version in go mod is now a required version as well, no longer an "indicative" / "recommended" version;

Here's a quick example using a "mymodule" module, which uses go 1.22 in its go.mod;

tree
.
├── go.mod
├── main.go
└── mymodule
    ├── foo.go
    └── go.mod


cat mymodule/go.mod
module example.com/mymodule

go 1.22

Trying to use such a module in a project; means it cannot be used on go1.20 or go1.21.

at go.mod
module myproject


require "example.com/mymodule" v0.0.0

replace "example.com/mymodule" => ./mymodule

This worked until go1.20.7 and go1.19.12;

docker run --rm -v $(pwd):/myproject -w /myproject golang:1.20.7 sh -c 'go run main.go'
hello:hello

docker run --rm -v $(pwd):/myproject -w /myproject golang:1.19.12 sh -c 'go run main.go'
hello:hello

But no longer works in go1.20.8 and go1.19.13;

docker run --rm -v $(pwd):/myproject -w /myproject golang:1.20.8 sh -c 'go run main.go'
example.com/mymodule: cannot compile Go 1.22 code

docker run --rm -v $(pwd):/myproject -w /myproject golang:1.19.13 sh -c 'go run main.go'
example.com/mymodule: cannot compile Go 1.22 code

Same for go1.21, but with a slightly different error-message;

go run main.go
go: example.com/mymodule@v0.0.0: module ./mymodule requires go >= 1.22 (running go 1.21.4)

@kiashok kiashok changed the title Update cri-api to v0.29.0-rc.1 Update cri-api to v0.29.0 Dec 13, 2023
@kiashok
Copy link
Contributor Author

kiashok commented Dec 18, 2023

@thaJeztah @dmcgowan @estesp @dcantah all the tests are passing. Could you please take a look when you have some time please?

@kiashok
Copy link
Contributor Author

kiashok commented Jan 2, 2024

@fuweid @estesp @dmcgowan @dcantah could you please take a look when you have some time please? :)

Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

.github/workflows/release.yml

The go version in release.yaml should be updated as well

go.mod Outdated Show resolved Hide resolved
@dims dims added the area/cri Container Runtime Interface (CRI) label Feb 7, 2024
Signed-off-by: kiashok <kiashok@microsoft.com>
@kiashok kiashok changed the title Update cri-api to v0.29.0 Update cri-api to v0.29.1 Feb 10, 2024
@kiashok
Copy link
Contributor Author

kiashok commented Feb 10, 2024

@dmcgowan @fuweid @mikebrow updated the PR to v0.29.1

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM noting we should tag, to this PR, the open issues/pr(s) handling the following cri changes for v028.2 -> v0.29.1:

  1. ImageSpec struct now contains RuntimeHandler string ... and a GetRuntimeHandler()
  2. ImageFsInfoResponse now contains ContainerFilesystems []*FilesystemUsage ... and a GetContainerFilesystems()

@mikebrow mikebrow added this pull request to the merge queue Feb 13, 2024
Merged via the queue into containerd:main with commit dd725fa Feb 13, 2024
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cri Container Runtime Interface (CRI) size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants