-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Depend only on containerd API and avoid vendoring unnecessary things from containerd #2908
Depend only on containerd API and avoid vendoring unnecessary things from containerd #2908
Conversation
a6c22cb
to
0d644ce
Compare
|
0d644ce
to
010599f
Compare
I'm not dev, but I think description is misleading and it would be good to split this to several commits; e.g. adding the containerd code to cadvisor, another changing usage of external containerd code to new internal copy, adding tests and updating go.mod/sum. |
@eero-t thanks for the feedback, as noted in the title, it's a Work In Progress (WIP) PR. |
You can simply change this to draft PR :) |
you are right @Creatone. thanks for the tip. |
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
github.com/containerd/containerd/api is now available ( https://github.com/containerd/containerd/blob/main/api/go.mod ) Now we wait for moby/moby#42624 and moby/moby#42623 |
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
51a81b5
to
950e3e2
Compare
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
950e3e2
to
d27911b
Compare
Discussed this with @dims offline. Long term ideally we can not copy these packages into cAdvisor and they will be an external module published by containerd we can consume. Until we get there this seems like the best option to remove k/k dependency loop. |
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com>
In containerd 1.5.x, we introduced support for go modules by adding a go.mod file in the root directory. This go.mod lists all the things needed across the whole code base (with the exception of integration/client which has its own go.mod). So when projects that need to make calls to containerd API will pull in some code from containerd/containerd, the `go mod` commands will add all the things listed in the root go.mod to the projects go.mod file. This causes some problems as the list of things needed to make a simple API call is enormous. in effect, making a API call will pull everything that a typical server needs as well as the root go.mod is all encompassing. In general if we had smaller things folks could use, that will make it easier by reducing the number of things that will end up in a consumers go.mod file. Now coming to a specific problem, the root containerd go.mod has various k8s.io/* modules listed. Also kubernetes depends on containerd indirectly via both moby/moby (working with docker maintainers seperately) and via google/cadvisor. So when the kubernetes maintainers try to use latest 1.5.x containerd, they will see the kubernetes go.mod ending up depending on the older version of kubernetes! So if we can expose just the minimum things needed to make a client API call then projects like cadvisor can adopt that instead of pulling in the entire go.mod from containerd. Looking at the existing code in cadvisor the minimum things needed would be the api/ directory from containerd. Please see proof of concept here: github.com/google/cadvisor/pull/2908 To enable that, in this PR, we add a go.mod file in api/ directory. we split the Protobuild.yaml into two, one for just the things in api/ directory and the rest in the root directory. We adjust various targets to build things correctly using `protobuild` and also ensure that we end up with the same generated code as before as well. To ensure we better take care of the various go.mod/go.sum files, we update the existing `make vendor` and also add a new `make verify-vendor` that one can run locally as well in the CI. Ideally, we would have a `containerd/client` either as a standalone repo or within `containerd/containerd` as a separate go module. but we will start here to experiment with a standalone api go module first. Also there are various follow ups we can do, for example @thaJeztah has identified two tasks we could do after this PR lands: github.com/containerd/containerd/pull/5716#discussion_r668821396 Signed-off-by: Davanum Srinivas <davanum@gmail.com> Containerd-test-commit: b9a98e2
When cadvisor vendors in things from containerd, there is a dependency loop between cadvisor<->containerd<->kubernetes as containerd vendors in cri-api. So we have been unable to move to newer versions of containerd that use go modules. To help with this effort in 1.6, containerd folks will publish
containerd/containerd/api
as a separate go module. To prep for this, we can cleanup our usage of containerd now to depend only on the code undercontainerd/containerd/api
by borrowing some code from containerd repository that essentially acts as a containerd client.When containerd 1.6 goes GA, we can switch over to the new
containerd/api
go module. Kubernetes also needs moby/moby#42624 from moby/moby to completely eliminate this dependency loop unless/until we get rid of dockershim (WIP for dockershim removal is here - kubernetes/kubernetes#97252). Either way we are in track to get rid of this dependency loop by kubernetes 1.24 🤞🏾 🙏🏾Signed-off-by: Davanum Srinivas davanum@gmail.com