Add support for fake, and speed up reproducible builds #12

mafredri · 2024-05-28T11:38:59Z

See this in use here: coder/envbuilder#213

This PR adds support for fake builds, essentially a way to detect if a build is cached and what the final image hash should be.

The final image hash requires reproducible builds.

Anothe option to using reproducible builds is to instead tag the final image with the final build step hash.

Part of coder/envbuilder#186

Example output from cache hit (DoFakeBuild):

#2: fakeStage 0 built successfully with digest sha256:d76c332b89231eb2b8324e28f53ea4f5a394231ee76efdc2a8c8ad11ffd0891b
#2: 🏗️ Built fake image! [1.245278609s]

DoBuild:

#2: Stage 0 built successfully with digest sha256:d76c332b89231eb2b8324e28f53ea4f5a394231ee76efdc2a8c8ad11ffd0891b
#3: 🏗️ Built image! [2.020869675s]
#4: 🏗️ Pushing image...
#2: Pushing image to 172.17.0.3:5000/local/cache
#2: Pushed 172.17.0.3:5000/local/cache@sha256:d76c332b89231eb2b8324e28f53ea4f5a394231ee76efdc2a8c8ad11ffd0891b
#4: 🏗️ Pushed image! [6.134857ms]

Example output from cache miss:

#2: failed to build fake image: error fake building stage: uncached command *commands.RunMarkerCommand is not supported in fake build
#2: 🏗️ Built fake image! [1.232616498s]

dannykopping · 2024-05-31T09:12:33Z

pkg/executor/build.go

@@ -444,6 +449,92 @@ func (s *stageBuilder) build() error {
 	return nil
 }

+// fakeBuild is like build(), but does not actually execute the commands or
+// extract files.
+func (s *stageBuilder) fakeBuild() error {


AFAICS this aligns with the implementation of build(). I'm concerned that this will drift; how do you plan to keep these two in lock-step?

As an aside: I think the term "fake" is a problematic one.

This PR adds support for fake builds, essentially a way to detect if a build is cached and what the final image hash should be.

Perhaps renaming it to something (admittedly less pithy) like "cacheProbeBuild" or "preemptiveBuild" might be more clear?

The term "fake" is quite overloaded and I don't think it expresses the intent well here.

That's a fair concern, one I have as well. I don't really have a plan for ensuring they're kept in sync other than adding tests to verify a build and "fakeBuild" produce the same (or not) hash in the end.

I also agree with you on "fake", and "cacheProbe" is actually a pretty good one, thanks for the suggestions.

johnstcn

LGTM so far, but I think this needs some more unit tests before merging.

johnstcn · 2024-05-31T09:10:42Z

pkg/util/tar_util.go

+		// PAX and GNU Format support additional timestamps in the header
+		if hdr.Format == tar.FormatPAX || hdr.Format == tar.FormatGNU {
+			hdr.AccessTime = ct
+			hdr.ChangeTime = ct
+		}


If canonical is set and we encounter other formats (such as formatSTAR or formatMax), should we throw an error?

I'm not sure, I don't know how we'd encounter that, actually 🤔. This was borrowed from:

kaniko/vendor/github.com/google/go-containerregistry/pkg/v1/mutate/mutate.go

Lines 471 to 475 in a8afc79

//PAX and GNU Format support additional timestamps in the header

if header.Format == tar.FormatPAX || header.Format == tar.FormatGNU {

header.AccessTime = t

header.ChangeTime = t

}

johnstcn · 2024-05-31T09:12:50Z

pkg/util/tar_util.go

+func NewCanonicalTar(f io.Writer) Tar {
+	w := tar.NewWriter(f)
+	return Tar{
+		w:         w,
+		hardlinks: map[uint64]string{},
+		canonical: true,
+	}
+}


This may require extra tests in tar_util_test.go

johnstcn · 2024-05-31T09:20:28Z

pkg/executor/build.go

@@ -444,6 +449,92 @@ func (s *stageBuilder) build() error {
 	return nil
 }

+// fakeBuild is like build(), but does not actually execute the commands or
+// extract files.
+func (s *stageBuilder) fakeBuild() error {


What would you think about making a separate fakeBuilder that embeds *stageBuilder but overrides the build() method?

e.g. https://go.dev/play/p/5ZxGr4ozeOV

Are you also thinking we'd merge the logic in Build and FakeBuild into one, just swap out the stageBuilder? (My main reason to keep them separate was to remove anything that could accidentally cause changes to the fs.)

I'll think about it, definitely doable.

non-blocking, just a suggestion

I think what could solve both my concern (about lock-step with build()) and Cian's is an adapter that can be passed in which would effect the changes. For your "fake" case, this adapter would not interact with the FS even though it satisfies the implementation.

I haven't looked too closely at the implementation so maybe this is im{practical,possible}, but thought I'd mention it.

non-blocking, just a suggestion

Ditto 👍

add test for NewReproducibleTar

johnstcn · 2024-06-10T14:11:34Z

I've renamed the DoFakeBuild etc. methods to ProbeCache, and added a minimal integration-style test for same.
CanonicalTar is now ReproducibleTar as that did not appear immediately descriptive to me.

BrunoQuaresma

Code looks good and tested. 👍

dannykopping

Great work, the new names are very clear as well 👍

dannykopping · 2024-06-10T14:23:02Z

pkg/executor/cache_probe_test.go

+	}
+	mFile := filepath.Join(testDir, "proc/mountinfo")
+	mountInfo := fmt.Sprintf(
+		`36 35 98:0 /kaniko %s/kaniko rw,noatime master:1 - ext3 /dev/root rw,errors=continue


That's just someone else's slick code I copy-pasted 🙃

mtojek

post-merge approval 👍

I'm curious if this modification works with multi-stage Dockerfiles.

johnstcn · 2024-06-11T11:34:41Z

post-merge approval 👍

I'm curious if this modification works with multi-stage Dockerfiles.

Good call. I added a separate test and it doesn't appear to work. Will raise a separate PR.

mtojek · 2024-06-11T15:45:17Z

I don't mind adding support for multi-stage Dockerfiles as a separate issue 👍

mafredri force-pushed the mafredri/feat-fake-and-faster-reproducible-builds branch from 549420b to 68a0b67 Compare May 30, 2024 13:55

mafredri mentioned this pull request May 30, 2024

feat: implement reproducible build and get cached image coder/envbuilder#213

Merged

mafredri self-assigned this May 30, 2024

mafredri requested review from dannykopping and johnstcn May 30, 2024 16:24

Add support for fake, and speed up reproducible builds

696ae8d

mafredri force-pushed the mafredri/feat-fake-and-faster-reproducible-builds branch from 68a0b67 to 696ae8d Compare May 30, 2024 17:32

dannykopping reviewed May 31, 2024

View reviewed changes

johnstcn reviewed May 31, 2024

View reviewed changes

mafredri mentioned this pull request Jun 1, 2024

PoC: Build a tool to validate file hashes against layer caches coder/envbuilder#186

Closed

mtojek assigned johnstcn Jun 3, 2024

mtojek mentioned this pull request Jun 3, 2024

Envbuilder v1.0 release coder/envbuilder#132

Closed

36 tasks

johnstcn added 6 commits June 7, 2024 14:30

rename NewCanonicalSnapshotter -> NewReproducibleSnapshotter

0dddc0a

add test for NewReproducibleTar

c03819e

Merge pull request #14 from coder/cj/reproducible-build

2bff75a

add test for NewReproducibleTar

rename to probeCache

3a6d655

add tests for DoCacheProbe

61cdf80

blurb

9d76d2f

johnstcn requested review from dannykopping and mtojek June 10, 2024 14:11

johnstcn requested a review from BrunoQuaresma June 10, 2024 14:13

BrunoQuaresma approved these changes Jun 10, 2024

View reviewed changes

dannykopping approved these changes Jun 10, 2024

View reviewed changes

johnstcn merged commit 0a73fcd into main Jun 10, 2024
9 checks passed

mtojek reviewed Jun 11, 2024

View reviewed changes

johnstcn mentioned this pull request Jun 11, 2024

coder/kaniko: support multi-stage builds with DoCacheProbe coder/envbuilder#230

Closed

mafredri deleted the mafredri/feat-fake-and-faster-reproducible-builds branch September 25, 2024 12:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for fake, and speed up reproducible builds #12

Add support for fake, and speed up reproducible builds #12

mafredri commented May 28, 2024 •

edited

Loading

dannykopping May 31, 2024 •

edited

Loading

mafredri May 31, 2024

johnstcn left a comment

johnstcn May 31, 2024

mafredri May 31, 2024

johnstcn May 31, 2024

johnstcn May 31, 2024

mafredri May 31, 2024

johnstcn May 31, 2024

dannykopping May 31, 2024

dannykopping May 31, 2024

johnstcn commented Jun 10, 2024 •

edited

Loading

BrunoQuaresma left a comment

dannykopping left a comment

dannykopping Jun 10, 2024

johnstcn Jun 10, 2024

mtojek left a comment

johnstcn commented Jun 11, 2024

mtojek commented Jun 11, 2024

	//PAX and GNU Format support additional timestamps in the header
	if header.Format == tar.FormatPAX \|\| header.Format == tar.FormatGNU {
	header.AccessTime = t
	header.ChangeTime = t
	}

Add support for fake, and speed up reproducible builds #12

Add support for fake, and speed up reproducible builds #12

Conversation

mafredri commented May 28, 2024 • edited Loading

dannykopping May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnstcn left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnstcn commented Jun 10, 2024 • edited Loading

BrunoQuaresma left a comment

Choose a reason for hiding this comment

dannykopping left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtojek left a comment

Choose a reason for hiding this comment

johnstcn commented Jun 11, 2024

mtojek commented Jun 11, 2024

mafredri commented May 28, 2024 •

edited

Loading

dannykopping May 31, 2024 •

edited

Loading

johnstcn commented Jun 10, 2024 •

edited

Loading