use containerd/containerd/api module #3526

akhilerm · 2024-05-04T03:01:42Z

use containerd/containerd/api module instead of having a copy in third_party
use containerd/errdefs module instead of keeping a local copy

k8s-ci-robot · 2024-05-04T03:01:51Z

Hi @akhilerm. Thanks for your PR.

I'm waiting for a google 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-sigs/prow repository.

bobbypage · 2024-05-04T03:41:29Z

/ok-to-test

bobbypage · 2024-05-04T03:41:37Z

Very exciting to see this happen!

akhilerm · 2024-05-04T05:06:13Z

@bobbypage Can you approve the github action workflows also?

dims · 2024-05-04T11:33:26Z

/ok-to-test

build/integration-in-docker.sh

akhilerm · 2024-05-07T06:26:44Z

/cc @bobbypage @dims for review

akhilerm · 2024-05-07T09:46:15Z

go.mod

@@ -1,73 +1,74 @@
 module github.com/google/cadvisor

-go 1.19
+go 1.22.0

 replace golang.org/x/net => golang.org/x/net v0.7.0


@bobbypage Shouldnt this be removed? I am not sure why the version was pinned. Also v0.23.0 fixes CVE-2023-45288

akhilerm · 2024-05-14T03:59:24Z

/hold
will merge once containerd/containerd/api v1.8.0 is released

akhilerm · 2024-06-21T03:55:19Z

@bobbypage Can you approve the workflow. I have rebased against the latest master branch

iwankgb · 2024-06-22T12:27:18Z

@akhilerm, you have conflicts against master.

akhilerm · 2024-06-22T17:42:24Z

rebased against master and pushed the changes.

akhilerm · 2024-06-22T18:02:35Z

The pull-cadvisor-e2e job is failing due to env setup error

/retest pull-cadvisor-e2e

k8s-ci-robot · 2024-06-22T18:02:50Z

@akhilerm: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test pull-cadvisor-e2e

Use /test all to run all jobs.

In response to this:

The pull-cadvisor-e2e job is failing due to env setup error

/retest pull-cadvisor-e2e

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.

akhilerm · 2024-06-22T18:02:54Z

/test pull-cadvisor-e2e

akhilerm · 2024-06-25T15:23:24Z

/test pull-cadvisor-e2e

iwankgb

Just two minor comments :)

iwankgb · 2024-06-28T15:37:10Z

container/containerd/containers/containers.go

@@ -33,7 +33,7 @@ import (
 	"context"
 	"time"

-	"github.com/gogo/protobuf/types"
+	anypb "google.golang.org/protobuf/types/known/anypb"


Is this alias necessary?

iwankgb · 2024-06-28T15:42:27Z

container/containerd/handler_test.go

+// Copied from https://github.com/containerd/containerd/blob/main/pkg/protobuf/any.go
+// FromAny converts typeurl.Any to github.com/containerd/containerd/protobuf/types.Any.
+//
+// TODO: vendor these functions once they are moved to github.com/containerd/typeurl.
+//
+//	Currently they are copied over so as not to introduce a dependency on containerd/containerd again
+func FromAny(from typeurl.Any) *anypb.Any {
+	if from == nil {
+		return nil
+	}
+
+	if pbany, ok := from.(*anypb.Any); ok {
+		return pbany
+	}
+
+	return &anypb.Any{
+		TypeUrl: from.GetTypeUrl(),
+		Value:   from.GetValue(),
+	}
+}
+
+// MarshalAnyToProto converts an arbitrary interface to github.com/containerd/containerd/protobuf/types.Any.
+func MarshalAnyToProto(from interface{}) (*anypb.Any, error) {
+	anyType, err := typeurl.MarshalAny(from)
+	if err != nil {
+		return nil, err
+	}
+	return FromAny(anyType), nil
+}


Can these two functions be unexported, please?

There is a PR in containerd/typeurl#45, so that these funcs can be vendored and used instead of copying it over here.

iwankgb · 2024-06-28T16:54:49Z

cmd/go.mod

@@ -44,6 +44,8 @@ require (
 	github.com/checkpoint-restore/go-criu/v5 v5.3.0 // indirect
 	github.com/cilium/ebpf v0.7.0 // indirect
 	github.com/containerd/console v1.0.3 // indirect
+	github.com/containerd/containerd/api v1.8.0-rc.2 // indirect


This PR shall not be merged with RC version.

This PR was raised so as to test the containerd/api module compatiblity with cadvisor. I am updating as each rc version releases so that we know there are no breaking changes.

PR needs to be merged only after containerd/api v1.8.0 is released.

More discussion on the same is happening on this thread https://kubernetes.slack.com/archives/CHGFYJVAN/p1714733633634819

Note that API v1.7.19 has been tagged; the v1.8.0-rc is the API version from the main branch (containerd v2.0-dev), so depending on what's needed, maybe the v1.7.19 version is an option?

Yes. Thats a better option. Will update to api/v1.7.19

Looks like the "indirect" here stuck on 1.8; could be either go modules's tendency to not downgrade once it's there, or could there be something else in the dependency tree pushing it up? 🤔

(Sorry, maybe you already noticed that, but just in case 🙈 🫶 )

My bad. Hadnt checked the new diff before pushing. Thought tidy will downgrade it automatically. It was because of this

could be either go modules's tendency to not downgrade once it's there

running just a go mod tidy will not downgrade, had to remove and run the tidy again so as to bring in 1.7.19 as indirect dependency.

Yes, that fooled me many times; if a dependency version (based on the dependency tree "minimum version selection") either becomes lower, or even is no longer a dependency, then go modules thinks it was a manual override / upgrade.

That sometimes makes sense (if you indeed updated the version because you wanted a newer version) but sometimes you don't, and just remove all indirects to force it re-resolving the minimum versions can help find those.

- use containerd/api v1.8.0-rc.2 instead of relying on third_party directory - remove script to copy /api from containerd/containerd Signed-off-by: Akhil Mohan <[email protected]>

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm · 2024-06-29T15:40:03Z

/hold

for containerd/typeurl#45 to merge

Signed-off-by: Akhil Mohan <[email protected]>

k8s-ci-robot added the needs-ok-to-test label May 4, 2024

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels May 4, 2024

bobbypage marked this pull request as ready for review May 4, 2024 03:41

akhilerm commented May 7, 2024

View reviewed changes

build/integration-in-docker.sh Outdated Show resolved Hide resolved

akhilerm commented May 7, 2024

View reviewed changes

akhilerm force-pushed the test-containerd-api-module branch from 2f42059 to 7547a2f Compare June 21, 2024 03:53

akhilerm force-pushed the test-containerd-api-module branch from 7547a2f to 144fff5 Compare June 22, 2024 17:41

akhilerm force-pushed the test-containerd-api-module branch from 144fff5 to cad3611 Compare June 22, 2024 17:49

iwankgb reviewed Jun 28, 2024

View reviewed changes

iwankgb requested changes Jun 28, 2024

View reviewed changes

akhilerm added 2 commits June 29, 2024 20:40

use containerd/api module instead of third_party

fbdac89

- use containerd/api v1.8.0-rc.2 instead of relying on third_party directory - remove script to copy /api from containerd/containerd Signed-off-by: Akhil Mohan <[email protected]>

fix gogo/protobuf -> google.golang.org/protobuf

522df10

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm force-pushed the test-containerd-api-module branch from 31e5bf7 to 076b07e Compare June 29, 2024 15:12

akhilerm added 2 commits June 29, 2024 20:45

use Marshal funcs from containerd and fix typeurl

76098b1

Signed-off-by: Akhil Mohan <[email protected]>

replace errdefs pkg with containerd/errdefs

2710887

Signed-off-by: Akhil Mohan <[email protected]>

remove anypb import aliasing

3a487fd

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm force-pushed the test-containerd-api-module branch from 076b07e to 3a487fd Compare June 29, 2024 15:15

akhilerm force-pushed the test-containerd-api-module branch from d05e3dd to 9aaa8d3 Compare June 29, 2024 17:41

Update containerd/containerd/api to v1.7.19

72387d4

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm force-pushed the test-containerd-api-module branch from 9aaa8d3 to 72387d4 Compare June 29, 2024 17:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use containerd/containerd/api module #3526

use containerd/containerd/api module #3526

akhilerm commented May 4, 2024 •

edited

Loading

k8s-ci-robot commented May 4, 2024

bobbypage commented May 4, 2024

bobbypage commented May 4, 2024

akhilerm commented May 4, 2024

dims commented May 4, 2024

akhilerm commented May 7, 2024

akhilerm May 7, 2024

akhilerm commented May 14, 2024

akhilerm commented Jun 21, 2024

iwankgb commented Jun 22, 2024

akhilerm commented Jun 22, 2024

akhilerm commented Jun 22, 2024

k8s-ci-robot commented Jun 22, 2024

akhilerm commented Jun 22, 2024

akhilerm commented Jun 25, 2024

iwankgb left a comment

iwankgb Jun 28, 2024

akhilerm Jun 29, 2024

iwankgb Jun 28, 2024

akhilerm Jun 29, 2024

iwankgb Jun 28, 2024

akhilerm Jun 29, 2024

thaJeztah Jun 29, 2024

akhilerm Jun 29, 2024

akhilerm Jun 29, 2024

thaJeztah Jun 29, 2024

akhilerm Jun 29, 2024

thaJeztah Jun 29, 2024

akhilerm commented Jun 29, 2024

use containerd/containerd/api module #3526

Are you sure you want to change the base?

use containerd/containerd/api module #3526

Conversation

akhilerm commented May 4, 2024 • edited Loading

k8s-ci-robot commented May 4, 2024

bobbypage commented May 4, 2024

bobbypage commented May 4, 2024

akhilerm commented May 4, 2024

dims commented May 4, 2024

akhilerm commented May 7, 2024

Choose a reason for hiding this comment

akhilerm commented May 14, 2024

akhilerm commented Jun 21, 2024

iwankgb commented Jun 22, 2024

akhilerm commented Jun 22, 2024

akhilerm commented Jun 22, 2024

k8s-ci-robot commented Jun 22, 2024

akhilerm commented Jun 22, 2024

akhilerm commented Jun 25, 2024

iwankgb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akhilerm commented Jun 29, 2024

akhilerm commented May 4, 2024 •

edited

Loading