Skip to content

Commit

Permalink
Merge pull request #17146 from coreydaley/github_16557_buildconfigins…
Browse files Browse the repository at this point in the history
…tantiatefailed_warning

Automatic merge from submit-queue.

Fix BuildConfigInstantiateFailed warning when lastVersion == 0

Build config instatiate should not emit a warning when
bc.Status.LastVersion != lastVersion and lastVersion == 0
Fixes #16557
  • Loading branch information
openshift-merge-robot authored Feb 20, 2018
2 parents b547bd2 + 8f39e27 commit 4e52d0a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 2 deletions.
7 changes: 6 additions & 1 deletion pkg/build/controller/buildconfig/buildconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package controller

import (
"fmt"
"strings"
"time"

"github.com/golang/glog"
Expand Down Expand Up @@ -136,7 +137,11 @@ func (c *BuildConfigController) handleBuildConfig(bc *buildapi.BuildConfig) erro
} else if buildgenerator.IsFatal(err) || kerrors.IsNotFound(err) || kerrors.IsBadRequest(err) || kerrors.IsForbidden(err) {
instantiateErr = fmt.Errorf("gave up on Build for BuildConfig %s due to fatal error: %v", bcDesc(bc), err)
utilruntime.HandleError(instantiateErr)
c.recorder.Event(bc, kapi.EventTypeWarning, "BuildConfigInstantiateFailed", instantiateErr.Error())
// Fixes https://github.com/openshift/origin/issues/16557
// Caused by a race condition between the ImageChangeTrigger and BuildConfigChangeTrigger
if !strings.Contains(instantiateErr.Error(), "does not match the build request LastVersion(0)") {
c.recorder.Event(bc, kapi.EventTypeWarning, "BuildConfigInstantiateFailed", instantiateErr.Error())
}
return &configControllerFatalError{err.Error()}
} else {
instantiateErr = fmt.Errorf("error instantiating Build from BuildConfig %s: %v", bcDesc(bc), err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func (g *BuildGenerator) instantiate(ctx apirequest.Context, request *buildapi.B
return g.createBuild(ctx, newBuild)
}

// checkBuildConfigLastVersion will return an error if the BuildConfig's LastVersion doesn't match the passed in lastVersion
// checkLastVersion will return an error if the BuildConfig's LastVersion doesn't match the passed in lastVersion
// when lastVersion is not nil
func (g *BuildGenerator) checkLastVersion(bc *buildapi.BuildConfig, lastVersion *int64) error {
if lastVersion != nil && bc.Status.LastVersion != *lastVersion {
Expand Down
6 changes: 6 additions & 0 deletions test/cmd/newapp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ os::cmd::expect_success 'oc delete all -l app=expose-output'
os::cmd::expect_success_and_text 'oc new-app mysql --as-test' 'Application is not exposed'
os::cmd::expect_success 'oc delete all -l app=mysql'

# ensure that oc new-app does not emit a BuildConfigInstantiateFailed event when creating
# a new application
os::cmd::expect_success 'oc new-app https://github.com/sclorg/ruby-ex'
os::cmd::expect_success_and_not_text 'oc describe bc/ruby-ex' 'BuildConfigInstantiateFailed'
os::cmd::expect_success 'oc delete all -l app=ruby-ex'

# test that imagestream references across imagestreams do not cause an error
os::cmd::try_until_success 'oc get imagestreamtags ruby:2.3'
os::cmd::expect_success 'oc create -f test/testdata/newapp/imagestream-ref.yaml'
Expand Down

0 comments on commit 4e52d0a

Please sign in to comment.