Browse code

Improve BuildConfig error reporting

When BuildConfig cannot be instantiated because the source ImageStream
is missing:
* emit event with the error
* show events in the output of "oc describe bc foobar"
* make the /instantiate REST endpoint return unprocessable entity
response that includes the ImageStream namespace in the error message

Martin Milata authored on 2016/09/10 00:34:06
Showing 6 changed files
... ...
@@ -83,6 +83,7 @@ func (c *BuildConfigController) HandleBuildConfig(bc *buildapi.BuildConfig) erro
83 83
 			return &ConfigControllerFatalError{err.Error()}
84 84
 		} else {
85 85
 			instantiateErr = fmt.Errorf("error instantiating Build from BuildConfig %s/%s: %v", bc.Namespace, bc.Name, err)
86
+			c.Recorder.Event(bc, kapi.EventTypeWarning, "BuildConfigInstantiateFailed", instantiateErr.Error())
86 87
 			utilruntime.HandleError(instantiateErr)
87 88
 		}
88 89
 		return instantiateErr
... ...
@@ -4,6 +4,8 @@ import (
4 4
 	"fmt"
5 5
 	"testing"
6 6
 
7
+	"k8s.io/kubernetes/pkg/client/record"
8
+
7 9
 	buildapi "github.com/openshift/origin/pkg/build/api"
8 10
 )
9 11
 
... ...
@@ -44,6 +46,7 @@ func TestHandleBuildConfig(t *testing.T) {
44 44
 		}
45 45
 		controller := &BuildConfigController{
46 46
 			BuildConfigInstantiator: instantiator,
47
+			Recorder:                &record.FakeRecorder{},
47 48
 		}
48 49
 		err := controller.HandleBuildConfig(tc.bc)
49 50
 		if err != nil {
... ...
@@ -227,6 +227,9 @@ func (g *BuildGenerator) Instantiate(ctx kapi.Context, request *buildapi.BuildRe
227 227
 	}
228 228
 
229 229
 	if err := g.updateImageTriggers(ctx, bc, request.From, request.TriggeredByImage); err != nil {
230
+		if _, ok := err.(errors.APIStatus); ok {
231
+			return nil, err
232
+		}
230 233
 		return nil, errors.NewInternalError(err)
231 234
 	}
232 235
 
... ...
@@ -573,10 +576,8 @@ func (g *BuildGenerator) resolveImageStreamReference(ctx kapi.Context, from kapi
573 573
 	case "ImageStreamImage":
574 574
 		imageStreamImage, err := g.Client.GetImageStreamImage(kapi.WithNamespace(ctx, namespace), from.Name)
575 575
 		if err != nil {
576
-			glog.V(2).Infof("Error ImageStreamReference %s in namespace %s: %v", from.Name, namespace, err)
577
-			if errors.IsNotFound(err) {
578
-				return "", err
579
-			}
576
+			err = resolveError(from.Kind, namespace, from.Name, err)
577
+			glog.V(2).Info(err)
580 578
 			return "", err
581 579
 		}
582 580
 		image := imageStreamImage.Image
... ...
@@ -585,10 +586,8 @@ func (g *BuildGenerator) resolveImageStreamReference(ctx kapi.Context, from kapi
585 585
 	case "ImageStreamTag":
586 586
 		imageStreamTag, err := g.Client.GetImageStreamTag(kapi.WithNamespace(ctx, namespace), from.Name)
587 587
 		if err != nil {
588
-			glog.V(2).Infof("Error resolving ImageStreamTag reference %s in namespace %s: %v", from.Name, namespace, err)
589
-			if errors.IsNotFound(err) {
590
-				return "", err
591
-			}
588
+			err = resolveError(from.Kind, namespace, from.Name, err)
589
+			glog.V(2).Info(err)
592 590
 			return "", err
593 591
 		}
594 592
 		image := imageStreamTag.Image
... ...
@@ -614,10 +613,8 @@ func (g *BuildGenerator) resolveImageStreamDockerRepository(ctx kapi.Context, fr
614 614
 	case "ImageStreamImage":
615 615
 		imageStreamImage, err := g.Client.GetImageStreamImage(kapi.WithNamespace(ctx, namespace), from.Name)
616 616
 		if err != nil {
617
-			glog.V(2).Infof("Error ImageStreamReference %s in namespace %s: %v", from.Name, namespace, err)
618
-			if errors.IsNotFound(err) {
619
-				return "", err
620
-			}
617
+			err = resolveError(from.Kind, namespace, from.Name, err)
618
+			glog.V(2).Info(err)
621 619
 			return "", err
622 620
 		}
623 621
 		image := imageStreamImage.Image
... ...
@@ -627,10 +624,8 @@ func (g *BuildGenerator) resolveImageStreamDockerRepository(ctx kapi.Context, fr
627 627
 		name := strings.Split(from.Name, ":")[0]
628 628
 		is, err := g.Client.GetImageStream(kapi.WithNamespace(ctx, namespace), name)
629 629
 		if err != nil {
630
-			glog.V(2).Infof("Error getting ImageStream %s/%s: %v", namespace, name, err)
631
-			if errors.IsNotFound(err) {
632
-				return "", err
633
-			}
630
+			err = resolveError("ImageStream", namespace, from.Name, err)
631
+			glog.V(2).Info(err)
634 632
 			return "", err
635 633
 		}
636 634
 		image, err := imageapi.DockerImageReferenceForStream(is)
... ...
@@ -674,6 +669,24 @@ func (g *BuildGenerator) resolveImageSecret(ctx kapi.Context, secrets []kapi.Sec
674 674
 	return nil
675 675
 }
676 676
 
677
+func resolveError(kind string, namespace string, name string, err error) error {
678
+	msg := fmt.Sprintf("Error resolving %s %s in namespace %s: %v", kind, name, namespace, err)
679
+	return &errors.StatusError{unversioned.Status{
680
+		Status:  unversioned.StatusFailure,
681
+		Code:    errors.StatusUnprocessableEntity,
682
+		Reason:  unversioned.StatusReasonInvalid,
683
+		Message: msg,
684
+		Details: &unversioned.StatusDetails{
685
+			Kind: kind,
686
+			Name: name,
687
+			Causes: []unversioned.StatusCause{{
688
+				Field:   "from",
689
+				Message: msg,
690
+			}},
691
+		},
692
+	}}
693
+}
694
+
677 695
 // getNextBuildName returns name of the next build and increments BuildConfig's LastVersion.
678 696
 func getNextBuildName(buildConfig *buildapi.BuildConfig) string {
679 697
 	buildConfig.Status.LastVersion++
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	"testing"
9 9
 
10 10
 	kapi "k8s.io/kubernetes/pkg/api"
11
+	"k8s.io/kubernetes/pkg/api/errors"
11 12
 	"k8s.io/kubernetes/pkg/api/resource"
12 13
 
13 14
 	"k8s.io/kubernetes/pkg/client/unversioned/testclient"
... ...
@@ -443,6 +444,30 @@ func TestInstantiateWithLastVersion(t *testing.T) {
443 443
 	}
444 444
 }
445 445
 
446
+func TestInstantiateWithMissingImageStream(t *testing.T) {
447
+	g := mockBuildGenerator()
448
+	c := g.Client.(Client)
449
+	c.GetImageStreamTagFunc = func(ctx kapi.Context, name string) (*imageapi.ImageStreamTag, error) {
450
+		return nil, errors.NewNotFound(imageapi.Resource("imagestreamtags"), "testRepo")
451
+	}
452
+	g.Client = c
453
+
454
+	_, err := g.Instantiate(kapi.NewDefaultContext(), &buildapi.BuildRequest{})
455
+	se, ok := err.(*errors.StatusError)
456
+
457
+	if !ok {
458
+		t.Errorf("Expected errors.StatusError, got %T", err)
459
+	}
460
+
461
+	if se.ErrStatus.Code != errors.StatusUnprocessableEntity {
462
+		t.Errorf("Expected status 422, got %d", se.ErrStatus.Code)
463
+	}
464
+
465
+	if !strings.Contains(se.ErrStatus.Message, "testns") {
466
+		t.Errorf("Error message does not contain namespace: %q", se.ErrStatus.Message)
467
+	}
468
+}
469
+
446 470
 func TestInstantiateWithLabelsAndAnnotations(t *testing.T) {
447 471
 	g := mockBuildGenerator()
448 472
 	c := g.Client.(Client)
... ...
@@ -38,7 +38,7 @@ import (
38 38
 func describerMap(c *client.Client, kclient kclient.Interface, host string) map[unversioned.GroupKind]kctl.Describer {
39 39
 	m := map[unversioned.GroupKind]kctl.Describer{
40 40
 		buildapi.Kind("Build"):                        &BuildDescriber{c, kclient},
41
-		buildapi.Kind("BuildConfig"):                  &BuildConfigDescriber{c, host},
41
+		buildapi.Kind("BuildConfig"):                  &BuildConfigDescriber{c, kclient, host},
42 42
 		deployapi.Kind("DeploymentConfig"):            &DeploymentConfigDescriber{c, kclient, nil},
43 43
 		authorizationapi.Kind("Identity"):             &IdentityDescriber{c},
44 44
 		imageapi.Kind("Image"):                        &ImageDescriber{c},
... ...
@@ -171,7 +171,8 @@ func describeBuildDuration(build *buildapi.Build) string {
171 171
 // BuildConfigDescriber generates information about a buildConfig
172 172
 type BuildConfigDescriber struct {
173 173
 	client.Interface
174
-	host string
174
+	kubeClient kclient.Interface
175
+	host       string
175 176
 }
176 177
 
177 178
 func nameAndNamespace(ns, name string) string {
... ...
@@ -439,23 +440,31 @@ func (d *BuildConfigDescriber) Describe(namespace, name string, settings kctl.De
439 439
 		describeCommonSpec(buildConfig.Spec.CommonSpec, out)
440 440
 		formatString(out, "\nBuild Run Policy", string(buildConfig.Spec.RunPolicy))
441 441
 		d.DescribeTriggers(buildConfig, out)
442
-		if len(buildList.Items) == 0 {
443
-			return nil
442
+
443
+		if len(buildList.Items) > 0 {
444
+			fmt.Fprintf(out, "\nBuild\tStatus\tDuration\tCreation Time\n")
445
+
446
+			builds := buildList.Items
447
+			sort.Sort(sort.Reverse(buildapi.BuildSliceByCreationTimestamp(builds)))
448
+
449
+			for i, build := range builds {
450
+				fmt.Fprintf(out, "%s \t%s \t%v \t%v\n",
451
+					build.Name,
452
+					strings.ToLower(string(build.Status.Phase)),
453
+					describeBuildDuration(&build),
454
+					build.CreationTimestamp.Rfc3339Copy().Time)
455
+				// only print the 10 most recent builds.
456
+				if i == 9 {
457
+					break
458
+				}
459
+			}
444 460
 		}
445
-		fmt.Fprintf(out, "\nBuild\tStatus\tDuration\tCreation Time\n")
446
-
447
-		builds := buildList.Items
448
-		sort.Sort(sort.Reverse(buildapi.BuildSliceByCreationTimestamp(builds)))
449
-
450
-		for i, build := range builds {
451
-			fmt.Fprintf(out, "%s \t%s \t%v \t%v\n",
452
-				build.Name,
453
-				strings.ToLower(string(build.Status.Phase)),
454
-				describeBuildDuration(&build),
455
-				build.CreationTimestamp.Rfc3339Copy().Time)
456
-			// only print the 10 most recent builds.
457
-			if i == 9 {
458
-				break
461
+
462
+		if settings.ShowEvents {
463
+			events, _ := d.kubeClient.Events(namespace).Search(buildConfig)
464
+			if events != nil {
465
+				fmt.Fprint(out, "\n")
466
+				kctl.DescribeEvents(events, out)
459 467
 			}
460 468
 		}
461 469
 		return nil
... ...
@@ -124,7 +124,7 @@ func TestDescribers(t *testing.T) {
124 124
 		name string
125 125
 	}{
126 126
 		{&BuildDescriber{c, fakeKube}, "bar"},
127
-		{&BuildConfigDescriber{c, ""}, "bar"},
127
+		{&BuildConfigDescriber{c, fakeKube, ""}, "bar"},
128 128
 		{&ImageDescriber{c}, "bar"},
129 129
 		{&ImageStreamDescriber{c}, "bar"},
130 130
 		{&ImageStreamTagDescriber{c}, "bar:latest"},