pkg/steps: pin bundle builds to amd64 nodes

pkg/steps/bundle_source.go

@@ -80,8 +80,9 @@ func (s *bundleSourceStep) run(ctx context.Context) error {
 	)
 
 	// Bundle images are not multi-arch by design. Here we build it without creating a manifest-listed image.
-	// Note that we are not configuring a node selector here, so the build will be scheduled on any available
-	// node no matter the architecture.
+	// The build must still run on a node whose architecture can resolve its manifest-listed inputs
+	// (e.g. pipeline:src) — see pinBuildToSingleArchNode.
+	pinBuildToSingleArchNode(build)
 	return handleBuild(ctx, s.client, s.podClient, *build)
 }
 

pkg/steps/bundle_source_test.go

@@ -2,20 +2,31 @@ package steps
 
 import (
 	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"testing"
+	"time"
 
+	coreapi "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8sruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
+	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	fakectrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	buildapi "github.com/openshift/api/build/v1"
+	"github.com/openshift/api/image/docker10"
 	imagev1 "github.com/openshift/api/image/v1"
 
 	"github.com/openshift/ci-tools/pkg/api"
 	"github.com/openshift/ci-tools/pkg/steps/loggingclient"
+	testhelper_kube "github.com/openshift/ci-tools/pkg/testhelper/kubernetes"
 )
 
 var subs = []api.PullSpecSubstitution{
@@ -163,3 +174,143 @@ RUN find . -type f -regex ".*\.\(yaml\|yml\)" -exec sed -i s?quay.io/openshift/o
 		t.Errorf("Generated bundle source dockerfile does not equal expected; generated dockerfile: %s", generatedDockerfile)
 	}
 }
+
+// pendingOnCreateBuildClient stamps newly created builds as Pending before they're persisted,
+// mimicking what a real build controller does immediately on admission. Without this, a build
+// created by the fake client sits at its zero-value phase forever (nothing ever transitions it),
+// so waitForBuild's pending-check — gated on seeing phase New/Pending on the *first* watched
+// event — never fires and the wait blocks until the context is done.
+type pendingOnCreateBuildClient struct {
+	BuildClient
+}
+
+func (c pendingOnCreateBuildClient) Create(ctx context.Context, obj ctrlruntimeclient.Object, opts ...ctrlruntimeclient.CreateOption) error {
+	if b, ok := obj.(*buildapi.Build); ok {
+		b.Status.Phase = buildapi.BuildPhasePending
+	}
+	return c.BuildClient.Create(ctx, obj, opts...)
+}
+
+// TestBundleSourceStepRunPinsBuildToSingleArchNode is a direct unit test of the code path
+// pinBuildToSingleArchNode is called from: it exercises bundleSourceStep.Run and inspects the
+// actual build object submitted to the API, rather than the invariants
+// TestSingleArchBuildPinMatchesPipelineArchGuarantees checks in isolation.
+func TestBundleSourceStepRunPinsBuildToSingleArchNode(t *testing.T) {
+	const namespace = "target-namespace"
+	metadata, err := json.Marshal(docker10.DockerImage{Config: &docker10.DockerConfig{WorkingDir: "/go/src"}})
+	if err != nil {
+		t.Fatalf("failed to marshal image metadata: %v", err)
+	}
+	underlying := &buildClient{LoggingClient: loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithRuntimeObjects(
+		&imagev1.ImageStreamTag{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: namespace,
+				Name:      fmt.Sprintf("%s:%s", api.PipelineImageStream, api.PipelineImageStreamTagReferenceSource),
+			},
+			Image: imagev1.Image{
+				ObjectMeta:          metav1.ObjectMeta{Name: "source-digest"},
+				DockerImageMetadata: k8sruntime.RawExtension{Raw: metadata},
+			},
+		},
+	).Build(), nil)}
+	client := pendingOnCreateBuildClient{BuildClient: underlying}
+	podClient := &testhelper_kube.FakePodClient{
+		FakePodExecutor: &testhelper_kube.FakePodExecutor{LoggingClient: client},
+		PendingTimeout:  0,
+	}
+
+	s := &bundleSourceStep{
+		jobSpec:   &api.JobSpec{},
+		client:    client,
+		podClient: podClient,
+	}
+	s.jobSpec.SetNamespace(namespace)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+	err = s.Run(ctx)
+	if err == nil || !strings.Contains(err.Error(), "didn't start running") {
+		t.Fatalf("expected a pending-timeout error proving the build was created and watched, got: %v", err)
+	}
+
+	created := &buildapi.Build{}
+	key := ctrlruntimeclient.ObjectKey{Namespace: namespace, Name: string(api.PipelineImageStreamTagReferenceBundleSource)}
+	if err := client.Get(context.Background(), key, created); err != nil {
+		t.Fatalf("failed to get created build: %v", err)
+	}
+	if arch := created.Spec.NodeSelector[coreapi.LabelArchStable]; arch != string(api.NodeArchitectureAMD64) {
+		t.Errorf("Run() submitted a build with %s=%q, want %q", coreapi.LabelArchStable, arch, api.NodeArchitectureAMD64)
+	}
+}
+
+// TestSingleArchBuildPinMatchesPipelineArchGuarantees guards the cross-file invariant behind
+// pinBuildToSingleArchNode (used for bundle builds here and in project_image.go): pinning bundle
+// builds to one architecture is only safe while every pipeline manifest list is guaranteed to
+// contain that architecture. If any assertion below fails, that guarantee changed — update
+// pinBuildToSingleArchNode together with the change that broke it.
+func TestSingleArchBuildPinMatchesPipelineArchGuarantees(t *testing.T) {
+	pinned := &buildapi.Build{}
+	pinBuildToSingleArchNode(pinned)
+	pinnedArch, ok := pinned.Spec.NodeSelector[coreapi.LabelArchStable]
+	if !ok {
+		t.Fatalf("pinBuildToSingleArchNode did not set a %s node selector", coreapi.LabelArchStable)
+	}
+
+	// This subtest never calls pinBuildToSingleArchNode: it asserts the pipeline-side
+	// invariant the pin depends on (default builds target the pinned arch), so a change
+	// to constructMultiArchBuilds that breaks that invariant fails here first.
+	t.Run("default pipeline build is the pinned architecture", func(t *testing.T) {
+		builds := constructMultiArchBuilds(buildapi.Build{}, nil)
+		if len(builds) != 1 {
+			t.Fatalf("expected exactly one default build, got %d", len(builds))
+		}
+		if got := builds[0].Spec.NodeSelector[coreapi.LabelArchStable]; got != pinnedArch {
+			t.Errorf("default pipeline build targets %q, but bundle builds pin to %q", got, pinnedArch)
+		}
+	})
+
+	// Also does not call pinBuildToSingleArchNode: guards ResolveMultiArch() itself, since a
+	// change there could drop the pinned arch from the resolved set without touching the pin.
+	t.Run("image steps resolve the pinned architecture unconditionally", func(t *testing.T) {
+		step := &projectDirectoryImageBuildStep{
+			config: api.ProjectDirectoryImageBuildStepConfiguration{
+				AdditionalArchitectures: []string{"arm64"},
+			},
+			architectures: sets.New[string](),
+		}
+		if resolved := step.ResolveMultiArch(); !resolved.Has(pinnedArch) {
+			t.Errorf("ResolveMultiArch() = %v does not contain %q, so a pipeline manifest list without a %q entry is now possible", sets.List(resolved), pinnedArch, pinnedArch)
+		}
+	})
+
+	// Also does not call pinBuildToSingleArchNode: guards constructMultiArchBuilds() across
+	// multiple configured architectures, the case pinning a bundle build actually relies on.
+	t.Run("multi-arch pipelines still build the pinned architecture", func(t *testing.T) {
+		step := &projectDirectoryImageBuildStep{
+			config: api.ProjectDirectoryImageBuildStepConfiguration{
+				AdditionalArchitectures: []string{"arm64", "ppc64le"},
+			},
+			architectures: sets.New[string](),
+		}
+		found := false
+		for _, b := range constructMultiArchBuilds(buildapi.Build{}, sets.List(step.ResolveMultiArch())) {
+			if b.Spec.NodeSelector[coreapi.LabelArchStable] == pinnedArch {
+				found = true
+			}
+		}
+		if !found {
+			t.Errorf("no %q build produced for a multi-arch pipeline, so its manifest list would lack the entry bundle builds pin to", pinnedArch)
+		}
+	})
+
+	t.Run("pinning merges into an existing node selector", func(t *testing.T) {
+		build := &buildapi.Build{Spec: buildapi.BuildSpec{CommonSpec: buildapi.CommonSpec{NodeSelector: map[string]string{"foo": "bar"}}}}
+		pinBuildToSingleArchNode(build)
+		if build.Spec.NodeSelector["foo"] != "bar" {
+			t.Error("pinBuildToSingleArchNode replaced the pre-existing node selector instead of merging into it")
+		}
+		if build.Spec.NodeSelector[coreapi.LabelArchStable] != pinnedArch {
+			t.Errorf("pinBuildToSingleArchNode did not set %s=%s", coreapi.LabelArchStable, pinnedArch)
+		}
+	})
+}

pkg/steps/project_image.go

@@ -71,7 +71,10 @@ func (s *projectDirectoryImageBuildStep) run(ctx context.Context) error {
 	)
 
 	// Bundle images are non multi-arch by design. No manifest list is needed. Here we spawn a single build.
+	// The build must still run on a node whose architecture can resolve its manifest-listed inputs —
+	// see pinBuildToSingleArchNode.
 	if s.config.IsBundleImage() {
+		pinBuildToSingleArchNode(build)
 		return handleBuild(ctx, s.client, s.podClient, *build)
 	}
 

pkg/steps/source.go

@@ -525,6 +525,20 @@ func handleBuilds(ctx context.Context, buildClient BuildClient, podClient kubern
 	return utilerrors.NewAggregate(errs)
 }
 
+// pinBuildToSingleArchNode pins a non-manifest-listed build (bundle images) to nodes of the one
+// architecture every pipeline manifest list is guaranteed to contain: constructMultiArchBuilds
+// builds NodeArchitectureAMD64 by default and AdditionalArchitectures/Capabilities are additive
+// (see ProjectDirectoryImageBuildStepConfiguration.AllCapabilities and ResolveMultiArch), so amd64
+// nodes can always resolve the build's inputs. Selector keys other than the architecture label are
+// merged rather than replaced so a selector set earlier on the build survives; any pre-existing
+// architecture value is intentionally overwritten with the pinned architecture.
+func pinBuildToSingleArchNode(build *buildapi.Build) {
+	if build.Spec.NodeSelector == nil {
+		build.Spec.NodeSelector = map[string]string{}
+	}
+	build.Spec.NodeSelector[corev1.LabelArchStable] = string(api.NodeArchitectureAMD64)
+}
+
 // constructMultiArchBuilds gets a specific build and constructs multiple builds for each architecture.
 // The name and the output image of the build is suffixed with the architecture name and it will include the nodeSelector for the specific architecture.
 // e.x if the build name is "foo" and the architectures are "amd64,arm64", the new builds will be "foo-amd64" and "foo-arm64".