Layout wraps v1 image #243

natalieparellano · 2024-01-26T18:34:30Z

No description provided.

The details of storing images can be left up to each implementation Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Natalie Arellano <narellano@vmware.com>

This will allow us to preserve the digest when no modifying options are provided Signed-off-by: Natalie Arellano <narellano@vmware.com>

… the image struct (image struct can go away) Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Natalie Arellano <narellano@vmware.com>

jjbustamante · 2024-02-01T16:49:16Z

cnb_image.go

@@ -346,9 +333,6 @@ func (i *CNBImageCore) AddLayerWithDiffIDAndHistory(path, _ string, history v1.H
 }

 func (i *CNBImageCore) Rebase(baseTopLayerDiffID string, withNewBase Image) error {
-	if i.Kind() != withNewBase.Kind() {


If my underlying image i is a remote image, does it mean I can rebase it with a local or layout image?

Yeah, in theory - I haven't tested it, but probably you can

layout/options.go

Signed-off-by: Natalie Arellano <narellano@vmware.com>

jjbustamante · 2024-02-01T17:04:53Z

I think you are still working in some TODOs but overall it looks good to me! As always, terrific job @natalieparellano!

…dia types (or other modifications) are requested Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Natalie Arellano <narellano@vmware.com>

codecov · 2024-02-02T20:36:19Z

Codecov Report

Attention: 141 lines in your changes are missing coverage. Please review.

Comparison is base (faf6f39) 58.86% compared to head (a59c8b4) 59.79%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #243      +/-   ##
==========================================
+ Coverage   58.86%   59.79%   +0.93%     
==========================================
  Files          32       32              
  Lines        3694     3369     -325     
==========================================
- Hits         2174     2014     -160     
+ Misses       1160     1044     -116     
+ Partials      360      311      -49

Signed-off-by: Natalie Arellano <narellano@vmware.com>

This reverts commit 59770c9.

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano · 2024-01-30T14:54:49Z

image.go

@@ -87,136 +85,16 @@ type Platform struct {
 	OSVersion    string
 }

-type MediaTypes int


Moved to new.go

natalieparellano · 2024-01-30T14:55:01Z

image.go

 	}
 	configFile.History = NormalizedHistory(configFile.History, len(configFile.RootFS.DiffIDs))
 	return mutate.ConfigFile(image, configFile)
 }

-func NormalizedHistory(history []v1.History, nLayers int) []v1.History {


Moved to new.go

natalieparellano · 2024-01-30T14:57:40Z

layout/new.go

-	if len(indexManifest.Manifests) > 1 {
-		// Find based on platform (os/arch)
-		for _, m := range indexManifest.Manifests {
-			if m.Platform.OS == platform.OS && m.Platform.Architecture == platform.OS {


m.Platform.Architecture == platform.OS seems unintended

natalieparellano · 2024-02-01T17:01:21Z

cnb_image.go

@@ -346,9 +333,6 @@ func (i *CNBImageCore) AddLayerWithDiffIDAndHistory(path, _ string, history v1.H
 }

 func (i *CNBImageCore) Rebase(baseTopLayerDiffID string, withNewBase Image) error {
-	if i.Kind() != withNewBase.Kind() {


Yeah, in theory - I haven't tested it, but probably you can

natalieparellano · 2024-02-05T16:39:53Z

cnb_image.go

-type ImageStore interface {
-	Contains(identifier string) bool
-	Delete(identifier string) error
-	Save(image IdentifiableV1Image, withName string, withAdditionalNames ...string) (string, error)
-	SaveFile(image IdentifiableV1Image, withName string) (string, error)
-
-	DownloadLayersFor(identifier string) error
-	Layers() []v1.Layer
-}


Originally I thought this could be an interface that all packages could implement, but since most packages would override the methods that call this interface it's not so useful

natalieparellano · 2024-02-05T16:42:26Z

layout/save.go

-
-	if i.Image, err = imgutil.OverrideHistoryIfNeeded(i.Image); err != nil {
-		return fmt.Errorf("override history: %w", err)
+	if err := i.SetCreatedAtAndHistory(); err != nil {


We have to do this at the point of save (vs instantiation) because otherwise we might modify the digest when there is a base image and that could be surprising

natalieparellano · 2024-02-05T16:43:01Z

layout/sparse/sparse_test.go

-				err = image.Save()
-				h.AssertNil(t, err)


Saving could mutate the digest and that is expected

natalieparellano · 2024-02-05T16:45:52Z

new.go

-	createdAt := NormalizedDateTime
-	if !options.CreatedAt.IsZero() {
-		createdAt = options.CreatedAt
-	}
-	if err = image.MutateConfigFile(func(c *v1.ConfigFile) {
-		c.Created = v1.Time{Time: createdAt}
-		c.Container = ""
-	}); err != nil {
-		return nil, err
-	}
-
-	if !options.PreserveHistory {
-		if err = image.MutateConfigFile(func(c *v1.ConfigFile) {
-			for j := range c.History {
-				c.History[j] = v1.History{Created: v1.Time{Time: createdAt}}
-			}
-		}); err != nil {
-			return nil, err
-		}
-	}


Moved this out of new method to avoid mutating the digest when there is a base image

natalieparellano

Worth noting, I tried importing this branch in pack and the lifecycle, and all tests are happy over there :)

Remove ImageStore interface

efd1f09

The details of storing images can be left up to each implementation Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the layout-wraps-v1 branch 6 times, most recently from a103082 to 5286195 Compare January 29, 2024 23:01

Refactor layout package to wrap a CNBImageCore

c3794cc

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the layout-wraps-v1 branch from 5286195 to c3794cc Compare January 30, 2024 14:49

natalieparellano added 4 commits January 30, 2024 10:56

Remove empty image from layout package

37c0fdc

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Set created at when we save, not when we initialize

8ca3b4a

This will allow us to preserve the digest when no modifying options are provided Signed-off-by: Natalie Arellano <narellano@vmware.com>

Fix locallayout facade by caching layer data on the layer struct, not…

b8563b8

… the image struct (image struct can go away) Signed-off-by: Natalie Arellano <narellano@vmware.com>

Fix sparse test - comparing digest is only possible before save

5a1af60

Signed-off-by: Natalie Arellano <narellano@vmware.com>

jjbustamante reviewed Feb 1, 2024

View reviewed changes

layout/options.go Outdated Show resolved Hide resolved

Remove unnecessary check on layer ordering

a65b1a9

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano marked this pull request as ready for review February 1, 2024 18:08

natalieparellano requested a review from a team as a code owner February 1, 2024 18:08

natalieparellano added 2 commits February 2, 2024 14:50

Fix: always preserve the digest when base image is provided and no me…

9cef4d4

…dia types (or other modifications) are requested Signed-off-by: Natalie Arellano <narellano@vmware.com>

Force compute

244aa16

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the layout-wraps-v1 branch 2 times, most recently from 93f1d0b to 0153fc0 Compare February 2, 2024 21:13

natalieparellano added 2 commits February 2, 2024 17:12

Add missing fixture

f737e8d

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Skip Windows

59770c9

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the layout-wraps-v1 branch from 0153fc0 to 59770c9 Compare February 2, 2024 22:21

natalieparellano added 3 commits February 5, 2024 11:24

Revert "Skip Windows"

78930ee

This reverts commit 59770c9.

Try to fix Windows

33de307

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Add docs comments

7ed58a5

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano commented Feb 5, 2024

View reviewed changes

natalieparellano requested a review from jabrown85 February 5, 2024 16:47

jabrown85 approved these changes Feb 5, 2024

View reviewed changes

Merge branch 'main' into layout-wraps-v1

a59c8b4

natalieparellano merged commit 8ca2c95 into main Feb 5, 2024
5 checks passed

natalieparellano deleted the layout-wraps-v1 branch February 5, 2024 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Layout wraps v1 image #243

Layout wraps v1 image #243

natalieparellano commented Jan 26, 2024

jjbustamante Feb 1, 2024

natalieparellano Feb 1, 2024

jjbustamante commented Feb 1, 2024

codecov bot commented Feb 2, 2024 •

edited

Loading

natalieparellano Jan 30, 2024

natalieparellano Jan 30, 2024

natalieparellano Jan 30, 2024

natalieparellano Feb 1, 2024

natalieparellano Feb 5, 2024

natalieparellano Feb 5, 2024

natalieparellano Feb 5, 2024

natalieparellano Feb 5, 2024

natalieparellano left a comment

Layout wraps v1 image #243

Layout wraps v1 image #243

Conversation

natalieparellano commented Jan 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjbustamante commented Feb 1, 2024

codecov bot commented Feb 2, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natalieparellano left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 2, 2024 •

edited

Loading