Skip to content

Add a manifest filter limiting the number of matches#2586

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
dmcgowan:platform-filter-limit
Aug 28, 2018
Merged

Add a manifest filter limiting the number of matches#2586
crosbymichael merged 1 commit intocontainerd:masterfrom
dmcgowan:platform-filter-limit

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan commented Aug 27, 2018

Adds a manifest filter for pulling which ensures only a manifest from a manifest list is pulled even when multiple matches. Removes unused filter platform list.

This prevents pulling multiple variants on arm if multiple are supported and included in an image, but only one is needed. This could also prevent pulling both Linux and Window images once LCOW support is added and only 1 image is required.

@dmcgowan dmcgowan force-pushed the platform-filter-limit branch 2 times, most recently from 1dd8532 to 4d6ff83 Compare August 27, 2018 22:08
@dmcgowan dmcgowan changed the title Add a platform filter limiting the number of matches Add a manifest filter limiting the number of matches Aug 27, 2018
@dmcgowan
Copy link
Copy Markdown
Member Author

Updated the implementation to just make a new handler wrapper which only operates on manifest lists and indexes since those are the types which need to have the limit applied.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 27, 2018

Codecov Report

Merging #2586 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2586   +/-   ##
=======================================
  Coverage   44.07%   44.07%           
=======================================
  Files          96       96           
  Lines       10125    10125           
=======================================
  Hits         4463     4463           
  Misses       4940     4940           
  Partials      722      722
Flag Coverage Δ
#linux 47.79% <ø> (ø) ⬆️
#windows 40.98% <ø> (ø) ⬆️

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 be42d77...3d1082e. Read the comment docs.

Comment thread images/handlers.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include a statement here about favoring entries that appear first in the manifests.

Adds a manifest filter for pulling which ensures only one
manifest from a manifest list is pulled even when multiple matches.
Removes unused filter platform list.

Signed-off-by: Derek McGowan <[email protected]>
@dmcgowan dmcgowan force-pushed the platform-filter-limit branch from 4d6ff83 to 3d1082e Compare August 28, 2018 00:04
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2586 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2586   +/-   ##
=======================================
  Coverage   44.07%   44.07%           
=======================================
  Files          96       96           
  Lines       10125    10125           
=======================================
  Hits         4463     4463           
  Misses       4940     4940           
  Partials      722      722
Flag Coverage Δ
#linux 47.79% <ø> (ø) ⬆️
#windows 40.98% <ø> (ø) ⬆️

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 be42d77...3d1082e. Read the comment docs.

2 similar comments
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2586 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2586   +/-   ##
=======================================
  Coverage   44.07%   44.07%           
=======================================
  Files          96       96           
  Lines       10125    10125           
=======================================
  Hits         4463     4463           
  Misses       4940     4940           
  Partials      722      722
Flag Coverage Δ
#linux 47.79% <ø> (ø) ⬆️
#windows 40.98% <ø> (ø) ⬆️

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 be42d77...3d1082e. Read the comment docs.

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2586 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2586   +/-   ##
=======================================
  Coverage   44.07%   44.07%           
=======================================
  Files          96       96           
  Lines       10125    10125           
=======================================
  Hits         4463     4463           
  Misses       4940     4940           
  Partials      722      722
Flag Coverage Δ
#linux 47.79% <ø> (ø) ⬆️
#windows 40.98% <ø> (ø) ⬆️

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 be42d77...3d1082e. Read the comment docs.

@dmcgowan dmcgowan added this to the 1.2 milestone Aug 28, 2018
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

@ehazlett
Copy link
Copy Markdown
Member

LGTM

@crosbymichael crosbymichael merged commit 68979c9 into containerd:master Aug 28, 2018
@dmcgowan dmcgowan deleted the platform-filter-limit branch September 10, 2019 17:43
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