Skip to content

Add sync option to lease removal#2493

Merged
estesp merged 2 commits intocontainerd:masterfrom
dmcgowan:sync-lease-removal
Jul 31, 2018
Merged

Add sync option to lease removal#2493
estesp merged 2 commits intocontainerd:masterfrom
dmcgowan:sync-lease-removal

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

Adds a sync option to lease deletion similar to image removal. This both provides a way to ensure synchronous cleanup of objects only referenced by a lease and an alternate way of triggering the garbage collector without requiring creating of a top level object like images.

This change required an API addition and I also included an 2 API error changes

  • Add sync option to lease deletion API, this is optional and backwards compatible
  • Update delete to return a not found error if the lease doesn't exist, this could have an impact
    on anyone who was creating leases themselves and checking the error with the expectation of
    this behavior. This should be low impact and anyone using the containerd go client will not be impacted.
  • Update lease create to return a correctly typed error on already exists. This would also only
    impact a client which had custom behavior expecting an unknown error on already exists. The client
    by default generates random ids making this change very low impact.

@dmcgowan dmcgowan added this to the 1.2 milestone Jul 24, 2018
Ensure delete returns a typed error on not found

Signed-off-by: Derek McGowan <[email protected]>
@dmcgowan dmcgowan force-pushed the sync-lease-removal branch from 254bfe1 to b760cee Compare July 25, 2018 21:56
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 25, 2018

Codecov Report

Merging #2493 into master will increase coverage by 0.04%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2493      +/-   ##
=========================================
+ Coverage   44.76%   44.8%   +0.04%     
=========================================
  Files          93      93              
  Lines        9555    9557       +2     
=========================================
+ Hits         4277    4282       +5     
+ Misses       4585    4583       -2     
+ Partials      693     692       -1
Flag Coverage Δ
#linux 49.02% <80%> (+0.03%) ⬆️
#windows 41.09% <83.33%> (+0.05%) ⬆️
Impacted Files Coverage Δ
metadata/leases.go 43.11% <83.33%> (+2.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26e2dd6...b760cee. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 4249f44 into containerd:master Jul 31, 2018
@dmcgowan dmcgowan deleted the sync-lease-removal branch September 10, 2019 17:47
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.

4 participants