From 9b93b6f417521dfae5421954fcec2e1252c60859 Mon Sep 17 00:00:00 2001 From: Hector Vido Date: Mon, 1 Jun 2026 15:03:04 -0300 Subject: [PATCH] Modifying build client to handle different build logics --- pkg/api/config.go | 4 ++++ pkg/api/types.go | 6 ++++++ pkg/defaults/defaults.go | 2 +- pkg/defaults/defaults_test.go | 2 +- pkg/steps/build_client.go | 9 ++++++++- pkg/steps/index_generator_test.go | 2 +- pkg/steps/source.go | 24 +++++++++++++++--------- pkg/steps/source_test.go | 14 +++++++++----- 8 files changed, 45 insertions(+), 18 deletions(-) diff --git a/pkg/api/config.go b/pkg/api/config.go index ae58ba32452..fc71346bfc8 100644 --- a/pkg/api/config.go +++ b/pkg/api/config.go @@ -57,6 +57,10 @@ func (config *ReleaseBuildConfiguration) Default() { for i := range config.Tests { defTest(&config.Tests[i]) } + + if config.BuildType == "" { + config.BuildType = BuildTypeOpenshift + } } // ImageStreamFor guesses at the ImageStream that will hold a tag. diff --git a/pkg/api/types.go b/pkg/api/types.go index 1af626139c2..3b7a201ff6e 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -75,6 +75,7 @@ type ReleaseBuildConfiguration struct { InputConfiguration `json:",inline"` + BuildType string `json:"build_type,omitempty"` // BinaryBuildCommands will create a "bin" image based on "src" that // contains the output of this command. This allows reuse of binary artifacts // across other steps. If empty, no "bin" image will be created. @@ -1822,3 +1823,8 @@ type ClusterClaimOwnerDetails struct { const ( EphemeralClusterTestDoneSignalSecretName = "test-done-signal" ) + +const ( + // BuildTypeOpenshift indicates the Openshift Build + BuildTypeOpenshift = "openshift" +) diff --git a/pkg/defaults/defaults.go b/pkg/defaults/defaults.go index 6827a969af3..7394c2e91d2 100644 --- a/pkg/defaults/defaults.go +++ b/pkg/defaults/defaults.go @@ -60,7 +60,7 @@ func FromConfig(ctx context.Context, cfg *Config) ([]api.Step, []api.Step, error if err != nil { return nil, nil, fmt.Errorf("could not get build client for cluster config: %w", err) } - buildClient := steps.NewBuildClient(client, buildGetter.RESTClient(), cfg.NodeArchitectures, cfg.ManifestToolDockerCfg, cfg.LocalRegistryDNS, cfg.MetricsAgent) + buildClient := steps.NewBuildClient(client, api.BuildTypeOpenshift, buildGetter.RESTClient(), cfg.NodeArchitectures, cfg.ManifestToolDockerCfg, cfg.LocalRegistryDNS, cfg.MetricsAgent) coreGetter, err := coreclientset.NewForConfig(cfg.ClusterConfig) if err != nil { diff --git a/pkg/defaults/defaults_test.go b/pkg/defaults/defaults_test.go index 6f03f119698..e1fa8090c18 100644 --- a/pkg/defaults/defaults_test.go +++ b/pkg/defaults/defaults_test.go @@ -1284,7 +1284,7 @@ func TestFromConfig(t *testing.T) { t.Fatal(err) } } - buildClient := steps.NewBuildClient(client, nil, nil, "", "", nil) + buildClient := steps.NewBuildClient(client, api.BuildTypeOpenshift, nil, nil, "", "", nil) podClient := kubernetes.NewPodClient(client, nil, nil, 0, nil) clusterPool := hivev1.ClusterPool{ diff --git a/pkg/steps/build_client.go b/pkg/steps/build_client.go index 8c2ca4656a6..4c4241e30ad 100644 --- a/pkg/steps/build_client.go +++ b/pkg/steps/build_client.go @@ -21,6 +21,7 @@ type BuildClient interface { ManifestToolDockerCfg() string LocalRegistryDNS() string MetricsAgent() *metrics.MetricsAgent + BuildType() string } type buildClient struct { @@ -30,9 +31,10 @@ type buildClient struct { manifestToolDockerCfg string localRegistryDNS string metricsAgent *metrics.MetricsAgent + buildType string } -func NewBuildClient(client loggingclient.LoggingClient, restClient rest.Interface, nodeArchitectures []string, manifestToolDockerCfg, localRegistryDNS string, metricsAgent *metrics.MetricsAgent) BuildClient { +func NewBuildClient(client loggingclient.LoggingClient, buildType string, restClient rest.Interface, nodeArchitectures []string, manifestToolDockerCfg, localRegistryDNS string, metricsAgent *metrics.MetricsAgent) BuildClient { return &buildClient{ LoggingClient: client, client: restClient, @@ -40,6 +42,7 @@ func NewBuildClient(client loggingclient.LoggingClient, restClient rest.Interfac manifestToolDockerCfg: manifestToolDockerCfg, localRegistryDNS: localRegistryDNS, metricsAgent: metricsAgent, + buildType: buildType, } } @@ -72,3 +75,7 @@ func (c *buildClient) MetricsAgent() *metrics.MetricsAgent { func (c *buildClient) Client() loggingclient.LoggingClient { return c.LoggingClient } + +func (c *buildClient) BuildType() string { + return c.buildType +} diff --git a/pkg/steps/index_generator_test.go b/pkg/steps/index_generator_test.go index a0043d65a89..aeb87628657 100644 --- a/pkg/steps/index_generator_test.go +++ b/pkg/steps/index_generator_test.go @@ -231,7 +231,7 @@ func TestDatabaseIndex(t *testing.T) { if err := yaml.Unmarshal(rawImageStreamTag, ist); err != nil { t.Fatalf("failed to unmarshal imagestreamTag: %v", err) } - actual, actualErr := databaseIndex(NewBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithObjects(ist, image).Build(), nil), nil, nil, "", "", nil), + actual, actualErr := databaseIndex(NewBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithObjects(ist, image).Build(), nil), api.BuildTypeOpenshift, nil, nil, "", "", nil), testCase.isTagName, "ns") if diff := cmp.Diff(testCase.expectedErr, actualErr, testhelper.EquateErrorMessage); diff != "" { t.Fatalf("actual did not match expected, diff: %s", diff) diff --git a/pkg/steps/source.go b/pkg/steps/source.go index 9ea8b4f0186..b6da1fa6e9a 100644 --- a/pkg/steps/source.go +++ b/pkg/steps/source.go @@ -492,9 +492,15 @@ func handleBuilds(ctx context.Context, buildClient BuildClient, podClient kubern wg.Add(len(builds)) for _, build := range builds { go func(b buildapi.Build) { + var err error defer wg.Done() metricsAgent.AddNodeWorkload(ctx, b.Namespace, fmt.Sprintf("%s-build", b.Name), b.Name, podClient) - if err := handleBuild(ctx, buildClient, podClient, b); err != nil { + if buildClient.BuildType() == api.BuildTypeOpenshift { + err = handleOpenshiftBuild(ctx, buildClient, podClient, b) + } else { + err = fmt.Errorf("undefined build type %s", buildClient.BuildType()) + } + if err != nil { errChan <- fmt.Errorf("error occurred handling build %s: %w", b.Name, err) } metricsAgent.RemoveNodeWorkload(b.Name) @@ -552,7 +558,7 @@ func constructMultiArchBuilds(build buildapi.Build, stepArchitectures []string) return ret } -func handleBuild(ctx context.Context, client BuildClient, podClient kubernetes.PodClient, build buildapi.Build) error { +func handleOpenshiftBuild(ctx context.Context, buildClient BuildClient, podClient kubernetes.PodClient, build buildapi.Build) error { const attempts = 5 ns, name := build.Namespace, build.Name var errs []error @@ -560,7 +566,7 @@ func handleBuild(ctx context.Context, client BuildClient, podClient kubernetes.P var attempt buildapi.Build build.DeepCopyInto(&attempt) - if err := client.Create(ctx, &attempt); err == nil { + if err := buildClient.Create(ctx, &attempt); err == nil { logrus.Infof("Created build %q", name) } else if kerrors.IsAlreadyExists(err) { logrus.Infof("Found existing build %q", name) @@ -568,19 +574,19 @@ func handleBuild(ctx context.Context, client BuildClient, podClient kubernetes.P return false, fmt.Errorf("could not create build %s: %w", name, err) } - client.MetricsAgent().AddNodeWorkload(ctx, ns, fmt.Sprintf("%s-build", name), name, podClient) - client.MetricsAgent().StoreMachinesSnapshotForBuildPod(ctx, ns, fmt.Sprintf("%s-build", name), podClient) - if err := waitForBuildOrTimeout(ctx, client, podClient, ns, name); err != nil { + buildClient.MetricsAgent().AddNodeWorkload(ctx, ns, fmt.Sprintf("%s-build", name), name, podClient) + buildClient.MetricsAgent().StoreMachinesSnapshotForBuildPod(ctx, ns, fmt.Sprintf("%s-build", name), podClient) + if err := waitForBuildOrTimeout(ctx, buildClient, podClient, ns, name); err != nil { errs = append(errs, err) - return false, handleFailedBuild(ctx, client, ns, name, err) + return false, handleFailedBuild(ctx, buildClient, ns, name, err) } - if err := gatherSuccessfulBuildLog(client, ns, name); err != nil { + if err := gatherSuccessfulBuildLog(buildClient, ns, name); err != nil { // log error but do not fail successful build logrus.WithError(err).Warnf("Failed gathering successful build %s logs into artifacts.", name) } return true, nil }); err != nil { - if err == wait.ErrWaitTimeout { + if wait.Interrupted(err) { return fmt.Errorf("build not successful after %d attempts: %w", attempts, utilerrors.NewAggregate(errs)) } return err diff --git a/pkg/steps/source_test.go b/pkg/steps/source_test.go index 38d82a72353..c087ca8f4db 100644 --- a/pkg/steps/source_test.go +++ b/pkg/steps/source_test.go @@ -401,7 +401,7 @@ func TestWaitForBuild(t *testing.T) { CompletionTimestamp: &end, }, }, - ).Build(), nil), nil, nil, "", "", nil), + ).Build(), nil), api.BuildTypeOpenshift, nil, nil, "", "", nil), expected: fmt.Errorf("build didn't start running within 0s (phase: Pending)"), }, { @@ -430,7 +430,7 @@ func TestWaitForBuild(t *testing.T) { Namespace: ns, }, }, - ).Build(), nil), nil, nil, "", "", nil), + ).Build(), nil), api.BuildTypeOpenshift, nil, nil, "", "", nil), expected: fmt.Errorf("build didn't start running within 0s (phase: Pending):\nFound 0 events for Pod some-build-build:"), }, { @@ -471,7 +471,7 @@ func TestWaitForBuild(t *testing.T) { }}, }, }, - ).Build(), nil), nil, nil, "", "", nil), + ).Build(), nil), api.BuildTypeOpenshift, nil, nil, "", "", nil), expected: fmt.Errorf(`build didn't start running within 0s (phase: Pending): * Container the-container is not ready with reason the_reason and message the_message Found 0 events for Pod some-build-build:`), @@ -490,7 +490,7 @@ Found 0 events for Pod some-build-build:`), StartTimestamp: &start, CompletionTimestamp: &end, }, - }).Build(), nil), nil, nil, "", "", nil), + }).Build(), nil), api.BuildTypeOpenshift, nil, nil, "", "", nil), timeout: 30 * time.Minute, }, { @@ -534,7 +534,7 @@ Found 0 events for Pod some-build-build:`), Time: now.Add(-59 * time.Minute), }, }, - }).Build(), nil), nil, nil, "", "", nil), + }).Build(), nil), api.BuildTypeOpenshift, nil, nil, "", "", nil), timeout: 30 * time.Minute, }, { @@ -721,6 +721,10 @@ func (c *fakeBuildClient) Client() loggingclient.LoggingClient { return c.LoggingClient } +func (c *fakeBuildClient) BuildType() string { + return api.BuildTypeOpenshift +} + func Test_constructMultiArchBuilds(t *testing.T) { tests := []struct { name string