diff --git a/cmd/ci-operator/main_test.go b/cmd/ci-operator/main_test.go index ab88fd6bcf7..2541e01ba56 100644 --- a/cmd/ci-operator/main_test.go +++ b/cmd/ci-operator/main_test.go @@ -992,7 +992,7 @@ func TestBuildPartialGraph(t *testing.T) { loggingclient.New(fakectrlruntimeclient.NewFakeClient(&imagev1.ImageStreamTag{ObjectMeta: metav1.ObjectMeta{Name: ":"}})), nil, ), - steps.SourceStep(api.SourceStepConfiguration{From: api.PipelineImageStreamTagReferenceRoot, To: api.PipelineImageStreamTagReferenceSource}, api.ResourceConfiguration{}, nil, nil, &api.JobSpec{}, nil, nil), + steps.SourceStep(api.SourceStepConfiguration{From: api.PipelineImageStreamTagReferenceRoot, To: api.PipelineImageStreamTagReferenceSource}, api.ResourceConfiguration{}, nil, &api.JobSpec{}, nil, nil), steps.ProjectDirectoryImageBuildStep( api.ProjectDirectoryImageBuildStepConfiguration{ From: api.PipelineImageStreamTagReferenceSource, @@ -1001,7 +1001,7 @@ func TestBuildPartialGraph(t *testing.T) { }, To: api.PipelineImageStreamTagReference("oc-bin-image"), }, - &api.ReleaseBuildConfiguration{}, api.ResourceConfiguration{}, nil, nil, nil, nil, + &api.ReleaseBuildConfiguration{}, api.ResourceConfiguration{}, nil, nil, nil, ), steps.OutputImageTagStep(api.OutputImageTagStepConfiguration{From: api.PipelineImageStreamTagReference("oc-bin-image")}, nil, nil), steps.ImagesReadyStep(steps.OutputImageTagStep(api.OutputImageTagStepConfiguration{From: api.PipelineImageStreamTagReference("oc-bin-image")}, nil, nil).Creates()), diff --git a/pkg/defaults/defaults.go b/pkg/defaults/defaults.go index eec19c03250..ec8e482ddc6 100644 --- a/pkg/defaults/defaults.go +++ b/pkg/defaults/defaults.go @@ -248,19 +248,19 @@ func fromConfig( step = steps.InputImageTagStep(&conf, client, jobSpec) inputImages[conf.InputImage] = struct{}{} } else if rawStep.PipelineImageCacheStepConfiguration != nil { - step = steps.PipelineImageCacheStep(*rawStep.PipelineImageCacheStepConfiguration, config.Resources, buildClient, podClient, jobSpec, pullSecret) + step = steps.PipelineImageCacheStep(*rawStep.PipelineImageCacheStepConfiguration, config.Resources, buildClient, jobSpec, pullSecret) } else if rawStep.SourceStepConfiguration != nil { - step = steps.SourceStep(*rawStep.SourceStepConfiguration, config.Resources, buildClient, podClient, jobSpec, cloneAuthConfig, pullSecret) + step = steps.SourceStep(*rawStep.SourceStepConfiguration, config.Resources, buildClient, jobSpec, cloneAuthConfig, pullSecret) } else if rawStep.BundleSourceStepConfiguration != nil { - step = steps.BundleSourceStep(*rawStep.BundleSourceStepConfiguration, config, config.Resources, buildClient, podClient, jobSpec, pullSecret) + step = steps.BundleSourceStep(*rawStep.BundleSourceStepConfiguration, config, config.Resources, buildClient, jobSpec, pullSecret) } else if rawStep.IndexGeneratorStepConfiguration != nil { - step = steps.IndexGeneratorStep(*rawStep.IndexGeneratorStepConfiguration, config, config.Resources, buildClient, podClient, jobSpec, pullSecret) + step = steps.IndexGeneratorStep(*rawStep.IndexGeneratorStepConfiguration, config, config.Resources, buildClient, jobSpec, pullSecret) } else if rawStep.ProjectDirectoryImageBuildStepConfiguration != nil { - step = steps.ProjectDirectoryImageBuildStep(*rawStep.ProjectDirectoryImageBuildStepConfiguration, config, config.Resources, buildClient, podClient, jobSpec, pullSecret) + step = steps.ProjectDirectoryImageBuildStep(*rawStep.ProjectDirectoryImageBuildStepConfiguration, config, config.Resources, buildClient, jobSpec, pullSecret) } else if rawStep.ProjectDirectoryImageBuildInputs != nil { - step = steps.GitSourceStep(*rawStep.ProjectDirectoryImageBuildInputs, config.Resources, buildClient, podClient, jobSpec, cloneAuthConfig, pullSecret) + step = steps.GitSourceStep(*rawStep.ProjectDirectoryImageBuildInputs, config.Resources, buildClient, jobSpec, cloneAuthConfig, pullSecret) } else if rawStep.RPMImageInjectionStepConfiguration != nil { - step = steps.RPMImageInjectionStep(*rawStep.RPMImageInjectionStepConfiguration, config.Resources, buildClient, podClient, jobSpec, pullSecret) + step = steps.RPMImageInjectionStep(*rawStep.RPMImageInjectionStepConfiguration, config.Resources, buildClient, jobSpec, pullSecret) } else if rawStep.RPMServeStepConfiguration != nil { step = steps.RPMServerStep(*rawStep.RPMServeStepConfiguration, client, jobSpec) } else if rawStep.OutputImageTagStepConfiguration != nil { diff --git a/pkg/kubernetes/client.go b/pkg/kubernetes/client.go index bbdb73d4870..fdf449987cb 100644 --- a/pkg/kubernetes/client.go +++ b/pkg/kubernetes/client.go @@ -71,7 +71,7 @@ func WaitForConditionOnObject(ctx context.Context, client ctrlruntimeclient.With type PodClient interface { loggingclient.LoggingClient - GetPendingTimeout() time.Duration + PendingTimeout() time.Duration // WithNewLoggingClient returns a new instance of the PodClient that resets // its LoggingClient. WithNewLoggingClient() PodClient @@ -95,7 +95,7 @@ type podClient struct { pendingTimeout time.Duration } -func (c podClient) GetPendingTimeout() time.Duration { return c.pendingTimeout } +func (c podClient) PendingTimeout() time.Duration { return c.pendingTimeout } func (c podClient) Exec(namespace, pod string, opts *coreapi.PodExecOptions) (remotecommand.Executor, error) { u := c.client.Post().Resource("pods").Namespace(namespace).Name(pod).SubResource("exec").VersionedParams(opts, scheme.ParameterCodec).URL() diff --git a/pkg/steps/bundle_source.go b/pkg/steps/bundle_source.go index 4591a2f21e3..b20816f17ce 100644 --- a/pkg/steps/bundle_source.go +++ b/pkg/steps/bundle_source.go @@ -13,7 +13,6 @@ import ( buildapi "github.com/openshift/api/build/v1" "github.com/openshift/ci-tools/pkg/api" - "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/results" "github.com/openshift/ci-tools/pkg/steps/utils" ) @@ -23,7 +22,6 @@ type bundleSourceStep struct { releaseBuildConfig *api.ReleaseBuildConfiguration resources api.ResourceConfiguration client BuildClient - podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -77,7 +75,7 @@ func (s *bundleSourceStep) run(ctx context.Context) error { s.pullSecret, nil, ) - return handleBuilds(ctx, s.client, s.podClient, *build) + return handleBuilds(ctx, s.client, *build) } func replaceCommand(pullSpec, with string) string { @@ -131,21 +129,12 @@ func (s *bundleSourceStep) Description() string { return fmt.Sprintf("Build image %s from the repository", api.PipelineImageStreamTagReferenceBundleSource) } -func BundleSourceStep( - config api.BundleSourceStepConfiguration, - releaseBuildConfig *api.ReleaseBuildConfiguration, - resources api.ResourceConfiguration, - client BuildClient, - podClient kubernetes.PodClient, - jobSpec *api.JobSpec, - pullSecret *coreapi.Secret, -) api.Step { +func BundleSourceStep(config api.BundleSourceStepConfiguration, releaseBuildConfig *api.ReleaseBuildConfiguration, resources api.ResourceConfiguration, client BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { return &bundleSourceStep{ config: config, releaseBuildConfig: releaseBuildConfig, resources: resources, client: client, - podClient: podClient, jobSpec: jobSpec, pullSecret: pullSecret, } diff --git a/pkg/steps/git_source.go b/pkg/steps/git_source.go index 6198f701a58..1a70204709f 100644 --- a/pkg/steps/git_source.go +++ b/pkg/steps/git_source.go @@ -11,7 +11,6 @@ import ( buildapi "github.com/openshift/api/build/v1" "github.com/openshift/ci-tools/pkg/api" - "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/results" ) @@ -19,7 +18,6 @@ type gitSourceStep struct { config api.ProjectDirectoryImageBuildInputs resources api.ResourceConfiguration buildClient BuildClient - podClient kubernetes.PodClient jobSpec *api.JobSpec cloneAuthConfig *CloneAuthConfig pullSecret *coreapi.Secret @@ -44,7 +42,7 @@ func (s *gitSourceStep) run(ctx context.Context) error { secretName = s.cloneAuthConfig.Secret.Name } - return handleBuilds(ctx, s.buildClient, s.podClient, *buildFromSource(s.jobSpec, "", api.PipelineImageStreamTagReferenceRoot, buildapi.BuildSource{ + return handleBuilds(ctx, s.buildClient, *buildFromSource(s.jobSpec, "", api.PipelineImageStreamTagReferenceRoot, buildapi.BuildSource{ Type: buildapi.BuildSourceGit, Dockerfile: s.config.DockerfileLiteral, ContextDir: s.config.ContextDir, @@ -101,20 +99,11 @@ func determineRefsWorkdir(refs *prowapi.Refs, extraRefs []prowapi.Refs) *prowapi } // GitSourceStep returns gitSourceStep that holds all the required information to create a build from a git source. -func GitSourceStep( - config api.ProjectDirectoryImageBuildInputs, - resources api.ResourceConfiguration, - buildClient BuildClient, - podClient kubernetes.PodClient, - jobSpec *api.JobSpec, - cloneAuthConfig *CloneAuthConfig, - pullSecret *coreapi.Secret, -) api.Step { +func GitSourceStep(config api.ProjectDirectoryImageBuildInputs, resources api.ResourceConfiguration, buildClient BuildClient, jobSpec *api.JobSpec, cloneAuthConfig *CloneAuthConfig, pullSecret *coreapi.Secret) api.Step { return &gitSourceStep{ config: config, resources: resources, buildClient: buildClient, - podClient: podClient, jobSpec: jobSpec, cloneAuthConfig: cloneAuthConfig, pullSecret: pullSecret, diff --git a/pkg/steps/index_generator.go b/pkg/steps/index_generator.go index d8ea1632d59..803c85e8702 100644 --- a/pkg/steps/index_generator.go +++ b/pkg/steps/index_generator.go @@ -16,7 +16,6 @@ import ( imagev1 "github.com/openshift/api/image/v1" "github.com/openshift/ci-tools/pkg/api" - "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/results" "github.com/openshift/ci-tools/pkg/steps/utils" ) @@ -26,7 +25,6 @@ type indexGeneratorStep struct { releaseBuildConfig *api.ReleaseBuildConfiguration resources api.ResourceConfiguration client BuildClient - podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -123,7 +121,7 @@ func (s *indexGeneratorStep) run(ctx context.Context) error { s.pullSecret, nil, ) - err = handleBuilds(ctx, s.client, s.podClient, *build) + err = handleBuilds(ctx, s.client, *build) if err != nil && strings.Contains(err.Error(), "error checking provided apis") { return results.ForReason("generating_index").WithError(err).Errorf("failed to generate operator index due to invalid bundle info: %v", err) } @@ -197,21 +195,12 @@ func (s *indexGeneratorStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func IndexGeneratorStep( - config api.IndexGeneratorStepConfiguration, - releaseBuildConfig *api.ReleaseBuildConfiguration, - resources api.ResourceConfiguration, - buildClient BuildClient, - podClient kubernetes.PodClient, - jobSpec *api.JobSpec, - pullSecret *coreapi.Secret, -) api.Step { +func IndexGeneratorStep(config api.IndexGeneratorStepConfiguration, releaseBuildConfig *api.ReleaseBuildConfiguration, resources api.ResourceConfiguration, buildClient BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { return &indexGeneratorStep{ config: config, releaseBuildConfig: releaseBuildConfig, resources: resources, client: buildClient, - podClient: podClient, jobSpec: jobSpec, pullSecret: pullSecret, } diff --git a/pkg/steps/multi_stage/run_test.go b/pkg/steps/multi_stage/run_test.go index cefd493e554..866f7aed83d 100644 --- a/pkg/steps/multi_stage/run_test.go +++ b/pkg/steps/multi_stage/run_test.go @@ -88,10 +88,7 @@ func TestRun(t *testing.T) { }, } jobSpec.SetNamespace("ns") - client := &testhelper_kube.FakePodClient{ - PendingTimeout: 30 * time.Minute, - FakePodExecutor: crclient, - } + client := &testhelper_kube.FakePodClient{FakePodExecutor: crclient} step := MultiStageTestStep(api.TestStepConfiguration{ As: name, MultiStageTestConfigurationLiteral: &api.MultiStageTestConfigurationLiteral{ diff --git a/pkg/steps/pipeline_image_cache.go b/pkg/steps/pipeline_image_cache.go index 5b8f399f89f..cb4598c8b8b 100644 --- a/pkg/steps/pipeline_image_cache.go +++ b/pkg/steps/pipeline_image_cache.go @@ -11,7 +11,6 @@ import ( buildapi "github.com/openshift/api/build/v1" "github.com/openshift/ci-tools/pkg/api" - "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/results" "github.com/openshift/ci-tools/pkg/steps/utils" ) @@ -25,7 +24,6 @@ type pipelineImageCacheStep struct { config api.PipelineImageCacheStepConfiguration resources api.ResourceConfiguration client BuildClient - podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -46,7 +44,7 @@ func (s *pipelineImageCacheStep) run(ctx context.Context) error { if err != nil { return err } - return handleBuild(ctx, s.client, s.podClient, *buildFromSource( + return handleBuild(ctx, s.client, *buildFromSource( s.jobSpec, s.config.From, s.config.To, buildapi.BuildSource{ Type: buildapi.BuildSourceDockerfile, @@ -87,19 +85,11 @@ func (s *pipelineImageCacheStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func PipelineImageCacheStep( - config api.PipelineImageCacheStepConfiguration, - resources api.ResourceConfiguration, - client BuildClient, - podClient kubernetes.PodClient, - jobSpec *api.JobSpec, - pullSecret *coreapi.Secret, -) api.Step { +func PipelineImageCacheStep(config api.PipelineImageCacheStepConfiguration, resources api.ResourceConfiguration, client BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { return &pipelineImageCacheStep{ config: config, resources: resources, client: client, - podClient: podClient, jobSpec: jobSpec, pullSecret: pullSecret, } diff --git a/pkg/steps/project_image.go b/pkg/steps/project_image.go index 9e30df26b41..fe8c8a80b3d 100644 --- a/pkg/steps/project_image.go +++ b/pkg/steps/project_image.go @@ -14,7 +14,6 @@ import ( imagev1 "github.com/openshift/api/image/v1" "github.com/openshift/ci-tools/pkg/api" - "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/results" "github.com/openshift/ci-tools/pkg/steps/utils" ) @@ -24,7 +23,6 @@ type projectDirectoryImageBuildStep struct { releaseBuildConfig *api.ReleaseBuildConfiguration resources api.ResourceConfiguration client BuildClient - podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -63,7 +61,7 @@ func (s *projectDirectoryImageBuildStep) run(ctx context.Context) error { s.pullSecret, s.config.BuildArgs, ) - return handleBuilds(ctx, s.client, s.podClient, *build) + return handleBuilds(ctx, s.client, *build) } type workingDir func(tag string) (string, error) @@ -163,21 +161,12 @@ func (s *projectDirectoryImageBuildStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func ProjectDirectoryImageBuildStep( - config api.ProjectDirectoryImageBuildStepConfiguration, - releaseBuildConfig *api.ReleaseBuildConfiguration, - resources api.ResourceConfiguration, - buildClient BuildClient, - podClient kubernetes.PodClient, - jobSpec *api.JobSpec, - pullSecret *coreapi.Secret, -) api.Step { +func ProjectDirectoryImageBuildStep(config api.ProjectDirectoryImageBuildStepConfiguration, releaseBuildConfig *api.ReleaseBuildConfiguration, resources api.ResourceConfiguration, buildClient BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { return &projectDirectoryImageBuildStep{ config: config, releaseBuildConfig: releaseBuildConfig, resources: resources, client: buildClient, - podClient: podClient, jobSpec: jobSpec, pullSecret: pullSecret, } diff --git a/pkg/steps/rpm_injection.go b/pkg/steps/rpm_injection.go index cbaec00b772..7777933293c 100644 --- a/pkg/steps/rpm_injection.go +++ b/pkg/steps/rpm_injection.go @@ -11,7 +11,6 @@ import ( routev1 "github.com/openshift/api/route/v1" "github.com/openshift/ci-tools/pkg/api" - "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/results" ) @@ -24,7 +23,6 @@ type rpmImageInjectionStep struct { config api.RPMImageInjectionStepConfiguration resources api.ResourceConfiguration client BuildClient - podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -50,7 +48,7 @@ func (s *rpmImageInjectionStep) run(ctx context.Context) error { if err != nil { return err } - return handleBuilds(ctx, s.client, s.podClient, *buildFromSource( + return handleBuilds(ctx, s.client, *buildFromSource( s.jobSpec, s.config.From, s.config.To, buildapi.BuildSource{ Type: buildapi.BuildSourceDockerfile, @@ -86,19 +84,11 @@ func (s *rpmImageInjectionStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func RPMImageInjectionStep( - config api.RPMImageInjectionStepConfiguration, - resources api.ResourceConfiguration, - buildClient BuildClient, - podClient kubernetes.PodClient, - jobSpec *api.JobSpec, - pullSecret *coreapi.Secret, -) api.Step { +func RPMImageInjectionStep(config api.RPMImageInjectionStepConfiguration, resources api.ResourceConfiguration, buildClient BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { return &rpmImageInjectionStep{ config: config, resources: resources, client: buildClient, - podClient: podClient, jobSpec: jobSpec, pullSecret: pullSecret, } diff --git a/pkg/steps/source.go b/pkg/steps/source.go index f6cdfec90db..d524f15d71d 100644 --- a/pkg/steps/source.go +++ b/pkg/steps/source.go @@ -8,11 +8,9 @@ import ( "sort" "strings" "sync" - "sync/atomic" "time" "github.com/sirupsen/logrus" - "golang.org/x/sync/errgroup" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" @@ -137,7 +135,6 @@ type sourceStep struct { config api.SourceStepConfiguration resources api.ResourceConfiguration client BuildClient - podClient kubernetes.PodClient jobSpec *api.JobSpec cloneAuthConfig *CloneAuthConfig pullSecret *corev1.Secret @@ -163,7 +160,7 @@ func (s *sourceStep) run(ctx context.Context) error { if err != nil { return err } - return handleBuilds(ctx, s.client, s.podClient, *createBuild(s.config, s.jobSpec, clonerefsRef, s.resources, s.cloneAuthConfig, s.pullSecret, fromDigest)) + return handleBuilds(ctx, s.client, *createBuild(s.config, s.jobSpec, clonerefsRef, s.resources, s.cloneAuthConfig, s.pullSecret, fromDigest)) } func createBuild(config api.SourceStepConfiguration, jobSpec *api.JobSpec, clonerefsRef corev1.ObjectReference, resources api.ResourceConfiguration, cloneAuthConfig *CloneAuthConfig, pullSecret *corev1.Secret, fromDigest string) *buildapi.Build { @@ -406,7 +403,7 @@ func isBuildPhaseTerminated(phase buildapi.BuildPhase) bool { return true } -func handleBuilds(ctx context.Context, buildClient BuildClient, podClient kubernetes.PodClient, build buildapi.Build) error { +func handleBuilds(ctx context.Context, buildClient BuildClient, build buildapi.Build) error { var wg sync.WaitGroup builds := constructMultiArchBuilds(build, buildClient.NodeArchitectures()) @@ -416,7 +413,7 @@ func handleBuilds(ctx context.Context, buildClient BuildClient, podClient kubern for _, build := range builds { go func(b buildapi.Build) { defer wg.Done() - if err := handleBuild(ctx, buildClient, podClient, b); err != nil { + if err := handleBuild(ctx, buildClient, b); err != nil { errChan <- fmt.Errorf("error occurred handling build %s: %w", b.Name, err) } }(build) @@ -452,7 +449,7 @@ func constructMultiArchBuilds(build buildapi.Build, nodeArchitectures []string) return ret } -func handleBuild(ctx context.Context, client BuildClient, podClient kubernetes.PodClient, build buildapi.Build) error { +func handleBuild(ctx context.Context, client BuildClient, build buildapi.Build) error { const attempts = 5 ns, name := build.Namespace, build.Name var errs []error @@ -466,7 +463,7 @@ func handleBuild(ctx context.Context, client BuildClient, podClient kubernetes.P } else { return false, fmt.Errorf("could not create build %s: %w", name, err) } - if err := waitForBuildOrTimeout(ctx, client, podClient, ns, name); err != nil { + if err := waitForBuildOrTimeout(ctx, client, ns, name); err != nil { errs = append(errs, err) return false, handleFailedBuild(ctx, client, ns, name, err) } @@ -542,45 +539,19 @@ func hintsAtInfraReason(logSnippet string) bool { strings.Contains(logSnippet, "connection reset by peer") } -func waitForBuildOrTimeout( - ctx context.Context, - buildClient BuildClient, - podClient kubernetes.PodClient, - namespace, name string, -) error { - return waitForBuild(ctx, buildClient, podClient, namespace, name) +func waitForBuildOrTimeout(ctx context.Context, buildClient BuildClient, namespace, name string) error { + return waitForBuild(ctx, buildClient, namespace, name) } -func waitForBuild( - ctx context.Context, - buildClient BuildClient, - podClient kubernetes.PodClient, - namespace, name string, -) error { +func waitForBuild(ctx context.Context, buildClient BuildClient, namespace, name string) error { logrus.WithFields(logrus.Fields{ "namespace": namespace, "name": name, }).Trace("Waiting for build to be complete.") - var ret atomic.Pointer[buildapi.Build] - var eg *errgroup.Group - eg, ctx = errgroup.WithContext(ctx) - pendingCtx, cancel := context.WithCancel(ctx) - pendingCheck := func() error { - timeout := podClient.GetPendingTimeout() - t0 := ret.Load().CreationTimestamp - select { - case <-pendingCtx.Done(): - case <-time.After(time.Until(t0.Add(timeout))): - err := util.PendingBuildError(ctx, podClient, ret.Load()) - logrus.Infof(err.Error()) - return err - } - return nil - } - eg.Go(func() error { - defer cancel() - return kubernetes.WaitForConditionOnObject(ctx, buildClient, ctrlruntimeclient.ObjectKey{Namespace: namespace, Name: name}, &buildapi.BuildList{}, &buildapi.Build{}, func(obj runtime.Object) (bool, error) { - build := obj.(*buildapi.Build) + + evaluatorFunc := func(obj runtime.Object) (bool, error) { + switch build := obj.(type) { + case *buildapi.Build: switch build.Status.Phase { case buildapi.BuildPhaseComplete: logrus.Infof("Build %s succeeded after %s", build.Name, buildDuration(build).Truncate(time.Second)) @@ -590,13 +561,13 @@ func waitForBuild( printBuildLogs(buildClient, build.Namespace, build.Name) return true, util.AppendLogToError(fmt.Errorf("the build %s failed after %s with reason %s: %s", build.Name, buildDuration(build).Truncate(time.Second), build.Status.Reason, build.Status.Message), build.Status.LogSnippet) } - if ret.Swap(build) == nil { - eg.Go(pendingCheck) - } - return false, nil - }, 0) - }) - return eg.Wait() + default: + return false, fmt.Errorf("build/%v ns/%v got an event that did not contain a build: %v", name, namespace, obj) + } + return false, nil + } + + return kubernetes.WaitForConditionOnObject(ctx, buildClient, ctrlruntimeclient.ObjectKey{Namespace: namespace, Name: name}, &buildapi.BuildList{}, &buildapi.Build{}, evaluatorFunc, 0) } func buildDuration(build *buildapi.Build) time.Duration { @@ -674,20 +645,12 @@ func (s *sourceStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func SourceStep( - config api.SourceStepConfiguration, - resources api.ResourceConfiguration, - buildClient BuildClient, - podClient kubernetes.PodClient, - jobSpec *api.JobSpec, - cloneAuthConfig *CloneAuthConfig, - pullSecret *corev1.Secret, -) api.Step { +func SourceStep(config api.SourceStepConfiguration, resources api.ResourceConfiguration, buildClient BuildClient, + jobSpec *api.JobSpec, cloneAuthConfig *CloneAuthConfig, pullSecret *corev1.Secret) api.Step { return &sourceStep{ config: config, resources: resources, client: buildClient, - podClient: podClient, jobSpec: jobSpec, cloneAuthConfig: cloneAuthConfig, pullSecret: pullSecret, diff --git a/pkg/steps/source_test.go b/pkg/steps/source_test.go index e7a149558e6..7b875064d49 100644 --- a/pkg/steps/source_test.go +++ b/pkg/steps/source_test.go @@ -22,7 +22,6 @@ import ( "github.com/openshift/ci-tools/pkg/api" "github.com/openshift/ci-tools/pkg/steps/loggingclient" "github.com/openshift/ci-tools/pkg/testhelper" - testhelper_kube "github.com/openshift/ci-tools/pkg/testhelper/kubernetes" ) func TestCreateBuild(t *testing.T) { @@ -387,112 +386,20 @@ func init() { } func TestWaitForBuild(t *testing.T) { - ns := "ns" now := meta.Time{Time: time.Now()} start, end := meta.Time{Time: now.Time.Add(-3 * time.Second)}, now var testCases = []struct { name string buildClient BuildClient - timeout time.Duration expected error }{ - { - name: "timeout", - buildClient: NewBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithRuntimeObjects( - &buildapi.Build{ - ObjectMeta: meta.ObjectMeta{ - Name: "some-build", - Namespace: ns, - CreationTimestamp: start, - Annotations: map[string]string{ - buildapi.BuildPodNameAnnotation: "some-build-build", - }, - }, - Status: buildapi.BuildStatus{ - Phase: buildapi.BuildPhasePending, - StartTimestamp: &start, - CompletionTimestamp: &end, - }, - }, - ).Build()), nil, nil), - expected: fmt.Errorf("build didn't start running within 0s (phase: Pending)"), - }, - { - name: "timeout with pod", - buildClient: NewBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithRuntimeObjects( - &buildapi.Build{ - ObjectMeta: meta.ObjectMeta{ - Name: "some-build", - Namespace: ns, - CreationTimestamp: start, - Annotations: map[string]string{ - buildapi.BuildPodNameAnnotation: "some-build-build", - }, - }, - Status: buildapi.BuildStatus{ - Phase: buildapi.BuildPhasePending, - StartTimestamp: &start, - CompletionTimestamp: &end, - }, - }, - &coreapi.Pod{ - ObjectMeta: meta.ObjectMeta{ - Name: "some-build-build", - Namespace: ns, - }, - }, - ).Build()), nil, nil), - expected: fmt.Errorf("build didn't start running within 0s (phase: Pending):\nFound 0 events for Pod some-build-build:"), - }, - { - name: "timeout with pod and events", - buildClient: NewBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithRuntimeObjects( - &buildapi.Build{ - ObjectMeta: meta.ObjectMeta{ - Name: "some-build", - Namespace: ns, - CreationTimestamp: start, - Annotations: map[string]string{ - buildapi.BuildPodNameAnnotation: "some-build-build", - }, - }, - Status: buildapi.BuildStatus{ - Phase: buildapi.BuildPhasePending, - StartTimestamp: &start, - CompletionTimestamp: &end, - }, - }, - &coreapi.Pod{ - ObjectMeta: meta.ObjectMeta{ - Name: "some-build-build", - Namespace: ns, - UID: "UID", - }, - Status: coreapi.PodStatus{ - ContainerStatuses: []coreapi.ContainerStatus{{ - Name: "the-container", - State: coreapi.ContainerState{ - Waiting: &coreapi.ContainerStateWaiting{ - Reason: "the_reason", - Message: "the_message", - }, - }, - }}, - }, - }, - ).Build()), 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:`), - }, { name: "build succeeded", buildClient: NewBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithRuntimeObjects( &buildapi.Build{ ObjectMeta: meta.ObjectMeta{ - Name: "some-build", - Namespace: ns, - CreationTimestamp: start, + Name: "some-build", + Namespace: "some-ns", }, Status: buildapi.BuildStatus{ Phase: buildapi.BuildPhaseComplete, @@ -500,16 +407,14 @@ Found 0 events for Pod some-build-build:`), CompletionTimestamp: &end, }, }).Build()), nil, nil), - timeout: 30 * time.Minute, }, { name: "build failed", buildClient: NewFakeBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithRuntimeObjects( &buildapi.Build{ ObjectMeta: meta.ObjectMeta{ - Name: "some-build", - Namespace: ns, - CreationTimestamp: start, + Name: "some-build", + Namespace: "some-ns", }, Status: buildapi.BuildStatus{ Phase: buildapi.BuildPhaseCancelled, @@ -520,70 +425,13 @@ Found 0 events for Pod some-build-build:`), CompletionTimestamp: &end, }, }).Build()), "abc\n"), // the line break is for gotestsum https://github.com/gotestyourself/gotestsum/issues/141#issuecomment-1209146526 - timeout: 30 * time.Minute, expected: fmt.Errorf("%s\n\n%s", "the build some-build failed after 3s with reason reason: msg", "snippet"), }, - { - name: "build already succeeded", - buildClient: NewBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithRuntimeObjects( - &buildapi.Build{ - ObjectMeta: meta.ObjectMeta{ - Name: "some-build", - Namespace: ns, - CreationTimestamp: meta.Time{ - Time: now.Add(-60 * time.Minute), - }, - }, - Status: buildapi.BuildStatus{ - Phase: buildapi.BuildPhaseComplete, - StartTimestamp: &meta.Time{ - Time: now.Add(-60 * time.Minute), - }, - CompletionTimestamp: &meta.Time{ - Time: now.Add(-59 * time.Minute), - }, - }, - }).Build()), nil, nil), - timeout: 30 * time.Minute, - }, - { - name: "build already failed", - buildClient: NewFakeBuildClient(loggingclient.New(fakectrlruntimeclient.NewClientBuilder().WithRuntimeObjects( - &buildapi.Build{ - ObjectMeta: meta.ObjectMeta{ - Name: "some-build", - Namespace: ns, - CreationTimestamp: meta.Time{ - Time: now.Add(-60 * time.Minute), - }, - }, - Status: buildapi.BuildStatus{ - Phase: buildapi.BuildPhaseCancelled, - Reason: "reason", - Message: "msg", - LogSnippet: "snippet", - StartTimestamp: &meta.Time{ - Time: now.Add(-60 * time.Minute), - }, - CompletionTimestamp: &meta.Time{ - Time: now.Add(-59 * time.Minute), - }, - }, - }).Build()), "abc\n"), - timeout: 30 * time.Minute, - expected: fmt.Errorf("%s\n\n%s", "the build some-build failed after 1m0s with reason reason: msg", "snippet"), - }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - client := testhelper_kube.FakePodClient{ - FakePodExecutor: &testhelper_kube.FakePodExecutor{ - LoggingClient: testCase.buildClient, - }, - PendingTimeout: testCase.timeout, - } - actual := waitForBuild(context.TODO(), testCase.buildClient, &client, ns, "some-build") + actual := waitForBuild(context.TODO(), testCase.buildClient, "some-ns", "some-build") if diff := cmp.Diff(testCase.expected, actual, testhelper.EquateErrorMessage); diff != "" { t.Errorf("%s: mismatch (-expected +actual), diff: %s", testCase.name, diff) } diff --git a/pkg/testhelper/kubernetes/pod.go b/pkg/testhelper/kubernetes/pod.go index 8ecfea70d62..263eeefc19d 100644 --- a/pkg/testhelper/kubernetes/pod.go +++ b/pkg/testhelper/kubernetes/pod.go @@ -95,12 +95,9 @@ func filter(list ctrlruntimeclient.ObjectList, opts ...ctrlruntimeclient.ListOpt type FakePodClient struct { *FakePodExecutor Namespace, Name string - PendingTimeout time.Duration } -func (f FakePodClient) GetPendingTimeout() time.Duration { - return f.PendingTimeout -} +func (FakePodClient) PendingTimeout() time.Duration { return 30 * time.Minute } func (f *FakePodClient) Exec(namespace, name string, opts *coreapi.PodExecOptions) (remotecommand.Executor, error) { if namespace != f.Namespace { diff --git a/pkg/util/builds.go b/pkg/util/builds.go deleted file mode 100644 index 8bddb13967f..00000000000 --- a/pkg/util/builds.go +++ /dev/null @@ -1,32 +0,0 @@ -package util - -import ( - "context" - "errors" - "fmt" - - "github.com/sirupsen/logrus" - - corev1 "k8s.io/api/core/v1" - ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" - - buildapi "github.com/openshift/api/build/v1" - - "github.com/openshift/ci-tools/pkg/kubernetes" -) - -// PendingBuildError fetches scheduling errors from the build pod's events -func PendingBuildError(ctx context.Context, client kubernetes.PodClient, build *buildapi.Build) (ret error) { - msg := fmt.Sprintf("build didn't start running within %s (phase: %s)", client.GetPendingTimeout(), build.Status.Phase) - var pod corev1.Pod - if name, ok := build.Annotations[buildapi.BuildPodNameAnnotation]; !ok { - logrus.Warnf("pod annotation missing for build %q", build.Name) - ret = errors.New(msg) - } else if err := client.Get(ctx, ctrlruntimeclient.ObjectKey{Namespace: build.Namespace, Name: name}, &pod); err != nil { - logrus.Warnf("failed to get pod for build %q: %v", build.Name, err) - ret = errors.New(msg) - } else { - ret = fmt.Errorf("%s:%s\n%s", msg, getReasonsForUnreadyContainers(&pod), getEventsForPod(ctx, &pod, client)) - } - return -} diff --git a/pkg/util/pods.go b/pkg/util/pods.go index 73e74d3f278..5d0f0df7a02 100644 --- a/pkg/util/pods.go +++ b/pkg/util/pods.go @@ -143,7 +143,7 @@ func waitForPodCompletionOrTimeout(ctx context.Context, podClient kubernetes.Pod eg, ctx = errgroup.WithContext(ctx) pendingCtx, cancel := context.WithCancel(ctx) pendingCheck := func() error { - timeout := podClient.GetPendingTimeout() + timeout := podClient.PendingTimeout() if pod, err := checkPendingPeriodic(pendingCtx.Done(), timeout, &ret); err != nil { err = fmt.Errorf("pod pending for more than %s: %w: %s\n%s", timeout, err, getReasonsForUnreadyContainers(pod), getEventsForPod(ctx, pod, podClient)) logrus.Info(err)