diff --git a/cmd/ci-operator/main_test.go b/cmd/ci-operator/main_test.go index 2541e01ba56..ab88fd6bcf7 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, &api.JobSpec{}, nil, nil), + steps.SourceStep(api.SourceStepConfiguration{From: api.PipelineImageStreamTagReferenceRoot, To: api.PipelineImageStreamTagReferenceSource}, api.ResourceConfiguration{}, nil, 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, + &api.ReleaseBuildConfiguration{}, api.ResourceConfiguration{}, nil, 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 ec8e482ddc6..eec19c03250 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, jobSpec, pullSecret) + step = steps.PipelineImageCacheStep(*rawStep.PipelineImageCacheStepConfiguration, config.Resources, buildClient, podClient, jobSpec, pullSecret) } else if rawStep.SourceStepConfiguration != nil { - step = steps.SourceStep(*rawStep.SourceStepConfiguration, config.Resources, buildClient, jobSpec, cloneAuthConfig, pullSecret) + step = steps.SourceStep(*rawStep.SourceStepConfiguration, config.Resources, buildClient, podClient, jobSpec, cloneAuthConfig, pullSecret) } else if rawStep.BundleSourceStepConfiguration != nil { - step = steps.BundleSourceStep(*rawStep.BundleSourceStepConfiguration, config, config.Resources, buildClient, jobSpec, pullSecret) + step = steps.BundleSourceStep(*rawStep.BundleSourceStepConfiguration, config, config.Resources, buildClient, podClient, jobSpec, pullSecret) } else if rawStep.IndexGeneratorStepConfiguration != nil { - step = steps.IndexGeneratorStep(*rawStep.IndexGeneratorStepConfiguration, config, config.Resources, buildClient, jobSpec, pullSecret) + step = steps.IndexGeneratorStep(*rawStep.IndexGeneratorStepConfiguration, config, config.Resources, buildClient, podClient, jobSpec, pullSecret) } else if rawStep.ProjectDirectoryImageBuildStepConfiguration != nil { - step = steps.ProjectDirectoryImageBuildStep(*rawStep.ProjectDirectoryImageBuildStepConfiguration, config, config.Resources, buildClient, jobSpec, pullSecret) + step = steps.ProjectDirectoryImageBuildStep(*rawStep.ProjectDirectoryImageBuildStepConfiguration, config, config.Resources, buildClient, podClient, jobSpec, pullSecret) } else if rawStep.ProjectDirectoryImageBuildInputs != nil { - step = steps.GitSourceStep(*rawStep.ProjectDirectoryImageBuildInputs, config.Resources, buildClient, jobSpec, cloneAuthConfig, pullSecret) + step = steps.GitSourceStep(*rawStep.ProjectDirectoryImageBuildInputs, config.Resources, buildClient, podClient, jobSpec, cloneAuthConfig, pullSecret) } else if rawStep.RPMImageInjectionStepConfiguration != nil { - step = steps.RPMImageInjectionStep(*rawStep.RPMImageInjectionStepConfiguration, config.Resources, buildClient, jobSpec, pullSecret) + step = steps.RPMImageInjectionStep(*rawStep.RPMImageInjectionStepConfiguration, config.Resources, buildClient, podClient, 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 fdf449987cb..bbdb73d4870 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 - PendingTimeout() time.Duration + GetPendingTimeout() 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) PendingTimeout() time.Duration { return c.pendingTimeout } +func (c podClient) GetPendingTimeout() 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 b20816f17ce..4591a2f21e3 100644 --- a/pkg/steps/bundle_source.go +++ b/pkg/steps/bundle_source.go @@ -13,6 +13,7 @@ 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" ) @@ -22,6 +23,7 @@ type bundleSourceStep struct { releaseBuildConfig *api.ReleaseBuildConfiguration resources api.ResourceConfiguration client BuildClient + podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -75,7 +77,7 @@ func (s *bundleSourceStep) run(ctx context.Context) error { s.pullSecret, nil, ) - return handleBuilds(ctx, s.client, *build) + return handleBuilds(ctx, s.client, s.podClient, *build) } func replaceCommand(pullSpec, with string) string { @@ -129,12 +131,21 @@ 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, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { +func BundleSourceStep( + config api.BundleSourceStepConfiguration, + releaseBuildConfig *api.ReleaseBuildConfiguration, + resources api.ResourceConfiguration, + client BuildClient, + podClient kubernetes.PodClient, + 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 1a70204709f..6198f701a58 100644 --- a/pkg/steps/git_source.go +++ b/pkg/steps/git_source.go @@ -11,6 +11,7 @@ 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" ) @@ -18,6 +19,7 @@ type gitSourceStep struct { config api.ProjectDirectoryImageBuildInputs resources api.ResourceConfiguration buildClient BuildClient + podClient kubernetes.PodClient jobSpec *api.JobSpec cloneAuthConfig *CloneAuthConfig pullSecret *coreapi.Secret @@ -42,7 +44,7 @@ func (s *gitSourceStep) run(ctx context.Context) error { secretName = s.cloneAuthConfig.Secret.Name } - return handleBuilds(ctx, s.buildClient, *buildFromSource(s.jobSpec, "", api.PipelineImageStreamTagReferenceRoot, buildapi.BuildSource{ + return handleBuilds(ctx, s.buildClient, s.podClient, *buildFromSource(s.jobSpec, "", api.PipelineImageStreamTagReferenceRoot, buildapi.BuildSource{ Type: buildapi.BuildSourceGit, Dockerfile: s.config.DockerfileLiteral, ContextDir: s.config.ContextDir, @@ -99,11 +101,20 @@ 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, jobSpec *api.JobSpec, cloneAuthConfig *CloneAuthConfig, pullSecret *coreapi.Secret) api.Step { +func GitSourceStep( + config api.ProjectDirectoryImageBuildInputs, + resources api.ResourceConfiguration, + buildClient BuildClient, + podClient kubernetes.PodClient, + 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 803c85e8702..d8ea1632d59 100644 --- a/pkg/steps/index_generator.go +++ b/pkg/steps/index_generator.go @@ -16,6 +16,7 @@ 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" ) @@ -25,6 +26,7 @@ type indexGeneratorStep struct { releaseBuildConfig *api.ReleaseBuildConfiguration resources api.ResourceConfiguration client BuildClient + podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -121,7 +123,7 @@ func (s *indexGeneratorStep) run(ctx context.Context) error { s.pullSecret, nil, ) - err = handleBuilds(ctx, s.client, *build) + err = handleBuilds(ctx, s.client, s.podClient, *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) } @@ -195,12 +197,21 @@ func (s *indexGeneratorStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func IndexGeneratorStep(config api.IndexGeneratorStepConfiguration, releaseBuildConfig *api.ReleaseBuildConfiguration, resources api.ResourceConfiguration, buildClient BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { +func IndexGeneratorStep( + config api.IndexGeneratorStepConfiguration, + releaseBuildConfig *api.ReleaseBuildConfiguration, + resources api.ResourceConfiguration, + buildClient BuildClient, + podClient kubernetes.PodClient, + 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 866f7aed83d..cefd493e554 100644 --- a/pkg/steps/multi_stage/run_test.go +++ b/pkg/steps/multi_stage/run_test.go @@ -88,7 +88,10 @@ func TestRun(t *testing.T) { }, } jobSpec.SetNamespace("ns") - client := &testhelper_kube.FakePodClient{FakePodExecutor: crclient} + client := &testhelper_kube.FakePodClient{ + PendingTimeout: 30 * time.Minute, + 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 cb4598c8b8b..5b8f399f89f 100644 --- a/pkg/steps/pipeline_image_cache.go +++ b/pkg/steps/pipeline_image_cache.go @@ -11,6 +11,7 @@ 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" ) @@ -24,6 +25,7 @@ type pipelineImageCacheStep struct { config api.PipelineImageCacheStepConfiguration resources api.ResourceConfiguration client BuildClient + podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -44,7 +46,7 @@ func (s *pipelineImageCacheStep) run(ctx context.Context) error { if err != nil { return err } - return handleBuild(ctx, s.client, *buildFromSource( + return handleBuild(ctx, s.client, s.podClient, *buildFromSource( s.jobSpec, s.config.From, s.config.To, buildapi.BuildSource{ Type: buildapi.BuildSourceDockerfile, @@ -85,11 +87,19 @@ func (s *pipelineImageCacheStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func PipelineImageCacheStep(config api.PipelineImageCacheStepConfiguration, resources api.ResourceConfiguration, client BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { +func PipelineImageCacheStep( + config api.PipelineImageCacheStepConfiguration, + resources api.ResourceConfiguration, + client BuildClient, + podClient kubernetes.PodClient, + 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 fe8c8a80b3d..9e30df26b41 100644 --- a/pkg/steps/project_image.go +++ b/pkg/steps/project_image.go @@ -14,6 +14,7 @@ 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" ) @@ -23,6 +24,7 @@ type projectDirectoryImageBuildStep struct { releaseBuildConfig *api.ReleaseBuildConfiguration resources api.ResourceConfiguration client BuildClient + podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -61,7 +63,7 @@ func (s *projectDirectoryImageBuildStep) run(ctx context.Context) error { s.pullSecret, s.config.BuildArgs, ) - return handleBuilds(ctx, s.client, *build) + return handleBuilds(ctx, s.client, s.podClient, *build) } type workingDir func(tag string) (string, error) @@ -161,12 +163,21 @@ func (s *projectDirectoryImageBuildStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func ProjectDirectoryImageBuildStep(config api.ProjectDirectoryImageBuildStepConfiguration, releaseBuildConfig *api.ReleaseBuildConfiguration, resources api.ResourceConfiguration, buildClient BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { +func ProjectDirectoryImageBuildStep( + config api.ProjectDirectoryImageBuildStepConfiguration, + releaseBuildConfig *api.ReleaseBuildConfiguration, + resources api.ResourceConfiguration, + buildClient BuildClient, + podClient kubernetes.PodClient, + 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 7777933293c..cbaec00b772 100644 --- a/pkg/steps/rpm_injection.go +++ b/pkg/steps/rpm_injection.go @@ -11,6 +11,7 @@ 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" ) @@ -23,6 +24,7 @@ type rpmImageInjectionStep struct { config api.RPMImageInjectionStepConfiguration resources api.ResourceConfiguration client BuildClient + podClient kubernetes.PodClient jobSpec *api.JobSpec pullSecret *coreapi.Secret } @@ -48,7 +50,7 @@ func (s *rpmImageInjectionStep) run(ctx context.Context) error { if err != nil { return err } - return handleBuilds(ctx, s.client, *buildFromSource( + return handleBuilds(ctx, s.client, s.podClient, *buildFromSource( s.jobSpec, s.config.From, s.config.To, buildapi.BuildSource{ Type: buildapi.BuildSourceDockerfile, @@ -84,11 +86,19 @@ func (s *rpmImageInjectionStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func RPMImageInjectionStep(config api.RPMImageInjectionStepConfiguration, resources api.ResourceConfiguration, buildClient BuildClient, jobSpec *api.JobSpec, pullSecret *coreapi.Secret) api.Step { +func RPMImageInjectionStep( + config api.RPMImageInjectionStepConfiguration, + resources api.ResourceConfiguration, + buildClient BuildClient, + podClient kubernetes.PodClient, + 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 d524f15d71d..f6cdfec90db 100644 --- a/pkg/steps/source.go +++ b/pkg/steps/source.go @@ -8,9 +8,11 @@ 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" @@ -135,6 +137,7 @@ type sourceStep struct { config api.SourceStepConfiguration resources api.ResourceConfiguration client BuildClient + podClient kubernetes.PodClient jobSpec *api.JobSpec cloneAuthConfig *CloneAuthConfig pullSecret *corev1.Secret @@ -160,7 +163,7 @@ func (s *sourceStep) run(ctx context.Context) error { if err != nil { return err } - return handleBuilds(ctx, s.client, *createBuild(s.config, s.jobSpec, clonerefsRef, s.resources, s.cloneAuthConfig, s.pullSecret, fromDigest)) + return handleBuilds(ctx, s.client, s.podClient, *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 { @@ -403,7 +406,7 @@ func isBuildPhaseTerminated(phase buildapi.BuildPhase) bool { return true } -func handleBuilds(ctx context.Context, buildClient BuildClient, build buildapi.Build) error { +func handleBuilds(ctx context.Context, buildClient BuildClient, podClient kubernetes.PodClient, build buildapi.Build) error { var wg sync.WaitGroup builds := constructMultiArchBuilds(build, buildClient.NodeArchitectures()) @@ -413,7 +416,7 @@ func handleBuilds(ctx context.Context, buildClient BuildClient, build buildapi.B for _, build := range builds { go func(b buildapi.Build) { defer wg.Done() - if err := handleBuild(ctx, buildClient, b); err != nil { + if err := handleBuild(ctx, buildClient, podClient, b); err != nil { errChan <- fmt.Errorf("error occurred handling build %s: %w", b.Name, err) } }(build) @@ -449,7 +452,7 @@ func constructMultiArchBuilds(build buildapi.Build, nodeArchitectures []string) return ret } -func handleBuild(ctx context.Context, client BuildClient, build buildapi.Build) error { +func handleBuild(ctx context.Context, client BuildClient, podClient kubernetes.PodClient, build buildapi.Build) error { const attempts = 5 ns, name := build.Namespace, build.Name var errs []error @@ -463,7 +466,7 @@ func handleBuild(ctx context.Context, client BuildClient, build buildapi.Build) } else { return false, fmt.Errorf("could not create build %s: %w", name, err) } - if err := waitForBuildOrTimeout(ctx, client, ns, name); err != nil { + if err := waitForBuildOrTimeout(ctx, client, podClient, ns, name); err != nil { errs = append(errs, err) return false, handleFailedBuild(ctx, client, ns, name, err) } @@ -539,19 +542,45 @@ func hintsAtInfraReason(logSnippet string) bool { strings.Contains(logSnippet, "connection reset by peer") } -func waitForBuildOrTimeout(ctx context.Context, buildClient BuildClient, namespace, name string) error { - return waitForBuild(ctx, buildClient, namespace, name) +func waitForBuildOrTimeout( + ctx context.Context, + buildClient BuildClient, + podClient kubernetes.PodClient, + namespace, name string, +) error { + return waitForBuild(ctx, buildClient, podClient, namespace, name) } -func waitForBuild(ctx context.Context, buildClient BuildClient, namespace, name string) error { +func waitForBuild( + ctx context.Context, + buildClient BuildClient, + podClient kubernetes.PodClient, + namespace, name string, +) error { logrus.WithFields(logrus.Fields{ "namespace": namespace, "name": name, }).Trace("Waiting for build to be complete.") - - evaluatorFunc := func(obj runtime.Object) (bool, error) { - switch build := obj.(type) { - case *buildapi.Build: + 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) switch build.Status.Phase { case buildapi.BuildPhaseComplete: logrus.Infof("Build %s succeeded after %s", build.Name, buildDuration(build).Truncate(time.Second)) @@ -561,13 +590,13 @@ func waitForBuild(ctx context.Context, buildClient BuildClient, namespace, name 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) } - 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) + if ret.Swap(build) == nil { + eg.Go(pendingCheck) + } + return false, nil + }, 0) + }) + return eg.Wait() } func buildDuration(build *buildapi.Build) time.Duration { @@ -645,12 +674,20 @@ func (s *sourceStep) Objects() []ctrlruntimeclient.Object { return s.client.Objects() } -func SourceStep(config api.SourceStepConfiguration, resources api.ResourceConfiguration, buildClient BuildClient, - jobSpec *api.JobSpec, cloneAuthConfig *CloneAuthConfig, pullSecret *corev1.Secret) api.Step { +func SourceStep( + config api.SourceStepConfiguration, + resources api.ResourceConfiguration, + buildClient BuildClient, + podClient kubernetes.PodClient, + 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 7b875064d49..e7a149558e6 100644 --- a/pkg/steps/source_test.go +++ b/pkg/steps/source_test.go @@ -22,6 +22,7 @@ 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) { @@ -386,20 +387,112 @@ 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: "some-ns", + Name: "some-build", + Namespace: ns, + CreationTimestamp: start, }, Status: buildapi.BuildStatus{ Phase: buildapi.BuildPhaseComplete, @@ -407,14 +500,16 @@ func TestWaitForBuild(t *testing.T) { 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: "some-ns", + Name: "some-build", + Namespace: ns, + CreationTimestamp: start, }, Status: buildapi.BuildStatus{ Phase: buildapi.BuildPhaseCancelled, @@ -425,13 +520,70 @@ func TestWaitForBuild(t *testing.T) { 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) { - actual := waitForBuild(context.TODO(), testCase.buildClient, "some-ns", "some-build") + client := testhelper_kube.FakePodClient{ + FakePodExecutor: &testhelper_kube.FakePodExecutor{ + LoggingClient: testCase.buildClient, + }, + PendingTimeout: testCase.timeout, + } + actual := waitForBuild(context.TODO(), testCase.buildClient, &client, 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 263eeefc19d..8ecfea70d62 100644 --- a/pkg/testhelper/kubernetes/pod.go +++ b/pkg/testhelper/kubernetes/pod.go @@ -95,9 +95,12 @@ func filter(list ctrlruntimeclient.ObjectList, opts ...ctrlruntimeclient.ListOpt type FakePodClient struct { *FakePodExecutor Namespace, Name string + PendingTimeout time.Duration } -func (FakePodClient) PendingTimeout() time.Duration { return 30 * time.Minute } +func (f FakePodClient) GetPendingTimeout() time.Duration { + return f.PendingTimeout +} 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 new file mode 100644 index 00000000000..8bddb13967f --- /dev/null +++ b/pkg/util/builds.go @@ -0,0 +1,32 @@ +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 5d0f0df7a02..73e74d3f278 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.PendingTimeout() + timeout := podClient.GetPendingTimeout() 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)