Skip to content

Commit 2a6ff3c

Browse files
tianontianon
committed
Use OCI "History" type instead of inventing our own copy
The most notable change here is that the OCI's type uses a pointer for `Created`, which we probably should've been too, so most of these changes are accounting for that (and embedding our `Equal` implementation in the one single place it was used). Signed-off-by: Tianon Gravi <[email protected]>
1 parent 4bef3e9 commit 2a6ff3c

12 files changed

Lines changed: 62 additions & 114 deletions

File tree

‎api/server/router/image/image_routes.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,13 +294,18 @@ func (ir *imageRouter) toImageInspect(img *image.Image) (*types.ImageInspect, er
294294
repoDigests = []string{}
295295
}
296296

297+
var created string
298+
if img.Created != nil {
299+
created = img.Created.Format(time.RFC3339Nano)
300+
}
301+
297302
return &types.ImageInspect{
298303
ID: img.ID().String(),
299304
RepoTags: repoTags,
300305
RepoDigests: repoDigests,
301306
Parent: img.Parent.String(),
302307
Comment: comment,
303-
Created: img.Created.Format(time.RFC3339Nano),
308+
Created: created,
304309
Container: img.Container,
305310
ContainerConfig: &img.ContainerConfig,
306311
DockerVersion: img.DockerVersion,

‎daemon/containerd/image.go‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -68,30 +68,12 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
6868
exposedPorts[nat.Port(k)] = v
6969
}
7070

71-
derefTimeSafely := func(t *time.Time) time.Time {
72-
if t != nil {
73-
return *t
74-
}
75-
return time.Time{}
76-
}
77-
78-
var imgHistory []image.History
79-
for _, h := range ociimage.History {
80-
imgHistory = append(imgHistory, image.History{
81-
Created: derefTimeSafely(h.Created),
82-
Author: h.Author,
83-
CreatedBy: h.CreatedBy,
84-
Comment: h.Comment,
85-
EmptyLayer: h.EmptyLayer,
86-
})
87-
}
88-
8971
img := image.NewImage(image.ID(desc.Digest))
9072
img.V1Image = image.V1Image{
9173
ID: string(desc.Digest),
9274
OS: ociimage.OS,
9375
Architecture: ociimage.Architecture,
94-
Created: derefTimeSafely(ociimage.Created),
76+
Created: ociimage.Created,
9577
Config: &containertypes.Config{
9678
Entrypoint: ociimage.Config.Entrypoint,
9779
Env: ociimage.Config.Env,
@@ -106,7 +88,7 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
10688
}
10789

10890
img.RootFS = rootfs
109-
img.History = imgHistory
91+
img.History = ociimage.History
11092

11193
if options.Details {
11294
lastUpdated := time.Unix(0, 0)

‎daemon/containerd/image_builder.go‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -402,21 +402,9 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
402402
exposedPorts[string(k)] = v
403403
}
404404

405-
var ociHistory []ocispec.History
406-
for _, history := range imgToCreate.History {
407-
created := history.Created
408-
ociHistory = append(ociHistory, ocispec.History{
409-
Created: &created,
410-
CreatedBy: history.CreatedBy,
411-
Author: history.Author,
412-
Comment: history.Comment,
413-
EmptyLayer: history.EmptyLayer,
414-
})
415-
}
416-
417405
// make an ocispec.Image from the docker/image.Image
418406
ociImgToCreate := ocispec.Image{
419-
Created: &imgToCreate.Created,
407+
Created: imgToCreate.Created,
420408
Author: imgToCreate.Author,
421409
Platform: ocispec.Platform{
422410
Architecture: imgToCreate.Architecture,
@@ -437,7 +425,7 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
437425
StopSignal: imgToCreate.Config.StopSignal,
438426
},
439427
RootFS: rootfs,
440-
History: ociHistory,
428+
History: imgToCreate.History,
441429
}
442430

443431
var layers []ocispec.Descriptor

‎daemon/images/image_import.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ func (i *ImageService) ImportImage(ctx context.Context, newRef reference.Named,
5858
Architecture: platform.Architecture,
5959
Variant: platform.Variant,
6060
OS: platform.OS,
61-
Created: created,
61+
Created: &created,
6262
Comment: msg,
6363
},
6464
RootFS: &image.RootFS{
6565
Type: "layers",
6666
DiffIDs: []layer.DiffID{l.DiffID()},
6767
},
6868
History: []image.History{{
69-
Created: created,
69+
Created: &created,
7070
Comment: msg,
7171
}},
7272
})

‎daemon/images/image_list.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
5353
}
5454
// Resolve multiple values to the oldest image,
5555
// equivalent to ANDing all the values together.
56-
if beforeFilter.IsZero() || beforeFilter.After(img.Created) {
57-
beforeFilter = img.Created
56+
if img.Created != nil && (beforeFilter.IsZero() || beforeFilter.After(*img.Created)) {
57+
beforeFilter = *img.Created
5858
}
5959
return nil
6060
})
@@ -69,8 +69,8 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
6969
}
7070
// Resolve multiple values to the newest image,
7171
// equivalent to ANDing all the values together.
72-
if sinceFilter.Before(img.Created) {
73-
sinceFilter = img.Created
72+
if img.Created != nil && sinceFilter.Before(*img.Created) {
73+
sinceFilter = *img.Created
7474
}
7575
return nil
7676
})
@@ -97,10 +97,10 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
9797
default:
9898
}
9999

100-
if !beforeFilter.IsZero() && !img.Created.Before(beforeFilter) {
100+
if !beforeFilter.IsZero() && (img.Created == nil || !img.Created.Before(beforeFilter)) {
101101
continue
102102
}
103-
if !sinceFilter.IsZero() && !img.Created.After(sinceFilter) {
103+
if !sinceFilter.IsZero() && (img.Created == nil || !img.Created.After(sinceFilter)) {
104104
continue
105105
}
106106

@@ -256,10 +256,14 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
256256
}
257257

258258
func newImageSummary(image *image.Image, size int64) *types.ImageSummary {
259+
var created int64
260+
if image.Created != nil {
261+
created = image.Created.Unix()
262+
}
259263
summary := &types.ImageSummary{
260264
ParentID: image.Parent.String(),
261265
ID: image.ID().String(),
262-
Created: image.Created.Unix(),
266+
Created: created,
263267
Size: size,
264268
// -1 indicates that the value has not been set (avoids ambiguity
265269
// between 0 (default) and "not set". We cannot use a pointer (nil)

‎daemon/images/image_prune.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (i *ImageService) ImagesPrune(ctx context.Context, pruneFilters filters.Arg
7575
if len(i.referenceStore.References(dgst)) == 0 && len(i.imageStore.Children(id)) != 0 {
7676
continue
7777
}
78-
if !until.IsZero() && img.Created.After(until) {
78+
if !until.IsZero() && (img.Created == nil || img.Created.After(until)) {
7979
continue
8080
}
8181
if img.Config != nil && !matchLabels(pruneFilters, img.Config.Labels) {

‎daemon/images/image_squash.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ func (i *ImageService) SquashImage(id, parent string) (string, error) {
7676
}
7777

7878
newImage.History = append(newImage.History, image.History{
79-
Created: now,
79+
Created: &now,
8080
Comment: historyComment,
8181
})
82-
newImage.Created = now
82+
newImage.Created = &now
8383

8484
b, err := json.Marshal(&newImage)
8585
if err != nil {

‎image/cache/cache.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
227227

228228
if compare(&img.ContainerConfig, config) {
229229
// check for the most up to date match
230-
if match == nil || match.Created.Before(img.Created) {
230+
if img.Created != nil && (match == nil || match.Created.Before(*img.Created)) {
231231
match = img
232232
}
233233
}

‎image/image.go‎

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"encoding/json"
55
"errors"
66
"io"
7-
"reflect"
87
"runtime"
98
"strings"
109
"time"
@@ -53,7 +52,7 @@ type V1Image struct {
5352
Comment string `json:"comment,omitempty"`
5453

5554
// Created is the timestamp at which the image was created
56-
Created time.Time `json:"created"`
55+
Created *time.Time `json:"created"`
5756

5857
// Container is the ID of the container that was used to create the image.
5958
//
@@ -261,44 +260,21 @@ func NewChildImage(img *Image, child ChildConfig, os string) *Image {
261260
}
262261

263262
// History stores build commands that were used to create an image
264-
type History struct {
265-
// Created is the timestamp at which the image was created
266-
Created time.Time `json:"created"`
267-
// Author is the name of the author that was specified when committing the
268-
// image, or as specified through MAINTAINER (deprecated) in the Dockerfile.
269-
Author string `json:"author,omitempty"`
270-
// CreatedBy keeps the Dockerfile command used while building the image
271-
CreatedBy string `json:"created_by,omitempty"`
272-
// Comment is the commit message that was set when committing the image
273-
Comment string `json:"comment,omitempty"`
274-
// EmptyLayer is set to true if this history item did not generate a
275-
// layer. Otherwise, the history item is associated with the next
276-
// layer in the RootFS section.
277-
EmptyLayer bool `json:"empty_layer,omitempty"`
278-
}
263+
type History = ocispec.History
279264

280265
// NewHistory creates a new history struct from arguments, and sets the created
281266
// time to the current time in UTC
282267
func NewHistory(author, comment, createdBy string, isEmptyLayer bool) History {
268+
now := time.Now().UTC()
283269
return History{
284270
Author: author,
285-
Created: time.Now().UTC(),
271+
Created: &now,
286272
CreatedBy: createdBy,
287273
Comment: comment,
288274
EmptyLayer: isEmptyLayer,
289275
}
290276
}
291277

292-
// Equal compares two history structs for equality
293-
func (h History) Equal(i History) bool {
294-
if !h.Created.Equal(i.Created) {
295-
return false
296-
}
297-
i.Created = h.Created
298-
299-
return reflect.DeepEqual(h, i)
300-
}
301-
302278
// Exporter provides interface for loading and saving images
303279
type Exporter interface {
304280
Load(io.ReadCloser, io.Writer, bool) error

‎image/image_test.go‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -56,29 +56,6 @@ func TestMarshalKeyOrder(t *testing.T) {
5656
}
5757
}
5858

59-
const sampleHistoryJSON = `{
60-
"created": "2021-01-13T09:35:56Z",
61-
"created_by": "image_test.go"
62-
}`
63-
64-
func TestHistoryEqual(t *testing.T) {
65-
h := historyFromJSON(t, sampleHistoryJSON)
66-
hCopy := h
67-
assert.Check(t, h.Equal(hCopy))
68-
69-
hUTC := historyFromJSON(t, `{"created": "2021-01-13T14:00:00Z"}`)
70-
hOffset0 := historyFromJSON(t, `{"created": "2021-01-13T14:00:00+00:00"}`)
71-
assert.Check(t, hUTC.Created != hOffset0.Created)
72-
assert.Check(t, hUTC.Equal(hOffset0))
73-
}
74-
75-
func historyFromJSON(t *testing.T, historyJSON string) History {
76-
var h History
77-
err := json.Unmarshal([]byte(historyJSON), &h)
78-
assert.Check(t, err)
79-
return h
80-
}
81-
8259
func TestImage(t *testing.T) {
8360
cid := "50a16564e727"
8461
config := &container.Config{

0 commit comments

Comments
 (0)