Skip to content

Fix order of operations when setting lease labels#7702

Merged
dmcgowan merged 1 commit intocontainerd:mainfrom
austinvazquez:resolve-order-of-operations-in-leases
Nov 30, 2022
Merged

Fix order of operations when setting lease labels#7702
dmcgowan merged 1 commit intocontainerd:mainfrom
austinvazquez:resolve-order-of-operations-in-leases

Conversation

@austinvazquez
Copy link
Copy Markdown
Member

@austinvazquez austinvazquez commented Nov 19, 2022

Fixes an edge case where WithLabels would overwrite WithExpiration on a lease when using both client options and a expire value is not provided via labels. A user may expect the maps to be merged.

For example, see https://github.com/containerd/containerd/blob/main/cmd/ctr/commands/images/mount.go#L75-L81

Signed-off-by: Austin Vazquez [email protected]

@k8s-ci-robot
Copy link
Copy Markdown

Hi @austinvazquez. 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/test-infra repository.

@austinvazquez austinvazquez marked this pull request as ready for review November 19, 2022 16:25
Comment thread leases/lease_test.go
Comment thread leases/lease.go
@austinvazquez austinvazquez force-pushed the resolve-order-of-operations-in-leases branch 2 times, most recently from b0a1057 to b32a4db Compare November 22, 2022 17:16
Fixes an edge case where `WithLabels` would overwrite `WithExpiration`
on a lease when using client options.

Signed-off-by: Austin Vazquez <[email protected]>
@austinvazquez austinvazquez force-pushed the resolve-order-of-operations-in-leases branch from b32a4db to c4dee23 Compare November 22, 2022 20:50
@dmcgowan dmcgowan merged commit 40a9464 into containerd:main Nov 30, 2022
@austinvazquez austinvazquez deleted the resolve-order-of-operations-in-leases branch December 1, 2022 21:26
@estesp estesp added cherry-picked/1.5.x PR commits are cherry-picked into release/1.5 branch cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch and removed needs-ok-to-test labels Dec 1, 2022
@estesp
Copy link
Copy Markdown
Member

estesp commented Dec 1, 2022

Cherry-picked into release/1.5 in PR #7746
Cherry-picked into release/1.6 in PR #7745

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked/1.5.x PR commits are cherry-picked into release/1.5 branch cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants