diff --git a/pkg/steps/bundle_source.go b/pkg/steps/bundle_source.go index e41d2256b4..d2edf7f5ab 100644 --- a/pkg/steps/bundle_source.go +++ b/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) } diff --git a/pkg/steps/bundle_source_test.go b/pkg/steps/bundle_source_test.go index d30e610598..38b34b4851 100644 --- a/pkg/steps/bundle_source_test.go +++ b/pkg/steps/bundle_source_test.go @@ -2,20 +2,30 @@ 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" + 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 +173,71 @@ 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) + } +} diff --git a/pkg/steps/project_image.go b/pkg/steps/project_image.go index bf605388cf..a062d6f54a 100644 --- a/pkg/steps/project_image.go +++ b/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) } diff --git a/pkg/steps/source.go b/pkg/steps/source.go index d4ea4aa4a4..0b24bc6d86 100644 --- a/pkg/steps/source.go +++ b/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".