diff --git a/pkg/build/apis/build/types.go b/pkg/build/apis/build/types.go index 3a92c2b262d1..279d55286035 100644 --- a/pkg/build/apis/build/types.go +++ b/pkg/build/apis/build/types.go @@ -514,6 +514,9 @@ const ( // range of build failures. StatusReasonGenericBuildFailed StatusReason = "GenericBuildFailed" + // StatusReasonOutOfMemoryKilled indicates that the build pod was killed for its memory consumption + StatusReasonOutOfMemoryKilled StatusReason = "OutOfMemoryKilled" + // StatusCannotRetrieveServiceAccount is the reason associated with a failure // to look up the service account associated with the BuildConfig. StatusReasonCannotRetrieveServiceAccount StatusReason = "CannotRetrieveServiceAccount" @@ -540,6 +543,7 @@ const ( StatusMessageNoBuildContainerStatus = "The pod for this build has no container statuses indicating success or failure." StatusMessageFailedContainer = "The pod for this build has at least one container with a non-zero exit status." StatusMessageGenericBuildFailed = "Generic Build failure - check logs for details." + StatusMessageOutOfMemoryKilled = "The build pod was killed due to an out of memory condition." StatusMessageUnresolvableEnvironmentVariable = "Unable to resolve build environment variable reference." StatusMessageCannotRetrieveServiceAccount = "Unable to look up the service account secrets for this build." ) diff --git a/pkg/build/controller/build/build_controller.go b/pkg/build/controller/build/build_controller.go index 66520be4dce6..65037820b5d4 100644 --- a/pkg/build/controller/build/build_controller.go +++ b/pkg/build/controller/build/build_controller.go @@ -1001,7 +1001,9 @@ func (bc *BuildController) handleActiveBuild(build *buildapi.Build, pod *v1.Pod) } } case v1.PodFailed: - if build.Status.Phase != buildapi.BuildPhaseFailed { + if isOOMKilled(pod) { + update = transitionToPhase(buildapi.BuildPhaseFailed, buildapi.StatusReasonOutOfMemoryKilled, buildapi.StatusMessageOutOfMemoryKilled) + } else if build.Status.Phase != buildapi.BuildPhaseFailed { // If a DeletionTimestamp has been set, it means that the pod will // soon be deleted. The build should be transitioned to the Error phase. if pod.DeletionTimestamp != nil { @@ -1014,11 +1016,33 @@ func (bc *BuildController) handleActiveBuild(build *buildapi.Build, pod *v1.Pod) return update, nil } +func isOOMKilled(pod *v1.Pod) bool { + if pod.Status.Reason == "OOMKilled" { + return true + } + for _, c := range pod.Status.InitContainerStatuses { + terminated := c.State.Terminated + if terminated != nil && terminated.Reason == "OOMKilled" { + return true + } + } + for _, c := range pod.Status.ContainerStatuses { + terminated := c.State.Terminated + if terminated != nil && terminated.Reason == "OOMKilled" { + return true + } + } + return false +} + // handleCompletedBuild will only be called on builds that are already in a terminal phase. It is used to setup the // completion timestamp and failure logsnippet as needed. func (bc *BuildController) handleCompletedBuild(build *buildapi.Build, pod *v1.Pod) (*buildUpdate, error) { update := &buildUpdate{} + if isOOMKilled(pod) { + update = transitionToPhase(buildapi.BuildPhaseFailed, buildapi.StatusReasonOutOfMemoryKilled, buildapi.StatusMessageOutOfMemoryKilled) + } setBuildCompletionData(build, pod, update) return update, nil diff --git a/pkg/build/registry/build/strategy.go b/pkg/build/registry/build/strategy.go index 8610e2414231..e2381004e5a2 100644 --- a/pkg/build/registry/build/strategy.go +++ b/pkg/build/registry/build/strategy.go @@ -53,7 +53,11 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { // of the reason and message. This is to prevent the build controller from // overwriting the reason and message that was set by the builder pod // when it updated the build's details. - if oldBuild.Status.Phase == buildapi.BuildPhaseFailed { + // Only allow OOMKilled override because various processes in a container + // can get OOMKilled and this confuses builder to prematurely populate + // failure reason + if oldBuild.Status.Phase == buildapi.BuildPhaseFailed && + newBuild.Status.Reason != buildapi.StatusReasonOutOfMemoryKilled { newBuild.Status.Reason = oldBuild.Status.Reason newBuild.Status.Message = oldBuild.Status.Message } diff --git a/test/extended/builds/failure_status.go b/test/extended/builds/failure_status.go index 68b118cdd5dd..585768e0a910 100644 --- a/test/extended/builds/failure_status.go +++ b/test/extended/builds/failure_status.go @@ -27,6 +27,7 @@ var _ = g.Describe("[Feature:Builds][Slow] update failure status", func() { fetchDockerSrc = exutil.FixturePath("testdata", "builds", "statusfail-fetchsourcedocker.yaml") fetchS2ISrc = exutil.FixturePath("testdata", "builds", "statusfail-fetchsources2i.yaml") badContextDirS2ISrc = exutil.FixturePath("testdata", "builds", "statusfail-badcontextdirs2i.yaml") + oomkilled = exutil.FixturePath("testdata", "builds", "statusfail-oomkilled.yaml") builderImageFixture = exutil.FixturePath("testdata", "builds", "statusfail-fetchbuilderimage.yaml") pushToRegistryFixture = exutil.FixturePath("testdata", "builds", "statusfail-pushtoregistry.yaml") failedAssembleFixture = exutil.FixturePath("testdata", "builds", "statusfail-failedassemble.yaml") @@ -130,6 +131,32 @@ var _ = g.Describe("[Feature:Builds][Slow] update failure status", func() { }) }) + g.Describe("Build status OutOfMemoryKilled", func() { + g.It("should contain OutOfMemoryKilled failure reason and message", func() { + err := oc.Run("create").Args("-f", oomkilled).Execute() + o.Expect(err).NotTo(o.HaveOccurred()) + + br, err := exutil.StartBuildAndWait(oc, "statusfail-oomkilled", "--build-loglevel=5") + o.Expect(err).NotTo(o.HaveOccurred()) + br.AssertFailure() + br.DumpLogs() + + var build *buildapi.Build + wait.PollImmediate(200*time.Millisecond, 30*time.Second, func() (bool, error) { + build, err = oc.BuildClient().Build().Builds(oc.Namespace()).Get(br.Build.Name, metav1.GetOptions{}) + if build.Status.Reason != buildapi.StatusReasonOutOfMemoryKilled { + return false, nil + } + return true, nil + }) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(build.Status.Reason).To(o.Equal(buildapi.StatusReasonOutOfMemoryKilled)) + o.Expect(build.Status.Message).To(o.Equal(buildapi.StatusMessageOutOfMemoryKilled)) + + exutil.CheckForBuildEvent(oc.KubeClient().Core(), br.Build, buildapi.BuildFailedEventReason, buildapi.BuildFailedEventMessage) + }) + }) + g.Describe("Build status S2I bad context dir failure", func() { g.It("should contain the S2I bad context dir failure reason and message", func() { err := oc.Run("create").Args("-f", badContextDirS2ISrc).Execute() diff --git a/test/extended/testdata/bindata.go b/test/extended/testdata/bindata.go index 565b03bdee70..dd9b96ced4ab 100644 --- a/test/extended/testdata/bindata.go +++ b/test/extended/testdata/bindata.go @@ -46,6 +46,7 @@ // test/extended/testdata/builds/statusfail-fetchsourcedocker.yaml // test/extended/testdata/builds/statusfail-fetchsources2i.yaml // test/extended/testdata/builds/statusfail-genericreason.yaml +// test/extended/testdata/builds/statusfail-oomkilled.yaml // test/extended/testdata/builds/statusfail-postcommithook.yaml // test/extended/testdata/builds/statusfail-pushtoregistry.yaml // test/extended/testdata/builds/sti-environment-build-app/.sti/environment @@ -1973,6 +1974,40 @@ func testExtendedTestdataBuildsStatusfailGenericreasonYaml() (*asset, error) { return a, nil } +var _testExtendedTestdataBuildsStatusfailOomkilledYaml = []byte(`kind: BuildConfig +apiVersion: v1 +metadata: + name: statusfail-oomkilled +spec: + resources: + limits: + memory: 10Mi + source: + git: + uri: "https://github.com/openshift/ruby-hello-world" + strategy: + type: Source + sourceStrategy: + from: + kind: DockerImage + name: centos/ruby-23-centos7:latest +`) + +func testExtendedTestdataBuildsStatusfailOomkilledYamlBytes() ([]byte, error) { + return _testExtendedTestdataBuildsStatusfailOomkilledYaml, nil +} + +func testExtendedTestdataBuildsStatusfailOomkilledYaml() (*asset, error) { + bytes, err := testExtendedTestdataBuildsStatusfailOomkilledYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "test/extended/testdata/builds/statusfail-oomkilled.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + var _testExtendedTestdataBuildsStatusfailPostcommithookYaml = []byte(`kind: BuildConfig apiVersion: v1 metadata: @@ -33607,6 +33642,7 @@ var _bindata = map[string]func() (*asset, error){ "test/extended/testdata/builds/statusfail-fetchsourcedocker.yaml": testExtendedTestdataBuildsStatusfailFetchsourcedockerYaml, "test/extended/testdata/builds/statusfail-fetchsources2i.yaml": testExtendedTestdataBuildsStatusfailFetchsources2iYaml, "test/extended/testdata/builds/statusfail-genericreason.yaml": testExtendedTestdataBuildsStatusfailGenericreasonYaml, + "test/extended/testdata/builds/statusfail-oomkilled.yaml": testExtendedTestdataBuildsStatusfailOomkilledYaml, "test/extended/testdata/builds/statusfail-postcommithook.yaml": testExtendedTestdataBuildsStatusfailPostcommithookYaml, "test/extended/testdata/builds/statusfail-pushtoregistry.yaml": testExtendedTestdataBuildsStatusfailPushtoregistryYaml, "test/extended/testdata/builds/sti-environment-build-app/.sti/environment": testExtendedTestdataBuildsStiEnvironmentBuildAppStiEnvironment, @@ -34076,6 +34112,7 @@ var _bintree = &bintree{nil, map[string]*bintree{ "statusfail-fetchsourcedocker.yaml": &bintree{testExtendedTestdataBuildsStatusfailFetchsourcedockerYaml, map[string]*bintree{}}, "statusfail-fetchsources2i.yaml": &bintree{testExtendedTestdataBuildsStatusfailFetchsources2iYaml, map[string]*bintree{}}, "statusfail-genericreason.yaml": &bintree{testExtendedTestdataBuildsStatusfailGenericreasonYaml, map[string]*bintree{}}, + "statusfail-oomkilled.yaml": &bintree{testExtendedTestdataBuildsStatusfailOomkilledYaml, map[string]*bintree{}}, "statusfail-postcommithook.yaml": &bintree{testExtendedTestdataBuildsStatusfailPostcommithookYaml, map[string]*bintree{}}, "statusfail-pushtoregistry.yaml": &bintree{testExtendedTestdataBuildsStatusfailPushtoregistryYaml, map[string]*bintree{}}, "sti-environment-build-app": &bintree{nil, map[string]*bintree{ diff --git a/test/extended/testdata/builds/statusfail-oomkilled.yaml b/test/extended/testdata/builds/statusfail-oomkilled.yaml new file mode 100644 index 000000000000..979956675b69 --- /dev/null +++ b/test/extended/testdata/builds/statusfail-oomkilled.yaml @@ -0,0 +1,17 @@ +kind: BuildConfig +apiVersion: v1 +metadata: + name: statusfail-oomkilled +spec: + resources: + limits: + memory: 10Mi + source: + git: + uri: "https://github.com/openshift/ruby-hello-world" + strategy: + type: Source + sourceStrategy: + from: + kind: DockerImage + name: centos/ruby-23-centos7:latest