Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ require (
github.com/onsi/ginkgo/v2 v2.23.3
github.com/onsi/gomega v1.37.0
github.com/opencontainers/go-digest v1.0.0
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250916161632-d81c09058835
github.com/openshift-eng/openshift-tests-extension v0.0.0-20251104171704-4de6984a8e50
github.com/openshift-kni/commatrix v0.0.4-0.20250917111054-6e3ad6c3a0e4
github.com/openshift/api v0.0.0-20251015095338-264e80a2b6e7
github.com/openshift/apiserver-library-go v0.0.0-20251015164739-79d04067059d
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -820,8 +820,8 @@ github.com/opencontainers/runtime-spec v1.2.0 h1:z97+pHb3uELt/yiAWD691HNHQIF07bE
github.com/opencontainers/runtime-spec v1.2.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
github.com/opencontainers/selinux v1.11.1 h1:nHFvthhM0qY8/m+vfhJylliSshm8G1jJ2jDMcgULaH8=
github.com/opencontainers/selinux v1.11.1/go.mod h1:E5dMC3VPuVvVHDYmi78qvhJp8+M586T4DlDRYpFkyec=
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250916161632-d81c09058835 h1:rkqIIfdYYkasXbF2XKVgh/3f1mhjSQK9By8WtVMgYo8=
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250916161632-d81c09058835/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M=
github.com/openshift-eng/openshift-tests-extension v0.0.0-20251104171704-4de6984a8e50 h1:IDbKc3X/GwWtwDraBK+l0H0nUNcE3vU8BCba7HTo7/0=
github.com/openshift-eng/openshift-tests-extension v0.0.0-20251104171704-4de6984a8e50/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M=
github.com/openshift-kni/commatrix v0.0.4-0.20250917111054-6e3ad6c3a0e4 h1:jTEZQxMok1So4jAISd40MdN9sCMutzKsV8fQqgH1u6E=
github.com/openshift-kni/commatrix v0.0.4-0.20250917111054-6e3ad6c3a0e4/go.mod h1:cDVdp0eda7EHE6tLuSeo4IqPWdAX/KJK+ogBirIGtsI=
github.com/openshift/api v0.0.0-20251015095338-264e80a2b6e7 h1:Ot2fbEEPmF3WlPQkyEW/bUCV38GMugH/UmZvxpWceNc=
Expand Down
79 changes: 68 additions & 11 deletions pkg/cmd/openshift-tests/images/images_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"time"

"golang.org/x/exp/slices"
k8simage "k8s.io/kubernetes/test/utils/image"

"github.com/openshift-eng/openshift-tests-extension/pkg/extension"
"github.com/openshift/library-go/pkg/image/reference"
"github.com/openshift/origin/pkg/clioptions/imagesetup"
"github.com/openshift/origin/pkg/cmd"
Expand All @@ -19,6 +19,7 @@ import (
"github.com/spf13/cobra"
"k8s.io/kube-openapi/pkg/util/sets"
"k8s.io/kubectl/pkg/util/templates"
k8simage "k8s.io/kubernetes/test/utils/image"
)

func NewImagesCommand() *cobra.Command {
Expand Down Expand Up @@ -112,14 +113,13 @@ type imagesOptions struct {
// TAG is the hash described above.
func createImageMirrorForInternalImages(prefix string, ref reference.DockerImageReference, mirrored bool) ([]string, error) {
source := ref.Exact()

externalImageSets := []extension.Image{}
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)
Expand All @@ -138,11 +138,31 @@ func createImageMirrorForInternalImages(prefix string, ref reference.DockerImage
if len(imageSetsFromBinaries) == 0 {
return nil, fmt.Errorf("no test images were reported by external binaries")
}
initialImageSets = imageSetsFromBinaries
externalImageSets = imageSetsFromBinaries
}

// Take the initial images coming from external binaries and remove any exceptions that might exist.
// Convert external images to initial and updated image sets
// Add mapped images to updated image set if they exist
exceptions := image.Exceptions.List()
updatedImageSets := []extensions.ImageSet{}
initial := extensions.ImageSet{}
updated := extensions.ImageSet{}
for _, image := range externalImageSets {
imageConfig := covertMappedImageToImageConfig(image)
if !slices.ContainsFunc(exceptions, func(e string) bool {
return strings.Contains(imageConfig.GetE2EImage(), e)
}) {
initial[k8simage.ImageID(image.Index)] = imageConfig
if image.Mapped != nil {
updated[k8simage.ImageID(image.Index)] = covertMappedImageToImageConfig(*image.Mapped)
}
}
}
if len(initial) > 0 {
initialImageSets = []extensions.ImageSet{initial}
}

// Take the initial images coming from external binaries and remove any exceptions that might exist.
defaultImageSets := []extensions.ImageSet{}
for i := range initialImageSets {
filtered := extensions.ImageSet{}
Expand All @@ -158,12 +178,40 @@ func createImageMirrorForInternalImages(prefix string, ref reference.DockerImage
}
}

// 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()))
// Map initial images to the target repository
for _, img := range defaultImageSets {
for imageID, imageConfig := range img {
// If the imageID is in the updated image set, skip it
if _, ok := updated[imageID]; ok {
continue
}
m := map[string]k8simage.ImageID{
imageConfig.GetE2EImage(): k8simage.ImageID(imageID),
}
mappedImage := map[string]string{}
switch imageID {
// These images are special and can't be run out of the cloud - some because they
// are authenticated, and others because they are not real images. Tests that depend
// on these images can't be run without access to the public internet.
case k8simage.InvalidRegistryImage, k8simage.AgnhostPrivate, k8simage.AuthenticatedAlpine:
mappedImage[imageConfig.GetE2EImage()] = imageConfig.GetE2EImage()
default:
mappedImage = image.GetMappedImages(m, source)
}
ref, err := reference.Parse(mappedImage[imageConfig.GetE2EImage()])
if err != nil {
continue
}
config := k8simage.Config{}
config.SetRegistry(ref.Registry)
config.SetName(ref.RepositoryName())
config.SetVersion(ref.Tag)
updated[k8simage.ImageID(imageID)] = config
}
}

updatedImageSets = []extensions.ImageSet{updated}

openshiftDefaults := image.OriginalImages()
openshiftUpdated := image.GetMappedImages(openshiftDefaults, imagesetup.DefaultTestImageMirrorLocation)

Expand All @@ -178,9 +226,9 @@ func createImageMirrorForInternalImages(prefix string, ref reference.DockerImage
covered := sets.NewString()
for i := range updatedImageSets {
for imageID, imageConfig := range updatedImageSets[i] {
defaultConfig := defaultImageSets[i][imageID]
originalConfig := defaultImageSets[i][imageID]
pullSpec := imageConfig.GetE2EImage()
if pullSpec == defaultConfig.GetE2EImage() {
if pullSpec == originalConfig.GetE2EImage() {
continue
}
if covered.Has(pullSpec) {
Expand Down Expand Up @@ -250,3 +298,12 @@ func createImageMirrorForInternalImages(prefix string, ref reference.DockerImage
sort.Strings(lines)
return lines, nil
}

func covertMappedImageToImageConfig(image extension.Image) k8simage.Config {
imageConfig := k8simage.Config{}
imageConfig.SetName(image.Name)
imageConfig.SetVersion(image.Version)
imageConfig.SetRegistry(image.Registry)

return imageConfig
}
23 changes: 7 additions & 16 deletions pkg/test/extensions/binary.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ func (b *TestBinary) RunTests(ctx context.Context, timeout time.Duration, env []
return results
}

func (b *TestBinary) ListImages(ctx context.Context) (ImageSet, error) {
func (b *TestBinary) ListImages(ctx context.Context) ([]extension.Image, error) {
start := time.Now()
binName := filepath.Base(b.binaryPath)

Expand All @@ -430,23 +430,14 @@ func (b *TestBinary) ListImages(ctx context.Context) (ImageSet, error) {
return nil, fmt.Errorf("failed running '%s list': %w\nOutput: %s", b.binaryPath, err, output)
}

var images []Image
var images []extension.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
return images, nil
}

// ExtractAllTestBinaries determines the optimal release payload to use, and extracts all the external
Expand Down Expand Up @@ -635,9 +626,9 @@ func (binaries TestBinaries) Info(ctx context.Context, parallelism int) ([]*Exte
return infos, nil
}

func (binaries TestBinaries) ListImages(ctx context.Context, parallelism int) ([]ImageSet, error) {
func (binaries TestBinaries) ListImages(ctx context.Context, parallelism int) ([]extension.Image, error) {
var (
allImages []ImageSet
allImages []extension.Image
mu sync.Mutex
wg sync.WaitGroup
errCh = make(chan error, len(binaries))
Expand Down Expand Up @@ -673,12 +664,12 @@ func (binaries TestBinaries) ListImages(ctx context.Context, parallelism int) ([
continue // Skip self - only external binaries need to be queried for images
}

imageConfig, err := binary.ListImages(ctx)
image, err := binary.ListImages(ctx)
if err != nil {
errCh <- err
}
mu.Lock()
allImages = append(allImages, imageConfig)
allImages = append(allImages, image...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change (ImageSet to extension.Image) is making this PR a lot harder to review. We now have confusing var names, like this in images_command.go:

	externalImageSets := []extension.Image{}
	initialImageSets := []extensions.ImageSet{

If you have a chance, and if it makes sense, I'd love to see this kind of change in separate, logical commits.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make changes to funcs in origin/pkg/test/extensions/binary.go but it means some funcs will have a different signatures moving forward. It may simplify the logic in images_command.go a bit but there will still be some changes.

Honestly, I find this whole operation combination (openshift-tests images + oc image mirror) is ugly because we modify from image paths from various image sources (upstream, internal and etc) which have their own sets of filters and modification on their own. Also, there are too many option combinations from two commands (are these mirrored already or not + upsteam or not) which lead to complex logic. Plus, we implemented this mirrored ImageSet only for one binary so it is a special case that is added on top of all of these options.

Anyway, let me make some changes on binary.go to see if those changes there will result in a simpler login on images_command.go side.

mu.Unlock()
}
}
Expand Down
1 change: 1 addition & 0 deletions test/extended/util/image/zz_generated.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# This file is generated by hack/update-generated.sh
docker.io/library/registry:2.8.0-beta.1 quay.io/openshift/community-e2e-images:e2e-docker-io-library-registry-2-8-0-beta-1-8x_YFKSuz9Xw6lZD
gcr.io/authenticated-image-pulling/windows-nanoserver:v1 quay.io/openshift/community-e2e-images:e2e-6-gcr-io-authenticated-image-pulling-windows-nanoserver-v1-JL_XVPCftjC9psmR
quay.io/keycloak/keycloak:25.0 quay.io/openshift/community-e2e-images:e2e-quay-io-keycloak-keycloak-25-0-rEIw9B2Zcc3L1M6k
quay.io/kubevirt/fedora-with-test-tooling-container-disk:20241024_891122a6fc quay.io/openshift/community-e2e-images:e2e-quay-io-kubevirt-fedora-with-test-tooling-container-disk-20241024_891122a6fc-IycYTh-87XrXse4E
quay.io/olmtest/webhook-operator:v0.0.5 quay.io/openshift/community-e2e-images:e2e-quay-io-olmtest-webhook-operator-v0-0-5--2OhUsk-Xv-pLaTI
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1419,7 +1419,7 @@ github.com/opencontainers/runtime-spec/specs-go
github.com/opencontainers/selinux/go-selinux
github.com/opencontainers/selinux/go-selinux/label
github.com/opencontainers/selinux/pkg/pwalkdir
# github.com/openshift-eng/openshift-tests-extension v0.0.0-20250916161632-d81c09058835
# github.com/openshift-eng/openshift-tests-extension v0.0.0-20251104171704-4de6984a8e50
## explicit; go 1.23.0
github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdinfo
github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdlist
Expand Down