Skip to content

Commit b308097

Browse files
Roman VolosatovsthaJeztah
Roman Volosatovs
and
thaJeztah
committed
daemon/images: refactor image listing
- Rename image summary constructor - Rename `newImage` into `newImageSummary`, since the returned type is `*types.ImageSummary` - Rename variables for clarity - Rename `newImage` into `summary`, since the variable type is `*types.ImageSummary` - Rename `imagesMap` into `summaryMap`, since the value type contained is `*types.ImageSummary` - Only compute `DiffSize` when more than 1 reference to the layer exists, since it is not used otherwise - Move variable declarations closer to where they are used Signed-off-by: Roman Volosatovs <[email protected]> Co-authored-by: Sebastiaan van Stijn <[email protected]>
1 parent 2a562b1 commit b308097

1 file changed

Lines changed: 64 additions & 59 deletions

File tree

‎daemon/images/images.go‎

Lines changed: 64 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -44,30 +44,23 @@ func (i *ImageService) Map() map[image.ID]*image.Image {
4444
// named all controls whether all images in the graph are filtered, or just
4545
// the heads.
4646
func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error) {
47-
var (
48-
allImages map[image.ID]*image.Image
49-
err error
50-
danglingOnly = false
51-
)
52-
5347
if err := imageFilters.Validate(acceptedImageFilterTags); err != nil {
5448
return nil, err
5549
}
5650

51+
var danglingOnly bool
5752
if imageFilters.Contains("dangling") {
5853
if imageFilters.ExactMatch("dangling", "true") {
5954
danglingOnly = true
6055
} else if !imageFilters.ExactMatch("dangling", "false") {
6156
return nil, invalidFilter{"dangling", imageFilters.Get("dangling")}
6257
}
6358
}
64-
if danglingOnly {
65-
allImages = i.imageStore.Heads()
66-
} else {
67-
allImages = i.imageStore.Map()
68-
}
6959

70-
var beforeFilter, sinceFilter *image.Image
60+
var (
61+
beforeFilter, sinceFilter *image.Image
62+
err error
63+
)
7164
err = imageFilters.WalkValues("before", func(value string) error {
7265
beforeFilter, err = i.GetImage(value, nil)
7366
return err
@@ -84,12 +77,20 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr
8477
return nil, err
8578
}
8679

87-
images := []*types.ImageSummary{}
88-
var imagesMap map[*image.Image]*types.ImageSummary
89-
var layerRefs map[layer.ChainID]int
90-
var allLayers map[layer.ChainID]layer.Layer
91-
var allContainers []*container.Container
80+
var allImages map[image.ID]*image.Image
81+
if danglingOnly {
82+
allImages = i.imageStore.Heads()
83+
} else {
84+
allImages = i.imageStore.Map()
85+
}
9286

87+
var (
88+
summaries []*types.ImageSummary
89+
summaryMap map[*image.Image]*types.ImageSummary
90+
layerRefs map[layer.ChainID]int
91+
allLayers map[layer.ChainID]layer.Layer
92+
allContainers []*container.Container
93+
)
9394
for id, img := range allImages {
9495
if beforeFilter != nil {
9596
if img.Created.Equal(beforeFilter.Created) || img.Created.After(beforeFilter.Created) {
@@ -121,9 +122,8 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr
121122
continue
122123
}
123124

124-
layerID := img.RootFS.ChainID()
125125
var size int64
126-
if layerID != "" {
126+
if layerID := img.RootFS.ChainID(); layerID != "" {
127127
l, err := i.layerStore.Get(layerID)
128128
if err != nil {
129129
// The layer may have been deleted between the call to `Map()` or
@@ -141,7 +141,7 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr
141141
}
142142
}
143143

144-
newImage := newImage(img, size)
144+
summary := newImageSummary(img, size)
145145

146146
for _, ref := range i.referenceStore.References(id.Digest()) {
147147
if imageFilters.Contains("reference") {
@@ -161,13 +161,13 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr
161161
}
162162
}
163163
if _, ok := ref.(reference.Canonical); ok {
164-
newImage.RepoDigests = append(newImage.RepoDigests, reference.FamiliarString(ref))
164+
summary.RepoDigests = append(summary.RepoDigests, reference.FamiliarString(ref))
165165
}
166166
if _, ok := ref.(reference.NamedTagged); ok {
167-
newImage.RepoTags = append(newImage.RepoTags, reference.FamiliarString(ref))
167+
summary.RepoTags = append(summary.RepoTags, reference.FamiliarString(ref))
168168
}
169169
}
170-
if newImage.RepoDigests == nil && newImage.RepoTags == nil {
170+
if summary.RepoDigests == nil && summary.RepoTags == nil {
171171
if all || len(i.imageStore.Children(id)) == 0 {
172172

173173
if imageFilters.Contains("dangling") && !danglingOnly {
@@ -177,75 +177,75 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr
177177
if imageFilters.Contains("reference") { // skip images with no references if filtering by reference
178178
continue
179179
}
180-
newImage.RepoDigests = []string{"<none>@<none>"}
181-
newImage.RepoTags = []string{"<none>:<none>"}
180+
summary.RepoDigests = []string{"<none>@<none>"}
181+
summary.RepoTags = []string{"<none>:<none>"}
182182
} else {
183183
continue
184184
}
185-
} else if danglingOnly && len(newImage.RepoTags) > 0 {
185+
} else if danglingOnly && len(summary.RepoTags) > 0 {
186186
continue
187187
}
188188

189189
if withExtraAttrs {
190190
// lazily init variables
191-
if imagesMap == nil {
191+
if summaryMap == nil {
192192
allContainers = i.containers.List()
193193
allLayers = i.layerStore.Map()
194-
imagesMap = make(map[*image.Image]*types.ImageSummary)
194+
summaryMap = make(map[*image.Image]*types.ImageSummary)
195195
layerRefs = make(map[layer.ChainID]int)
196196
}
197197

198198
// Get container count
199-
newImage.Containers = 0
199+
var containers int64
200200
for _, c := range allContainers {
201201
if c.ImageID == id {
202-
newImage.Containers++
202+
containers++
203203
}
204204
}
205+
// NOTE: By default, Containers is -1, or "not set"
206+
summary.Containers = containers
205207

206208
// count layer references
207209
rootFS := *img.RootFS
208210
rootFS.DiffIDs = nil
209211
for _, id := range img.RootFS.DiffIDs {
210212
rootFS.Append(id)
211-
chid := rootFS.ChainID()
212-
layerRefs[chid]++
213-
if _, ok := allLayers[chid]; !ok {
214-
return nil, fmt.Errorf("layer %v was not found (corruption?)", chid)
215-
}
213+
layerRefs[rootFS.ChainID()]++
216214
}
217-
imagesMap[img] = newImage
215+
summaryMap[img] = summary
218216
}
219-
220-
images = append(images, newImage)
217+
summaries = append(summaries, summary)
221218
}
222219

223220
if withExtraAttrs {
224221
// Get Shared sizes
225-
for img, newImage := range imagesMap {
222+
for img, summary := range summaryMap {
226223
rootFS := *img.RootFS
227224
rootFS.DiffIDs = nil
228225

229-
newImage.SharedSize = 0
226+
// Indicate that we collected shared size information (default is -1, or "not set")
227+
summary.SharedSize = 0
230228
for _, id := range img.RootFS.DiffIDs {
231229
rootFS.Append(id)
232230
chid := rootFS.ChainID()
233231

234-
diffSize, err := allLayers[chid].DiffSize()
235-
if err != nil {
236-
return nil, err
237-
}
238-
239232
if layerRefs[chid] > 1 {
240-
newImage.SharedSize += diffSize
233+
if _, ok := allLayers[chid]; !ok {
234+
return nil, fmt.Errorf("layer %v was not found (corruption?)", chid)
235+
}
236+
diffSize, err := allLayers[chid].DiffSize()
237+
if err != nil {
238+
return nil, err
239+
}
240+
summary.SharedSize += diffSize
241241
}
242242
}
243243
}
244244
}
245245

246-
sort.Sort(sort.Reverse(byCreated(images)))
246+
sort.Sort(sort.Reverse(byCreated(summaries)))
247247

248-
return images, nil
248+
return summaries, nil
249249
}
250250

251251
// SquashImage creates a new image with the diff of the specified image and the specified parent.
@@ -335,17 +335,22 @@ func (i *ImageService) SquashImage(id, parent string) (string, error) {
335335
return string(newImgID), nil
336336
}
337337

338-
func newImage(image *image.Image, size int64) *types.ImageSummary {
339-
newImage := new(types.ImageSummary)
340-
newImage.ParentID = image.Parent.String()
341-
newImage.ID = image.ID().String()
342-
newImage.Created = image.Created.Unix()
343-
newImage.Size = size
344-
newImage.VirtualSize = size
345-
newImage.SharedSize = -1
346-
newImage.Containers = -1
338+
func newImageSummary(image *image.Image, size int64) *types.ImageSummary {
339+
summary := &types.ImageSummary{
340+
ParentID: image.Parent.String(),
341+
ID: image.ID().String(),
342+
Created: image.Created.Unix(),
343+
Size: size,
344+
VirtualSize: size,
345+
// -1 indicates that the value has not been set (avoids ambiguity
346+
// between 0 (default) and "not set". We cannot use a pointer (nil)
347+
// for this, as the JSON representation uses "omitempty", which would
348+
// consider both "0" and "nil" to be "empty".
349+
SharedSize: -1,
350+
Containers: -1,
351+
}
347352
if image.Config != nil {
348-
newImage.Labels = image.Config.Labels
353+
summary.Labels = image.Config.Labels
349354
}
350-
return newImage
355+
return summary
351356
}

0 commit comments

Comments
 (0)