Skip to content

client/New: Don't unlazy the gRPC connection implicitly#11509

Merged
dmcgowan merged 1 commit intocontainerd:mainfrom
vvoland:client-lazy-runtime
Jul 9, 2025
Merged

client/New: Don't unlazy the gRPC connection implicitly#11509
dmcgowan merged 1 commit intocontainerd:mainfrom
vvoland:client-lazy-runtime

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented Mar 7, 2025

When moving to gRPC 1.64 (commit 63b4688) the usage of the deprecated grpc.DialContext was replaced with grpc.NewClient. However, this change also required to drop the WithBlock option, which made sure that the connection is actually established before returning.

Now, grpc.NewClient doesn't attempt to perform the connection but defers it to the actual first RPC.

Querying the default runtime on client creation breaks that property depending on whether the default namespace is set or not.

This commit moves the runtime field initialization behind a sync.Once and defers it to the first time the field actually needs to be used.

@k8s-ci-robot
Copy link

Hi @vvoland. 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/client Go client label Mar 7, 2025
@vvoland vvoland changed the title client/New: Don't unlazy the gRPC connection when creating client/New: Don't implicitly unlazy the gRPC connection when creating Mar 7, 2025
@vvoland vvoland force-pushed the client-lazy-runtime branch from 5087ee1 to 22e2308 Compare March 7, 2025 17:09
@vvoland vvoland changed the title client/New: Don't implicitly unlazy the gRPC connection when creating client/New: Don't unlazy the gRPC connection implicitly Mar 7, 2025
@vvoland vvoland force-pushed the client-lazy-runtime branch from 22e2308 to e88841d Compare March 10, 2025 17:08
@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Update in Pull Request Review Mar 10, 2025
When moving to gRPC 1.64 (commit 63b4688) the usage of the deprecated
`grpc.DialContext` was replaced with `grpc.NewClient`. However, this
change also required to drop the `WithBlock` option, which made sure
that the connection is actually established before returning.

Now, `grpc.NewClient` doesn't attempt to perform the connection but
defers it to the actual first RPC.

Querying the default runtime on client creation breaks that property
depending on whether the default namespace is set or not.

This commit defers the `runtime` field initialization to the first time
the field is actually needed.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the client-lazy-runtime branch from e88841d to 8bc62da Compare March 11, 2025 12:47
@vvoland vvoland requested a review from cpuguy83 March 17, 2025 15:36
@vvoland vvoland requested a review from dmcgowan May 27, 2025 13:10
@estesp estesp added cherry-pick/2.0.x Change to be cherry picked to release/2.0 branch cherry-pick/2.1.x Change to be cherry picked to release/2.1 branch and removed needs-ok-to-test labels Jul 9, 2025
@dmcgowan dmcgowan added this pull request to the merge queue Jul 9, 2025
Merged via the queue into containerd:main with commit f8fc469 Jul 9, 2025
56 checks passed
@github-project-automation github-project-automation bot moved this from Needs Update to Done in Pull Request Review Jul 9, 2025
@estesp
Copy link
Member

estesp commented Jul 9, 2025

/cherry-pick release/2.1

@estesp
Copy link
Member

estesp commented Jul 9, 2025

/cherry-pick release/2.0

@k8s-infra-cherrypick-robot

@estesp: new pull request created: #12079

Details

In response to this:

/cherry-pick release/2.1

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-infra-cherrypick-robot

@estesp: new pull request created: #12080

Details

In response to this:

/cherry-pick release/2.0

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.

@estesp estesp added cherry-picked/2.0.x PR commits are cherry picked into the release/2.0 branch cherry-picked/2.1.x PR commits are cherry picked into the release/2.1 branch and removed cherry-pick/2.0.x Change to be cherry picked to release/2.0 branch cherry-pick/2.1.x Change to be cherry picked to release/2.1 branch labels Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/client Go client cherry-picked/2.0.x PR commits are cherry picked into the release/2.0 branch cherry-picked/2.1.x PR commits are cherry picked into the release/2.1 branch size/M

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants