tolerate multiple bcs pointing to same istag for oc status

Author: gabemontero

pkg/api/graph/test/missing-istag.yaml

@@ -32,6 +32,38 @@ items:
       type: Generic
     - imageChange: {}
       type: ImageChange
+- apiVersion: v1
+  kind: BuildConfig
+  metadata:
+    creationTimestamp: null
+    labels:
+      app: ruby
+    name: ruby-hello-world-2
+  spec:
+    output:
+      to:
+        kind: ImageStreamTag
+        name: ruby-goodbye-world:latest
+    resources: {}
+    source:
+      git:
+        uri: https://github.com/openshift/ruby-hello-world
+      type: Git
+    strategy:
+      dockerStrategy:
+        from:
+          kind: ImageStreamTag
+          name: ruby-22-centos7:latest
+      type: Docker
+    triggers:
+    - github:
+        secret: LyddbeCAaw1a0x08xz9n
+      type: GitHub
+    - generic:
+        secret: ZnYJJeEvo1ri0Gk0f6YY
+      type: Generic
+    - imageChange: {}
+      type: ImageChange
 - apiVersion: v1
   kind: ImageStream
   metadata:

pkg/api/graph/test/unpushable-build.yaml

@@ -35,6 +35,41 @@ items:
       type: ImageChange
   status:
     lastVersion: 0
+- apiVersion: v1
+  kind: BuildConfig
+  metadata:
+    creationTimestamp: null
+    labels:
+      app: ruby
+    name: ruby-hello-world-2
+    namespace: example
+  spec:
+    output:
+      to:
+        kind: ImageStreamTag
+        name: ruby-hello-world:latest
+    resources: {}
+    source:
+      git:
+        uri: https://github.com/openshift/ruby-hello-world
+      type: Git
+    strategy:
+      dockerStrategy:
+        from:
+          kind: ImageStreamTag
+          name: ruby-22-centos7:latest
+      type: Docker
+    triggers:
+    - github:
+        secret: LyddbeCAaw1a0x08xz9n
+      type: GitHub
+    - generic:
+        secret: ZnYJJeEvo1ri0Gk0f6YY
+      type: Generic
+    - imageChange: {}
+      type: ImageChange
+  status:
+    lastVersion: 0
 - apiVersion: v1
   kind: Build
   metadata:

pkg/build/graph/analysis/bc.go

@@ -162,6 +162,91 @@ func FindCircularBuilds(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
 	return markers
 }
 
+// multiBCStartBuildSuggestion builds the `oc start-build` suggestion string with multiple build configs
+func multiBCStartBuildSuggestion(bcNodes []*buildgraph.BuildConfigNode) string {
+	var ret string
+	if len(bcNodes) > 1 {
+		ret = "Run one of the following commands: "
+	}
+	for i, bcNode := range bcNodes {
+		// use of f.ResourceName(bcNode) will produce a string like  oc start-build BuildConfig|example/ruby-hello-world
+		ret = ret + fmt.Sprintf("oc start-build %s", bcNode.BuildConfig.GetName())
+		if i < (len(bcNodes) - 1) {
+			ret = ret + ", "
+		}
+	}
+	return ret
+}
+
+// bcNodesToRelatedNodes takes an array of BuildConfigNode's and returns an array of graph.Node's for the Marker.RelatedNodes field
+func bcNodesToRelatedNodes(bcNodes []*buildgraph.BuildConfigNode) []graph.Node {
+	relatedNodes := []graph.Node{}
+	for _, bcNode := range bcNodes {
+		relatedNodes = append(relatedNodes, graph.Node(bcNode))
+	}
+	return relatedNodes
+}
+
+// findPendingTagMarkers is the guts behind FindPendingTags .... break out some of the content and reduce some indentation
+func findPendingTagMarkers(istNode *imagegraph.ImageStreamTagNode, g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
+	markers := []osgraph.Marker{}
+
+	buildFound := false
+	bcNodes := buildedges.BuildConfigsForTag(g, graph.Node(istNode))
+	for _, bcNode := range bcNodes {
+		latestBuild := buildedges.GetLatestBuild(g, bcNode)
+
+		// A build config points to the non existent tag but no current build exists.
+		if latestBuild == nil {
+			continue
+		}
+		buildFound = true
+
+		// A build config points to the non existent tag but something is going on with
+		// the latest build.
+		// TODO: Handle other build phases.
+		switch latestBuild.Build.Status.Phase {
+		case buildapi.BuildPhaseCancelled:
+			// TODO: Add a warning here.
+		case buildapi.BuildPhaseError:
+			// TODO: Add a warning here.
+		case buildapi.BuildPhaseComplete:
+			// We should never hit this. The output of our build is missing but the build is complete.
+			// Most probably the user has messed up?
+		case buildapi.BuildPhaseFailed:
+			// Since the tag hasn't been populated yet, we assume there hasn't been a successful
+			// build so far.
+			markers = append(markers, osgraph.Marker{
+				Node:         graph.Node(latestBuild),
+				RelatedNodes: []graph.Node{graph.Node(istNode), graph.Node(bcNode)},
+
+				Severity:   osgraph.ErrorSeverity,
+				Key:        LatestBuildFailedErr,
+				Message:    fmt.Sprintf("%s has failed.", f.ResourceName(latestBuild)),
+				Suggestion: osgraph.Suggestion(fmt.Sprintf("Inspect the build failure with 'oc logs -f bc/%s'", bcNode.BuildConfig.GetName())),
+			})
+		default:
+			// Do nothing when latest build is new, pending, or running.
+		}
+
+	}
+
+	// if no current builds exist for any of the build configs, append marker for that
+	// but ignore ISTs which have no build configs
+	if !buildFound && len(bcNodes) > 0 {
+		markers = append(markers, osgraph.Marker{
+			Node:         graph.Node(istNode),
+			RelatedNodes: bcNodesToRelatedNodes(bcNodes),
+
+			Severity:   osgraph.WarningSeverity,
+			Key:        TagNotAvailableWarning,
+			Message:    fmt.Sprintf("%s needs to be imported or created by a build.", f.ResourceName(istNode)),
+			Suggestion: osgraph.Suggestion(multiBCStartBuildSuggestion(bcNodes)),
+		})
+	}
+	return markers
+}
+
 // FindPendingTags inspects all imageStreamTags that serve as outputs to builds.
 //
 // Precedence of failures:
@@ -172,49 +257,8 @@ func FindPendingTags(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
 
 	for _, uncastIstNode := range g.NodesByKind(imagegraph.ImageStreamTagNodeKind) {
 		istNode := uncastIstNode.(*imagegraph.ImageStreamTagNode)
-		if bcNode := buildedges.BuildConfigForTag(g, uncastIstNode); bcNode != nil && !istNode.Found() {
-			latestBuild := buildedges.GetLatestBuild(g, bcNode)
-
-			// A build config points to the non existent tag but no current build exists.
-			if latestBuild == nil {
-				markers = append(markers, osgraph.Marker{
-					Node:         graph.Node(bcNode),
-					RelatedNodes: []graph.Node{uncastIstNode},
-
-					Severity:   osgraph.WarningSeverity,
-					Key:        TagNotAvailableWarning,
-					Message:    fmt.Sprintf("%s needs to be imported or created by a build.", f.ResourceName(istNode)),
-					Suggestion: osgraph.Suggestion(fmt.Sprintf("oc start-build %s", f.ResourceName(bcNode))),
-				})
-				continue
-			}
-
-			// A build config points to the non existent tag but something is going on with
-			// the latest build.
-			// TODO: Handle other build phases.
-			switch latestBuild.Build.Status.Phase {
-			case buildapi.BuildPhaseCancelled:
-				// TODO: Add a warning here.
-			case buildapi.BuildPhaseError:
-				// TODO: Add a warning here.
-			case buildapi.BuildPhaseComplete:
-				// We should never hit this. The output of our build is missing but the build is complete.
-				// Most probably the user has messed up?
-			case buildapi.BuildPhaseFailed:
-				// Since the tag hasn't been populated yet, we assume there hasn't been a successful
-				// build so far.
-				markers = append(markers, osgraph.Marker{
-					Node:         graph.Node(latestBuild),
-					RelatedNodes: []graph.Node{uncastIstNode, graph.Node(bcNode)},
-
-					Severity:   osgraph.ErrorSeverity,
-					Key:        LatestBuildFailedErr,
-					Message:    fmt.Sprintf("%s has failed.", f.ResourceName(latestBuild)),
-					Suggestion: osgraph.Suggestion(fmt.Sprintf("Inspect the build failure with 'oc logs %s'", f.ResourceName(latestBuild))),
-				})
-			default:
-				// Do nothing when latest build is new, pending, or running.
-			}
+		if !istNode.Found() {
+			markers = append(markers, findPendingTagMarkers(istNode, g, f)...)
 		}
 	}
 

pkg/build/graph/analysis/bc_test.go

@@ -24,7 +24,7 @@ func TestUnpushableBuild(t *testing.T) {
 	imageedges.AddAllImageStreamImageRefEdges(g)
 
 	markers := FindUnpushableBuildConfigs(g, osgraph.DefaultNamer)
-	if e, a := 1, len(markers); e != a {
+	if e, a := 2, len(markers); e != a {
 		t.Fatalf("expected %v, got %v", e, a)
 	}
 
@@ -33,9 +33,10 @@ func TestUnpushableBuild(t *testing.T) {
 	}
 
 	actualBC := osgraph.GetTopLevelContainerNode(g, markers[0].Node)
-	expectedBC := g.Find(osgraph.UniqueName("BuildConfig|example/ruby-hello-world"))
-	if e, a := expectedBC.ID(), actualBC.ID(); e != a {
-		t.Errorf("expected %v, got %v", e, a)
+	expectedBC1 := g.Find(osgraph.UniqueName("BuildConfig|example/ruby-hello-world"))
+	expectedBC2 := g.Find(osgraph.UniqueName("BuildConfig|example/ruby-hello-world-2"))
+	if e1, e2, a := expectedBC1.ID(), expectedBC2.ID(), actualBC.ID(); e1 != a && e2 != a {
+		t.Errorf("expected either %v or %v, got %v", e1, e2, a)
 	}
 
 	actualIST := markers[0].RelatedNodes[0]
@@ -229,4 +230,7 @@ func TestLatestBuildFailed(t *testing.T) {
 	if got, expected := markers[0].Key, LatestBuildFailedErr; got != expected {
 		t.Fatalf("expected marker key %q, got %q", expected, got)
 	}
+	if !strings.Contains(markers[0].Suggestion.String(), "oc logs -f bc/ruby-hello-world") {
+		t.Fatalf("expected oc logs -f bc/ruby-hello-world, got %s", markers[0].Suggestion.String())
+	}
 }

pkg/build/graph/helpers.go

@@ -82,13 +82,13 @@ func defaultNamespace(value, defaultValue string) string {
 	return value
 }
 
-// BuildConfigForTag returns the buildConfig that points to the provided imageStreamTag.
-// TODO: Handle multiple buildconfigs pointing to the same tag.
-func BuildConfigForTag(g osgraph.Graph, istag graph.Node) *buildgraph.BuildConfigNode {
+// BuildConfigsForTag returns the buildConfig that points to the provided imageStreamTag.
+func BuildConfigsForTag(g osgraph.Graph, istag graph.Node) []*buildgraph.BuildConfigNode {
+	bcs := []*buildgraph.BuildConfigNode{}
 	for _, bcNode := range g.PredecessorNodesByEdgeKind(istag, BuildOutputEdgeKind) {
-		return bcNode.(*buildgraph.BuildConfigNode)
+		bcs = append(bcs, bcNode.(*buildgraph.BuildConfigNode))
 	}
-	return nil
+	return bcs
 }
 
 // GetLatestBuild returns the latest build for the provided buildConfig.

pkg/deploy/graph/analysis/dc.go

@@ -63,7 +63,7 @@ func ictMarker(g osgraph.Graph, f osgraph.Namer, dcNode *deploygraph.DeploymentC
 				}
 			}
 
-			if bcNode := buildedges.BuildConfigForTag(g, istNode); bcNode != nil {
+			for _, bcNode := range buildedges.BuildConfigsForTag(g, istNode) {
 				// Avoid warning for the dc image trigger in case there is a build in flight.
 				if latestBuild := buildedges.GetLatestBuild(g, bcNode); latestBuild != nil && !buildutil.IsBuildComplete(latestBuild.Build) {
 					return nil