Skip to content

Update google.golang.org/grpc and github.com/golang/protobuf#5613

Merged
dims merged 3 commits intocontainerd:masterfrom
kzys:upgrade-grpc
Jun 17, 2021
Merged

Update google.golang.org/grpc and github.com/golang/protobuf#5613
dims merged 3 commits intocontainerd:masterfrom
kzys:upgrade-grpc

Conversation

@kzys
Copy link
Member

@kzys kzys commented Jun 16, 2021

google.golang.org/grpc v1.38.0 is used by Kubernetes since
kubernetes/kubernetes#100488.

Signed-off-by: Kazuyoshi Kato katokazu@amazon.com

@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 16, 2021

Build succeeded.

@kzys kzys marked this pull request as draft June 16, 2021 18:50
Kazuyoshi Kato added 2 commits June 16, 2021 12:05
v1.38.0 is used by Kubernetes since
kubernetes/kubernetes#100488.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
google.golang.org/grpc doesn't work with protobuf v1.3.5.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 16, 2021

Build succeeded.

1 similar comment
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 16, 2021

Build succeeded.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 16, 2021

Build succeeded.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 16, 2021

Build succeeded.

@kzys kzys changed the title Update google.golang.org/grpc from v1.27.1 to v1.38.0 Update google.golang.org/grpc and github.com/golang/protobuf Jun 16, 2021
@kzys kzys requested review from dims and mikebrow June 16, 2021 19:35
@kzys kzys marked this pull request as ready for review June 16, 2021 19:35
@kzys kzys added this to the 1.6 milestone Jun 16, 2021
@kzys
Copy link
Member Author

kzys commented Jun 16, 2021

TestContentClient has been flaky (see #4927 and #5430).

Copy link
Member

@dims dims left a comment

Choose a reason for hiding this comment

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

the version updates are good:

[dims@dims-a01 17:01] ~/go/src/k8s.io/kubernetes ⟩ rg "(google.golang.org/grpc|google.golang.org/protobuf)" go.mod | grep -v "="
google.golang.org/grpc v1.38.0
google.golang.org/protobuf v1.26.0

LGTM

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.

LGTM

@dims dims merged commit 1bbee57 into containerd:master Jun 17, 2021
@zhsj
Copy link
Contributor

zhsj commented Jun 20, 2021

Hi, I'm curious about this change. Does it mean containerd is now no longer affected by containerd/ttrpc#62 ?

@zhsj
Copy link
Contributor

zhsj commented Jun 20, 2021

This PR forgot to sync go.mod in integration/client/go.mod, so the new protobuf is not tested during intergation test. I try to run the integration in my repo zhsj@0fa92b5 and it seems the test fails. However the log doesn't give any useful message, so I'm not sure if it's related to protobuf updates. (Probably currently there's issue with GH infra, since containerd master also fails without useful logs)

@thaJeztah
Copy link
Member

thaJeztah commented Jun 20, 2021

Also curious if this won't have side effects; I should probably dust off #5208

@kzys
Copy link
Member Author

kzys commented Jun 21, 2021

This PR forgot to sync go.mod in integration/client/go.mod, so the new protobuf is not tested during intergation test.

Ah, sorry. #5623 will address that (thanks @dims!).

@kzys
Copy link
Member Author

kzys commented Jun 21, 2021

Does it mean containerd is now no longer affected by containerd/ttrpc#62 ?

I believe these replace directives (github.com/gogo/googleapis and google.golang.org/genproto) are the workaround we have for the issue. Let me confirm.

containerd/go.mod

Lines 76 to 80 in c07711c

github.com/gogo/googleapis => github.com/gogo/googleapis v1.3.2
// urfave/cli must be <= v1.22.1 due to a regression: https://github.com/urfave/cli/issues/1092
github.com/urfave/cli => github.com/urfave/cli v1.22.1
google.golang.org/genproto => google.golang.org/genproto v0.0.0-20200224152610-e50cd9704f63
)

@thaJeztah
Copy link
Member

The github.com/gogo/googleapis looks like it's redundant (v1.3.2 is their latest release), but perhaps the genproto one is related (?)

I'm mostly concerned about making sure we remain backward compatible with (older) clients; I'm not very familiar with the issues, but I know there were some hairy situations, and want to be sure existing clients don't break in unexpected ways when using the containerd API (otherwise it would require a major version update of the containerd API).

@brandond
Copy link
Contributor

brandond commented Aug 20, 2021

We embed containerd and kubernetes together in a single binary in k3s, and we're running into containerd/ttrpc#62 after updating Kubernetes to 1.22, as it requires grpc v1.32.0 or newer. It seems like someone needs to bite the bullet on updating ttrpc to modern grpc?

TBBle added a commit to TBBle/hcsshim that referenced this pull request Sep 26, 2021
This was originally locked in microsoft#1000 based on containerd's resolution for
containerd/ttrpc#62. However, this was dropped
in containerd/containerd#5613 while upgrading
libraries we don't actually use, so it was probably never applicable for
us.

See also containerd/ttrpc#89, which documents
that only google.golang.org/genproto needs to be locked.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
TBBle added a commit to TBBle/hcsshim that referenced this pull request Nov 27, 2021
This was originally locked in microsoft#1000 based on containerd's resolution for
containerd/ttrpc#62. However, this was dropped
in containerd/containerd#5613 while upgrading
libraries we don't actually use, so it was probably never applicable for
us.

See also containerd/ttrpc#89, which documents
that only google.golang.org/genproto needs to be locked.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
TBBle added a commit to TBBle/hcsshim that referenced this pull request May 4, 2022
This was originally locked in microsoft#1000 based on containerd's resolution for
containerd/ttrpc#62. However, this was dropped
in containerd/containerd#5613 while upgrading
libraries we don't actually use, so it was probably never applicable for
us.

See also containerd/ttrpc#89, which documents
that only google.golang.org/genproto needs to be locked.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
TBBle added a commit to TBBle/hcsshim that referenced this pull request May 4, 2022
This was originally locked in microsoft#1000 based on containerd's resolution for
containerd/ttrpc#62. However, this was dropped
in containerd/containerd#5613 while upgrading
libraries we don't actually use, so it was probably never applicable for
us.

See also containerd/ttrpc#89, which documents
that only google.golang.org/genproto needs to be locked.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants