Merge pull request #29450 from bertinatto/list-images-external-test-b…

…inary

TRT-1286: List images from external binaries

go.mod

@@ -50,6 +50,7 @@ require (
 	go.etcd.io/etcd/client/pkg/v3 v3.5.16
 	go.etcd.io/etcd/client/v3 v3.5.16
 	golang.org/x/crypto v0.31.0
+	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
 	golang.org/x/mod v0.21.0
 	golang.org/x/net v0.30.0
 	golang.org/x/oauth2 v0.23.0
@@ -268,7 +269,6 @@ require (
 	go.uber.org/mock v0.4.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.27.0 // indirect
-	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
 	golang.org/x/image v0.11.0 // indirect
 	golang.org/x/sys v0.28.0 // indirect
 	golang.org/x/term v0.27.0 // indirect

hack/update-generated.sh

@@ -13,8 +13,10 @@ ENABLE_STORAGE_GCE_PD_DRIVER=yes go generate -mod vendor ./test/extended
 go run -mod vendor ./cmd/openshift-tests render test-report --output-dir=./zz_generated.manifests
 
 # Update mirror mapping from upstream to quay
+# By default, "openshift-tests images" lists images from external binaries. However, we force
+# this script to list images from built-in tests in order to avoid requiring an OCP release image.
 echo "# This file is generated by hack/update-generated.sh" > test/extended/util/image/zz_generated.txt
-go run -mod vendor ./cmd/openshift-tests images --upstream --to-repository quay.io/openshift/community-e2e-images >> test/extended/util/image/zz_generated.txt
+OPENSHIFT_SKIP_EXTERNAL_TESTS=1 go run -mod vendor ./cmd/openshift-tests images --upstream --to-repository quay.io/openshift/community-e2e-images >> test/extended/util/image/zz_generated.txt
 
 os::build::setup_env
 

pkg/cmd/openshift-tests/images/images_command.go

@@ -1,16 +1,20 @@
 package images
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"sort"
 	"strings"
+	"time"
 
+	"golang.org/x/exp/slices"
 	k8simage "k8s.io/kubernetes/test/utils/image"
 
 	"github.com/openshift/library-go/pkg/image/reference"
 	"github.com/openshift/origin/pkg/clioptions/imagesetup"
 	"github.com/openshift/origin/pkg/cmd"
+	"github.com/openshift/origin/pkg/test/extensions"
 	"github.com/openshift/origin/test/extended/util/image"
 	"github.com/spf13/cobra"
 	"k8s.io/kube-openapi/pkg/util/sets"
@@ -109,21 +113,57 @@ type imagesOptions struct {
 func createImageMirrorForInternalImages(prefix string, ref reference.DockerImageReference, mirrored bool) ([]string, error) {
 	source := ref.Exact()
 
-	initialDefaults := k8simage.GetOriginalImageConfigs()
-	exceptions := image.Exceptions.List()
-	defaults := map[k8simage.ImageID]k8simage.Config{}
+	initialImageSets := []extensions.ImageSet{
+		k8simage.GetOriginalImageConfigs(),
+	}
+
+	// If ENV is not set, the list of images should come from external binaries
+	if len(os.Getenv("OPENSHIFT_SKIP_EXTERNAL_TESTS")) == 0 {
+		// Extract all test binaries
+		extractionContext, extractionContextCancel := context.WithTimeout(context.Background(), 30*time.Minute)
+		defer extractionContextCancel()
+		cleanUpFn, externalBinaries, err := extensions.ExtractAllTestBinaries(extractionContext, 10)
+		if err != nil {
+			return nil, err
+		}
+		defer cleanUpFn()
+
+		// List test images from all available binaries
+		listContext, listContextCancel := context.WithTimeout(context.Background(), time.Minute)
+		defer listContextCancel()
+		imageSetsFromBinaries, err := externalBinaries.ListImages(listContext, 10)
+		if err != nil {
+			return nil, err
+		}
+		if len(imageSetsFromBinaries) == 0 {
+			return nil, fmt.Errorf("no test images were reported by external binaries")
+		}
+		initialImageSets = imageSetsFromBinaries
+	}
 
-imageLoop:
-	for i, config := range initialDefaults {
-		for _, exception := range exceptions {
-			if strings.Contains(config.GetE2EImage(), exception) {
-				continue imageLoop
+	// Take the initial images coming from external binaries and remove any exceptions that might exist.
+	exceptions := image.Exceptions.List()
+	defaultImageSets := []extensions.ImageSet{}
+	for i := range initialImageSets {
+		filtered := extensions.ImageSet{}
+		for imageID, imageConfig := range initialImageSets[i] {
+			if !slices.ContainsFunc(exceptions, func(e string) bool {
+				return strings.Contains(imageConfig.GetE2EImage(), e)
+			}) {
+				filtered[imageID] = imageConfig
 			}
 		}
-		defaults[i] = config
+		if len(filtered) > 0 {
+			defaultImageSets = append(defaultImageSets, filtered)
+		}
+	}
+
+	// Created a new slice with the updatedImageSets addresses for the images
+	updatedImageSets := []extensions.ImageSet{}
+	for i := range defaultImageSets {
+		updatedImageSets = append(updatedImageSets, k8simage.GetMappedImageConfigs(defaultImageSets[i], ref.Exact()))
 	}
 
-	updated := k8simage.GetMappedImageConfigs(defaults, ref.Exact())
 	openshiftDefaults := image.OriginalImages()
 	openshiftUpdated := image.GetMappedImages(openshiftDefaults, imagesetup.DefaultTestImageMirrorLocation)
 
@@ -136,27 +176,29 @@ imageLoop:
 
 		// calculate the mapping of upstream images by setting defaults to baseRef
 		covered := sets.NewString()
-		for i, config := range updated {
-			defaultConfig := defaults[i]
-			pullSpec := config.GetE2EImage()
-			if pullSpec == defaultConfig.GetE2EImage() {
-				continue
-			}
-			if covered.Has(pullSpec) {
-				continue
-			}
-			covered.Insert(pullSpec)
-			e2eRef, err := reference.Parse(pullSpec)
-			if err != nil {
-				return nil, fmt.Errorf("invalid test image: %s: %v", pullSpec, err)
-			}
-			if len(e2eRef.Tag) == 0 {
-				return nil, fmt.Errorf("invalid test image: %s: no tag", pullSpec)
+		for i := range updatedImageSets {
+			for imageID, imageConfig := range updatedImageSets[i] {
+				defaultConfig := defaultImageSets[i][imageID]
+				pullSpec := imageConfig.GetE2EImage()
+				if pullSpec == defaultConfig.GetE2EImage() {
+					continue
+				}
+				if covered.Has(pullSpec) {
+					continue
+				}
+				covered.Insert(pullSpec)
+				e2eRef, err := reference.Parse(pullSpec)
+				if err != nil {
+					return nil, fmt.Errorf("invalid test image: %s: %v", pullSpec, err)
+				}
+				if len(e2eRef.Tag) == 0 {
+					return nil, fmt.Errorf("invalid test image: %s: no tag", pullSpec)
+				}
+				imageConfig.SetRegistry(baseRef.Registry)
+				imageConfig.SetName(baseRef.RepositoryName())
+				imageConfig.SetVersion(e2eRef.Tag)
+				defaultImageSets[i][imageID] = imageConfig
 			}
-			config.SetRegistry(baseRef.Registry)
-			config.SetName(baseRef.RepositoryName())
-			config.SetVersion(e2eRef.Tag)
-			defaults[i] = config
 		}
 
 		// calculate the mapping for openshift images by populating openshiftUpdated
@@ -179,17 +221,19 @@ imageLoop:
 
 	covered := sets.NewString()
 	var lines []string
-	for i := range updated {
-		a, b := defaults[i], updated[i]
-		from, to := a.GetE2EImage(), b.GetE2EImage()
-		if from == to {
-			continue
-		}
-		if covered.Has(from) {
-			continue
+	for i := range updatedImageSets {
+		for imageID := range updatedImageSets[i] {
+			a, b := defaultImageSets[i][imageID], updatedImageSets[i][imageID]
+			from, to := a.GetE2EImage(), b.GetE2EImage()
+			if from == to {
+				continue
+			}
+			if covered.Has(from) {
+				continue
+			}
+			covered.Insert(from)
+			lines = append(lines, fmt.Sprintf("%s %s%s", from, prefix, to))
 		}
-		covered.Insert(from)
-		lines = append(lines, fmt.Sprintf("%s %s%s", from, prefix, to))
 	}
 
 	for from, to := range openshiftUpdated {

pkg/test/extensions/binary.go

@@ -22,6 +22,7 @@ import (
 	kapierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
+	k8simage "k8s.io/kubernetes/test/utils/image"
 
 	"github.com/openshift/origin/test/extended/util"
 )
@@ -37,6 +38,18 @@ type TestBinary struct {
 	info *ExtensionInfo
 }
 
+// ImageSet maps a Kubernetes image ID to its corresponding configuration.
+// It represents a collection of container images with their registry, name, and version.
+type ImageSet map[k8simage.ImageID]k8simage.Config
+
+// Image represents a single container image generated by the "images" command.
+type Image struct {
+	Index    int    `json:"index"`
+	Registry string `json:"registry"`
+	Name     string `json:"name"`
+	Version  string `json:"version"`
+}
+
 var extensionBinaries = []TestBinary{
 	{
 		imageTag:   "hyperkube",
@@ -191,6 +204,36 @@ func (b *TestBinary) RunTests(ctx context.Context, timeout time.Duration, env []
 	return results
 }
 
+func (b *TestBinary) ListImages(ctx context.Context) (ImageSet, error) {
+	start := time.Now()
+	binName := filepath.Base(b.binaryPath)
+
+	logrus.Infof("Listing images for %q", binName)
+	command := exec.Command(b.binaryPath, "images")
+	output, err := runWithTimeout(ctx, command, 10*time.Minute)
+	if err != nil {
+		return nil, fmt.Errorf("failed running '%s list': %w", b.binaryPath, err)
+	}
+
+	var images []Image
+	err = json.Unmarshal(output, &images)
+	if err != nil {
+		return nil, err
+	}
+
+	result := make(ImageSet, len(images))
+	for _, image := range images {
+		imageConfig := k8simage.Config{}
+		imageConfig.SetName(image.Name)
+		imageConfig.SetVersion(image.Version)
+		imageConfig.SetRegistry(image.Registry)
+		result[k8simage.ImageID(image.Index)] = imageConfig
+	}
+
+	logrus.Infof("Listed %d test images for %q in %v", len(images), binName, time.Since(start))
+	return result, nil
+}
+
 // ExtractAllTestBinaries determines the optimal release payload to use, and extracts all the external
 // test binaries from it, and returns a slice of them.
 func ExtractAllTestBinaries(ctx context.Context, parallelism int) (func(), TestBinaries, error) {
@@ -397,6 +440,68 @@ func (binaries TestBinaries) Info(ctx context.Context, parallelism int) ([]*Exte
 	return infos, nil
 }
 
+func (binaries TestBinaries) ListImages(ctx context.Context, parallelism int) ([]ImageSet, error) {
+	var (
+		allImages []ImageSet
+		mu        sync.Mutex
+		wg        sync.WaitGroup
+		errCh     = make(chan error, len(binaries))
+		jobCh     = make(chan *TestBinary)
+	)
+
+	// Producer: sends jobs to the jobCh channel
+	go func() {
+		defer close(jobCh)
+		for _, binary := range binaries {
+			select {
+			case <-ctx.Done():
+				return // Exit when context is cancelled
+			case jobCh <- binary:
+			}
+		}
+	}()
+
+	// Consumer workers: extract tests concurrently
+	for i := 0; i < parallelism; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for {
+				select {
+				case <-ctx.Done():
+					return // Exit when context is cancelled
+				case binary, ok := <-jobCh:
+					if !ok {
+						return // Channel was closed
+					}
+					imageConfig, err := binary.ListImages(ctx)
+					if err != nil {
+						errCh <- err
+					}
+					mu.Lock()
+					allImages = append(allImages, imageConfig)
+					mu.Unlock()
+				}
+			}
+		}()
+	}
+
+	// Wait for all workers to finish
+	wg.Wait()
+	close(errCh)
+
+	// Check if any errors were reported
+	var errs []string
+	for err := range errCh {
+		errs = append(errs, err.Error())
+	}
+	if len(errs) > 0 {
+		return nil, fmt.Errorf("encountered errors while listing tests: %s", strings.Join(errs, ";"))
+	}
+
+	return allImages, nil
+}
+
 // ListTests extracts the tests from all TestBinaries using the specified parallelism,
 // and passes the provided EnvironmentFlags for proper filtering of results.
 func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int, envFlags EnvironmentFlags) (ExtensionTestSpecs, error) {