From b27237b7ff0cae3a1e9bccbf3cf1ecf77c0b00c8 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 22 Apr 2021 22:50:50 +0200 Subject: [PATCH] Allow to pull images from multiple-repo while generating all artifacts before, without --only-target-package, we were forcing to build the images locally. Now we extend it also to that use-case. Also revisit how we pass by the builder image hash, so it's easier to read. This change also re-enables tagging and bulding builder images all the times. Previously, we regressed into not tagging build images coming out-of-the tree, with the unpleasant side-effect of not be able to re-build the same artifacts for those leaf packages. --- pkg/compiler/compiler.go | 106 ++++++++++++------ .../integration/27_buildwithnotree_values.sh | 10 +- tests/integration/28_nobuildtreedocker.sh | 4 +- 3 files changed, 79 insertions(+), 41 deletions(-) diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index 20ad87d8..4561c8e1 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -295,6 +295,17 @@ func (cs *LuetCompiler) unpackDelta(concurrency int, keepPermissions bool, p *co return artifact, nil } +func (cs *LuetCompiler) genBuilderImageTag(p *compilerspec.LuetCompilationSpec, packageImage string) string { + // Use packageImage as salt into the fp being used + // so the hash is unique also in cases where + // some package deps does have completely different + // depgraphs + // TODO: We should use the image tag, or pass by the package assertion hash which is unique + // and identifies the deptree of the package. + return fmt.Sprintf("builder-%s", p.GetPackage().HashFingerprint(helpers.StripRegistryFromImage(packageImage))) + +} + func (cs *LuetCompiler) buildPackageImage(image, buildertaggedImage, packageImage string, concurrency int, keepPermissions bool, p *compilerspec.LuetCompilationSpec) (backend.Options, backend.Options, error) { @@ -303,20 +314,6 @@ func (cs *LuetCompiler) buildPackageImage(image, buildertaggedImage, packageImag pkgTag := ":package: " + p.GetPackage().HumanReadableString() - // Use packageImage as salt into the fp being used - // so the hash is unique also in cases where - // some package deps does have completely different - // depgraphs - // TODO: We should use the image tag, or pass by the package assertion hash which is unique - // and identifies the deptree of the package. - - fp := p.GetPackage().HashFingerprint(helpers.StripRegistryFromImage(packageImage)) - - if buildertaggedImage == "" { - buildertaggedImage = cs.Options.PushImageRepository + ":builder-" + fp - Debug(pkgTag, "Creating intermediary image", buildertaggedImage, "from", image) - } - // TODO: Cleanup, not actually hit if packageImage == "" { return runnerOpts, builderOpts, errors.New("no package image given") @@ -354,9 +351,15 @@ func (cs *LuetCompiler) buildPackageImage(image, buildertaggedImage, packageImag return builderOpts, runnerOpts, errors.Wrap(err, "Could not generate image definition") } - if len(p.GetPreBuildSteps()) == 0 { - buildertaggedImage = image - } + // Even if we don't have prelude steps, we want to push + // An intermediate image to tag images which are outside of the tree. + // Those don't have an hash otherwise, and thus makes build unreproducible + // see SKIPBUILD for the other logic + // if len(p.GetPreBuildSteps()) == 0 { + // buildertaggedImage = image + // } + // We might want to skip this phase but replacing with a tag that we push. But in case + // steps in prelude are == 0 those are equivalent. // Then we write the step image, which uses the builder one if err := p.WriteStepImageDefinition(buildertaggedImage, filepath.Join(buildDir, p.GetPackage().GetFingerPrint()+".dockerfile")); err != nil { @@ -401,12 +404,13 @@ func (cs *LuetCompiler) buildPackageImage(image, buildertaggedImage, packageImag } return nil } - if len(p.GetPreBuildSteps()) != 0 { - Info(pkgTag, ":whale: Generating 'builder' image from", image, "as", buildertaggedImage, "with prelude steps") - if err := buildAndPush(builderOpts); err != nil { - return builderOpts, runnerOpts, errors.Wrapf(err, "Could not push image: %s %s", image, builderOpts.DockerFileName) - } + // SKIPBUILD + // if len(p.GetPreBuildSteps()) != 0 { + Info(pkgTag, ":whale: Generating 'builder' image from", image, "as", buildertaggedImage, "with prelude steps") + if err := buildAndPush(builderOpts); err != nil { + return builderOpts, runnerOpts, errors.Wrapf(err, "Could not push image: %s %s", image, builderOpts.DockerFileName) } + //} // Even if we might not have any steps to build, we do that so we can tag the image used in this moment and use that to cache it in a registry, or in the system. // acting as a docker tag. @@ -425,7 +429,7 @@ func (cs *LuetCompiler) genArtifact(p *compilerspec.LuetCompilationSpec, builder var rootfs string var err error pkgTag := ":package: " + p.GetPackage().HumanReadableString() - + Debug(pkgTag, "Generating artifact") // We can't generate delta in this case. It implies the package is a virtual, and nothing has to be done really if p.EmptyPackage() { fakePackage := p.Rel(p.GetPackage().GetFingerPrint() + ".package.tar") @@ -520,6 +524,7 @@ func oneOfImagesAvailable(images []string, b CompilerBackend) (bool, string) { func (cs *LuetCompiler) resolveExistingImageHash(imageHash string, p *compilerspec.LuetCompilationSpec) string { var resolvedImage string + Debug("Resolving image hash for", p.Package.HumanReadableString(), "hash", imageHash, "Pull repositories", p.BuildOptions.PullImageRepository) toChecklist := append([]string{fmt.Sprintf("%s:%s", cs.Options.PushImageRepository, imageHash)}, genImageList(p.BuildOptions.PullImageRepository, imageHash)...) if exists, which := oneOfImagesExists(toChecklist, cs.Backend); exists { @@ -555,6 +560,7 @@ func LoadArtifactFromYaml(spec *compilerspec.LuetCompilationSpec) (*artifact.Pac func (cs *LuetCompiler) getImageArtifact(hash string, p *compilerspec.LuetCompilationSpec) (*artifact.PackageArtifact, error) { // we check if there is an available image with the given hash and // we return a full artifact if can be loaded locally. + Debug("Get image artifact for", p.Package.HumanReadableString(), "hash", hash, "Pull repositories", p.BuildOptions.PullImageRepository) toChecklist := append([]string{fmt.Sprintf("%s:%s", cs.Options.PushImageRepository, hash)}, genImageList(p.BuildOptions.PullImageRepository, hash)...) @@ -578,7 +584,7 @@ func (cs *LuetCompiler) getImageArtifact(hash string, p *compilerspec.LuetCompil // image buildertaggedImage. // Images that can be resolved from repositories are prefered over the local ones if PullFirst is set to true // avoiding to rebuild images as much as possible -func (cs *LuetCompiler) compileWithImage(image, buildertaggedImage string, packageTagHash string, +func (cs *LuetCompiler) compileWithImage(image, builderHash string, packageTagHash string, concurrency int, keepPermissions, keepImg bool, p *compilerspec.LuetCompilationSpec, generateArtifact bool) (*artifact.PackageArtifact, error) { @@ -591,16 +597,40 @@ func (cs *LuetCompiler) compileWithImage(image, buildertaggedImage string, packa } if !generateArtifact { - // try to avoid regenerating the image if possible by checking the hash in the - // given repositories - // It is best effort. If we fail resolving, we will generate the images and keep going if art, err := cs.getImageArtifact(packageTagHash, p); err == nil { + // try to avoid regenerating the image if possible by checking the hash in the + // given repositories + // It is best effort. If we fail resolving, we will generate the images and keep going return art, nil } } - // always going to point at the destination from the repo defined packageImage := fmt.Sprintf("%s:%s", cs.Options.PushImageRepository, packageTagHash) + buildertaggedImage := fmt.Sprintf("%s:%s", cs.Options.PushImageRepository, builderHash) + //generated := false + // if buildertaggedImage == "" { + // buildertaggedImage = fmt.Sprintf("%s:%s", cs.Options.PushImageRepository, buildertaggedImage) + // generated = true + // // Debug(pkgTag, "Creating intermediary image", buildertaggedImage, "from", image) + // } + + if cs.Options.PullFirst { + Debug("Checking if an image is already available") + // FIXUP here. If packageimage hash exists and pull is true, generate package + resolved := cs.resolveExistingImageHash(packageTagHash, p) + + builderResolved := cs.resolveExistingImageHash(builderHash, p) + + // + if resolved != packageImage && buildertaggedImage != builderResolved { // an image is there already + Debug("Images available for", p.Package.HumanReadableString(), "generating artifact from remote images:", resolved) + return cs.genArtifact(p, backend.Options{ImageName: builderResolved}, backend.Options{ImageName: resolved}, concurrency, keepPermissions) + } else { + Debug("Images not available for", p.Package.HumanReadableString()) + } + } + + // always going to point at the destination from the repo defined builderOpts, runnerOpts, err := cs.buildPackageImage(image, buildertaggedImage, packageImage, concurrency, keepPermissions, p) if err != nil { return nil, errors.Wrap(err, "failed building package image") @@ -726,7 +756,7 @@ func genImageList(refs []string, hash string) []string { } func (cs *LuetCompiler) inheritSpecBuildOptions(p *compilerspec.LuetCompilationSpec) { - Debug("Build options before inherit", p.BuildOptions) + Debug(p.GetPackage().HumanReadableString(), "Build options before inherit", p.BuildOptions) // Append push repositories from buildpsec buildoptions as pull if found. // This allows to resolve the hash automatically if we pulled the metadata from @@ -740,7 +770,7 @@ func (cs *LuetCompiler) inheritSpecBuildOptions(p *compilerspec.LuetCompilationS p.BuildOptions.PullImageRepository = append(p.BuildOptions.PullImageRepository, cs.Options.PullImageRepository...) Debug("Inheriting pull repository from PullImageRepository buildoptions", p.BuildOptions.PullImageRepository) } - Debug("Build options after inherit", p.BuildOptions) + Debug(p.GetPackage().HumanReadableString(), "Build options after inherit", p.BuildOptions) } func (cs *LuetCompiler) compile(concurrency int, keepPermissions bool, p *compilerspec.LuetCompilationSpec) (*artifact.PackageArtifact, error) { @@ -772,12 +802,14 @@ func (cs *LuetCompiler) compile(concurrency int, keepPermissions bool, p *compil // Update compilespec build options - it will be then serialized into the compilation metadata file //p.SetBuildOptions(cs.Options) - cs.inheritSpecBuildOptions(p) + p.BuildOptions.PushImageRepository = cs.Options.PushImageRepository + p.BuildOptions.BuildValues = cs.Options.BuildValues + //p.BuildOptions.BuildValuesFile = cs.Options.BuildValuesFile // - If image is set we just generate a plain dockerfile // Treat last case (easier) first. The image is provided and we just compute a plain dockerfile with the images listed as above if p.GetImage() != "" { - return cs.compileWithImage(p.GetImage(), "", targetAssertion.Hash.PackageHash, concurrency, keepPermissions, cs.Options.KeepImg, p, true) + return cs.compileWithImage(p.GetImage(), cs.genBuilderImageTag(p, targetAssertion.Hash.PackageHash), targetAssertion.Hash.PackageHash, concurrency, keepPermissions, cs.Options.KeepImg, p, true) } // - If image is not set, we read a base_image. Then we will build one image from it to kick-off our build based @@ -833,7 +865,7 @@ func (cs *LuetCompiler) compile(concurrency int, keepPermissions bool, p *compil if compileSpec.GetImage() != "" { Debug(pkgTag, " :wrench: Compiling "+compileSpec.GetPackage().HumanReadableString()+" from image") - a, err := cs.compileWithImage(compileSpec.GetImage(), resolvedBuildImage, assertion.Hash.PackageHash, concurrency, keepPermissions, cs.Options.KeepImg, compileSpec, packageDeps) + a, err := cs.compileWithImage(compileSpec.GetImage(), assertion.Hash.BuildHash, assertion.Hash.PackageHash, concurrency, keepPermissions, cs.Options.KeepImg, compileSpec, packageDeps) if err != nil { return nil, errors.Wrap(err, "Failed compiling "+compileSpec.GetPackage().HumanReadableString()) } @@ -843,7 +875,7 @@ func (cs *LuetCompiler) compile(concurrency int, keepPermissions bool, p *compil } Debug(pkgTag, " :wrench: Compiling "+compileSpec.GetPackage().HumanReadableString()+" from tree") - a, err := cs.compileWithImage(resolvedBuildImage, "", assertion.Hash.PackageHash, concurrency, keepPermissions, cs.Options.KeepImg, compileSpec, packageDeps) + a, err := cs.compileWithImage(resolvedBuildImage, cs.genBuilderImageTag(compileSpec, targetAssertion.Hash.PackageHash), assertion.Hash.PackageHash, concurrency, keepPermissions, cs.Options.KeepImg, compileSpec, packageDeps) if err != nil { return nil, errors.Wrap(err, "Failed compiling "+compileSpec.GetPackage().HumanReadableString()) } @@ -868,7 +900,7 @@ func (cs *LuetCompiler) compile(concurrency int, keepPermissions bool, p *compil resolvedBuildImage := cs.resolveExistingImageHash(lastHash, p) Info(":rocket: All dependencies are satisfied, building package requested by the user", p.GetPackage().HumanReadableString()) Info(":package:", p.GetPackage().HumanReadableString(), " Using image: ", resolvedBuildImage) - a, err := cs.compileWithImage(resolvedBuildImage, "", targetAssertion.Hash.PackageHash, concurrency, keepPermissions, cs.Options.KeepImg, p, true) + a, err := cs.compileWithImage(resolvedBuildImage, cs.genBuilderImageTag(p, targetAssertion.Hash.PackageHash), targetAssertion.Hash.PackageHash, concurrency, keepPermissions, cs.Options.KeepImg, p, true) if err != nil { return a, err } @@ -998,7 +1030,9 @@ func (cs *LuetCompiler) FromPackage(p pkg.Package) (*compilerspec.LuetCompilatio } Debug("Read build options:", art.CompileSpec.BuildOptions, "from", artifactMetadataFile) - opts = *art.CompileSpec.BuildOptions + if art.CompileSpec.BuildOptions != nil { + opts = *art.CompileSpec.BuildOptions + } } else if !os.IsNotExist(err) { Debug("error reading artifact metadata file: ", err.Error()) } else if os.IsNotExist(err) { diff --git a/tests/integration/27_buildwithnotree_values.sh b/tests/integration/27_buildwithnotree_values.sh index 91f95c2e..861d4d08 100755 --- a/tests/integration/27_buildwithnotree_values.sh +++ b/tests/integration/27_buildwithnotree_values.sh @@ -3,11 +3,13 @@ export LUET_NOLOCK=true oneTimeSetUp() { -export tmpdir="$(mktemp -d)" + export tmpdir="$(mktemp -d)" + docker images --filter='reference=luet/cache' --format='{{.Repository}}:{{.Tag}}' | xargs -r docker rmi } oneTimeTearDown() { rm -rf "$tmpdir" + docker images --filter='reference=luet/cache' --format='{{.Repository}}:{{.Tag}}' | xargs -r docker rmi } testBuild() { @@ -15,7 +17,7 @@ testBuild() { bb: "ttt" EOF mkdir $tmpdir/testbuild - luet build --tree "$ROOT_DIR/tests/fixtures/build_values" --values $tmpdir/default.yaml --destination $tmpdir/testbuild --compression gzip --all + luet build --tree "$ROOT_DIR/tests/fixtures/build_values" --values $tmpdir/default.yaml --destination $tmpdir/testbuild --compression gzip distro/a distro/b test/foo distro/c buildst=$? assertEquals 'builds successfully' "$buildst" "0" assertTrue 'create package B' "[ -e '$tmpdir/testbuild/b-distro-0.3.package.tar.gz' ]" @@ -63,7 +65,7 @@ EOF testBuildWithNoTree() { mkdir $tmpdir/testbuild2 mkdir $tmpdir/emptytree - luet build --from-repositories --tree $tmpdir/emptytree --config $tmpdir/luet.yaml test/c --destination $tmpdir/testbuild2 --compression gzip --all + luet build --from-repositories --tree $tmpdir/emptytree --config $tmpdir/luet.yaml distro/c --destination $tmpdir/testbuild2 --compression gzip distro/a distro/b test/foo distro/c buildst=$? assertEquals 'builds successfully' "$buildst" "0" assertTrue 'create package B' "[ -e '$tmpdir/testbuild2/b-distro-0.3.package.tar.gz' ]" @@ -187,7 +189,7 @@ foo: "sq" EOF mkdir $tmpdir/testbuild3 mkdir $tmpdir/emptytree - luet build --from-repositories --values $tmpdir/default.yaml --tree $tmpdir/emptytree --config $tmpdir/luet.yaml test/c --destination $tmpdir/testbuild3 --compression gzip --all + luet build --from-repositories --values $tmpdir/default.yaml --tree $tmpdir/emptytree --config $tmpdir/luet.yaml distro/c --destination $tmpdir/testbuild3 --compression gzip distro/a distro/b test/foo buildst=$? assertEquals 'builds successfully' "$buildst" "0" assertTrue 'create package B' "[ -e '$tmpdir/testbuild3/b-distro-0.3.package.tar.gz' ]" diff --git a/tests/integration/28_nobuildtreedocker.sh b/tests/integration/28_nobuildtreedocker.sh index 21aba49b..136a993b 100755 --- a/tests/integration/28_nobuildtreedocker.sh +++ b/tests/integration/28_nobuildtreedocker.sh @@ -51,7 +51,9 @@ testBuild() { assertTrue 'create package' "[ -e '$tmpdir/testbuild/c-test-1.0.package.tar.zst' ]" assertTrue 'create package Z' "[ -e '$tmpdir/testbuild/z-test-1.0+2.package.tar.zst' ]" assertTrue 'create package interpolated' "[ -e '$tmpdir/testbuild/interpolated-test-1.0+2.package.tar.zst' ]" - assertContains 'Does use the upstream cache without specifying it' "$build_output" "Downloaded image: quay.io/mocaccinoos/integration-test-cache:6490e800fe443b99328fc363529aee74bda513930fb27ce6ab814d692bba068e" + assertContains 'Does use the upstream cache without specifying it test/c' "$build_output" "Images available for test/c-1.0 generating artifact from remote images: quay.io/mocaccinoos/integration-test-cache:d620e573c81eab36a9dc5cc314e80fd7b6e04aeff26127de4225bf24fe1f8e71" + assertContains 'Does use the upstream cache without specifying it test/z' "$build_output" "Images available for test/z-1.0+2 generating artifact from remote images: quay.io/mocaccinoos/integration-test-cache:b0f34b0d2d271f0f2619324476b2857b3b39ca895bddc2474a741f3c8c1acbbc" + assertContains 'Does use the upstream cache without specifying it test/interpolated' "$build_output" "Images available for test/interpolated-1.0+2 generating artifact from remote images: quay.io/mocaccinoos/integration-test-cache:c1f11f48113cd71d8795a06c7b49e1558bd7211d2aa88f5d79a3334f0393c64d" } testRepo() {