Skip to content

Commit

Permalink
Merge pull request #5335 from nalind/only-most-platforms
Browse files Browse the repository at this point in the history
build --all-platforms: skip some base "image" platforms
  • Loading branch information
openshift-merge-bot[bot] authored Feb 12, 2024
2 parents d62beb9 + 98f8707 commit a626bd1
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
32 changes: 30 additions & 2 deletions imagebuildah/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"errors"
"fmt"
gotypes "go/types"
"io"
"net/http"
"os"
Expand Down Expand Up @@ -36,6 +37,7 @@ import (
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/openshift/imagebuilder"
"github.com/sirupsen/logrus"
"golang.org/x/exp/slices"
"golang.org/x/sync/semaphore"
)

Expand Down Expand Up @@ -493,6 +495,26 @@ func preprocessContainerfileContents(logger *logrus.Logger, containerfile string
return &stdoutBuffer, nil
}

// platformIsUnknown checks if the platform value indicates that the
// corresponding index entry is suitable for use as a base image
func platformIsAcceptable(platform *v1.Platform, logger *logrus.Logger) bool {
if platform == nil {
logger.Trace("rejecting potential base image with no platform information")
return false
}
if gotypes.SizesFor("gc", platform.Architecture) == nil {
// the compiler's never heard of this
logger.Tracef("rejecting potential base image architecture %q for which Go has no knowledge of how to do unsafe code", platform.Architecture)
return false
}
if slices.Contains([]string{"", "unknown"}, platform.OS) {
// we're hard-wired to reject images with these values
logger.Tracef("rejecting potential base image for which the OS value is always-rejected value %q", platform.OS)
return false
}
return true
}

// platformsForBaseImages resolves the names of base images from the
// dockerfiles, and if they are all valid references to manifest lists, returns
// the list of platforms that are supported by all of the base images.
Expand Down Expand Up @@ -570,7 +592,10 @@ func platformsForBaseImages(ctx context.Context, logger *logrus.Logger, dockerfi
if baseImageIndex == 0 {
// populate the list with the first image's normalized platforms
for _, instance := range index.Manifests {
if instance.Platform == nil {
if !platformIsAcceptable(instance.Platform, logger) {
continue
}
if instance.ArtifactType != "" {
continue
}
platform := internalUtil.NormalizePlatform(*instance.Platform)
Expand All @@ -581,7 +606,10 @@ func platformsForBaseImages(ctx context.Context, logger *logrus.Logger, dockerfi
// prune the list of any normalized platforms this base image doesn't support
imagePlatforms := make(map[string]struct{})
for _, instance := range index.Manifests {
if instance.Platform == nil {
if !platformIsAcceptable(instance.Platform, logger) {
continue
}
if instance.ArtifactType != "" {
continue
}
platform := internalUtil.NormalizePlatform(*instance.Platform)
Expand Down
2 changes: 2 additions & 0 deletions tests/bud/from-base/Containerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
FROM base
ADD . .
27 changes: 27 additions & 0 deletions tests/lists.bats
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,30 @@ IMAGE_LIST_S390X_INSTANCE_DIGEST=sha256:882a20ee0df7399a445285361d38b711c299ca09
# since manifest does not exist in local storage this should exit with `1`
run_buildah 1 manifest exists test2
}

@test "manifest-skip-some-base-images-with-all-platforms" {
start_registry
run_buildah manifest create localhost:"${REGISTRY_PORT}"/base
run_buildah manifest add --all localhost:"${REGISTRY_PORT}"/base ${IMAGE_LIST}
# get a count of how many "real" base images there are
run_buildah manifest inspect localhost:"${REGISTRY_PORT}"/base
nbaseplatforms=$(grep '"platform"' <<< "$output" | wc -l)
echo $nbaseplatforms base platforms
# add some trash that we expect to skip in a --all-platforms build
run_buildah build --manifest localhost:"${REGISTRY_PORT}"/base --platform unknown/unknown --no-cache -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch
run_buildah build --manifest localhost:"${REGISTRY_PORT}"/base --platform linux/unknown --no-cache -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch
run_buildah build --manifest localhost:"${REGISTRY_PORT}"/base --platform unknown/amd64p32 --no-cache -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch
# add a known combination of OS/arch that we can be pretty sure wasn't already there
run_buildah build --manifest localhost:"${REGISTRY_PORT}"/base --platform linux/amd64p32 --no-cache -f $BUDFILES/from-scratch/Containerfile2 $BUDFILES/from-scratch
# push the list to the local registry and clean up our local copy
run_buildah manifest push --tls-verify=false --creds testuser:testpassword --all localhost:"${REGISTRY_PORT}"/base
run_buildah rmi localhost:"${REGISTRY_PORT}"/base
# build a new list based on the valid base images in the list we just pushed
run_buildah build --tls-verify=false --creds testuser:testpassword --manifest derived --all-platforms --from localhost:"${REGISTRY_PORT}"/base $BUDFILES/from-base
run_buildah manifest inspect derived
nderivedplatforms=$(grep '"platform"' <<< "$output" | wc -l)
echo $nderivedplatforms derived platforms
nexpectedderivedplatforms=$((nbaseplatforms+1))
echo expected $nexpectedderivedplatforms derived platforms
[[ $nderivedplatforms -eq $nexpectedderivedplatforms ]]
}

0 comments on commit a626bd1

Please sign in to comment.