Skip to content

Commit f7ba983

Browse files
gabemonterogabemontero
committed
tolerate multiple bcs pointing to same istag for oc status
1 parent 4148579 commit f7ba983

File tree

6 files changed

+137
-56
lines changed

6 files changed

+137
-56
lines changed

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,38 @@ items:
3232
type: Generic
3333
- imageChange: {}
3434
type: ImageChange
35+
- apiVersion: v1
36+
kind: BuildConfig
37+
metadata:
38+
creationTimestamp: null
39+
labels:
40+
app: ruby
41+
name: ruby-hello-world-2
42+
spec:
43+
output:
44+
to:
45+
kind: ImageStreamTag
46+
name: ruby-goodbye-world:latest
47+
resources: {}
48+
source:
49+
git:
50+
uri: https://github.com/openshift/ruby-hello-world
51+
type: Git
52+
strategy:
53+
dockerStrategy:
54+
from:
55+
kind: ImageStreamTag
56+
name: ruby-22-centos7:latest
57+
type: Docker
58+
triggers:
59+
- github:
60+
secret: LyddbeCAaw1a0x08xz9n
61+
type: GitHub
62+
- generic:
63+
secret: ZnYJJeEvo1ri0Gk0f6YY
64+
type: Generic
65+
- imageChange: {}
66+
type: ImageChange
3567
- apiVersion: v1
3668
kind: ImageStream
3769
metadata:

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,41 @@ items:
3535
type: ImageChange
3636
status:
3737
lastVersion: 0
38+
- apiVersion: v1
39+
kind: BuildConfig
40+
metadata:
41+
creationTimestamp: null
42+
labels:
43+
app: ruby
44+
name: ruby-hello-world-2
45+
namespace: example
46+
spec:
47+
output:
48+
to:
49+
kind: ImageStreamTag
50+
name: ruby-hello-world:latest
51+
resources: {}
52+
source:
53+
git:
54+
uri: https://github.com/openshift/ruby-hello-world
55+
type: Git
56+
strategy:
57+
dockerStrategy:
58+
from:
59+
kind: ImageStreamTag
60+
name: ruby-22-centos7:latest
61+
type: Docker
62+
triggers:
63+
- github:
64+
secret: LyddbeCAaw1a0x08xz9n
65+
type: GitHub
66+
- generic:
67+
secret: ZnYJJeEvo1ri0Gk0f6YY
68+
type: Generic
69+
- imageChange: {}
70+
type: ImageChange
71+
status:
72+
lastVersion: 0
3873
- apiVersion: v1
3974
kind: Build
4075
metadata:

‎pkg/build/graph/analysis/bc.go

Lines changed: 57 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,62 @@ func FindCircularBuilds(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
162162
return markers
163163
}
164164

165+
// findPendingTags is the guts behind FindPendingTags .... break out some of the content and reduce some indentation
166+
func findPendingTags(uncastIstNode graph.Node, g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
167+
markers := []osgraph.Marker{}
168+
istNode := uncastIstNode.(*imagegraph.ImageStreamTagNode)
169+
if istNode.Found() {
170+
return markers
171+
}
172+
bcNodes := buildedges.BuildConfigsForTag(g, uncastIstNode)
173+
for _, bcNode := range bcNodes {
174+
latestBuild := buildedges.GetLatestBuild(g, bcNode)
175+
176+
// A build config points to the non existent tag but no current build exists.
177+
if latestBuild == nil {
178+
markers = append(markers, osgraph.Marker{
179+
Node: graph.Node(bcNode),
180+
RelatedNodes: []graph.Node{uncastIstNode},
181+
182+
Severity: osgraph.WarningSeverity,
183+
Key: TagNotAvailableWarning,
184+
Message: fmt.Sprintf("%s needs to be imported or created by a build.", f.ResourceName(istNode)),
185+
Suggestion: osgraph.Suggestion(fmt.Sprintf("oc start-build %s", f.ResourceName(bcNode))),
186+
})
187+
continue
188+
}
189+
190+
// A build config points to the non existent tag but something is going on with
191+
// the latest build.
192+
// TODO: Handle other build phases.
193+
switch latestBuild.Build.Status.Phase {
194+
case buildapi.BuildPhaseCancelled:
195+
// TODO: Add a warning here.
196+
case buildapi.BuildPhaseError:
197+
// TODO: Add a warning here.
198+
case buildapi.BuildPhaseComplete:
199+
// We should never hit this. The output of our build is missing but the build is complete.
200+
// Most probably the user has messed up?
201+
case buildapi.BuildPhaseFailed:
202+
// Since the tag hasn't been populated yet, we assume there hasn't been a successful
203+
// build so far.
204+
markers = append(markers, osgraph.Marker{
205+
Node: graph.Node(latestBuild),
206+
RelatedNodes: []graph.Node{uncastIstNode, graph.Node(bcNode)},
207+
208+
Severity: osgraph.ErrorSeverity,
209+
Key: LatestBuildFailedErr,
210+
Message: fmt.Sprintf("%s has failed.", f.ResourceName(latestBuild)),
211+
Suggestion: osgraph.Suggestion(fmt.Sprintf("Inspect the build failure with 'oc logs %s'", f.ResourceName(latestBuild))),
212+
})
213+
default:
214+
// Do nothing when latest build is new, pending, or running.
215+
}
216+
217+
}
218+
return markers
219+
}
220+
165221
// FindPendingTags inspects all imageStreamTags that serve as outputs to builds.
166222
//
167223
// Precedence of failures:
@@ -171,51 +227,7 @@ func FindPendingTags(g osgraph.Graph, f osgraph.Namer) []osgraph.Marker {
171227
markers := []osgraph.Marker{}
172228

173229
for _, uncastIstNode := range g.NodesByKind(imagegraph.ImageStreamTagNodeKind) {
174-
istNode := uncastIstNode.(*imagegraph.ImageStreamTagNode)
175-
if bcNode := buildedges.BuildConfigForTag(g, uncastIstNode); bcNode != nil && !istNode.Found() {
176-
latestBuild := buildedges.GetLatestBuild(g, bcNode)
177-
178-
// A build config points to the non existent tag but no current build exists.
179-
if latestBuild == nil {
180-
markers = append(markers, osgraph.Marker{
181-
Node: graph.Node(bcNode),
182-
RelatedNodes: []graph.Node{uncastIstNode},
183-
184-
Severity: osgraph.WarningSeverity,
185-
Key: TagNotAvailableWarning,
186-
Message: fmt.Sprintf("%s needs to be imported or created by a build.", f.ResourceName(istNode)),
187-
Suggestion: osgraph.Suggestion(fmt.Sprintf("oc start-build %s", f.ResourceName(bcNode))),
188-
})
189-
continue
190-
}
191-
192-
// A build config points to the non existent tag but something is going on with
193-
// the latest build.
194-
// TODO: Handle other build phases.
195-
switch latestBuild.Build.Status.Phase {
196-
case buildapi.BuildPhaseCancelled:
197-
// TODO: Add a warning here.
198-
case buildapi.BuildPhaseError:
199-
// TODO: Add a warning here.
200-
case buildapi.BuildPhaseComplete:
201-
// We should never hit this. The output of our build is missing but the build is complete.
202-
// Most probably the user has messed up?
203-
case buildapi.BuildPhaseFailed:
204-
// Since the tag hasn't been populated yet, we assume there hasn't been a successful
205-
// build so far.
206-
markers = append(markers, osgraph.Marker{
207-
Node: graph.Node(latestBuild),
208-
RelatedNodes: []graph.Node{uncastIstNode, graph.Node(bcNode)},
209-
210-
Severity: osgraph.ErrorSeverity,
211-
Key: LatestBuildFailedErr,
212-
Message: fmt.Sprintf("%s has failed.", f.ResourceName(latestBuild)),
213-
Suggestion: osgraph.Suggestion(fmt.Sprintf("Inspect the build failure with 'oc logs %s'", f.ResourceName(latestBuild))),
214-
})
215-
default:
216-
// Do nothing when latest build is new, pending, or running.
217-
}
218-
}
230+
markers = append(markers, findPendingTags(uncastIstNode, g, f)...)
219231
}
220232

221233
return markers

‎pkg/build/graph/analysis/bc_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func TestUnpushableBuild(t *testing.T) {
2424
imageedges.AddAllImageStreamImageRefEdges(g)
2525

2626
markers := FindUnpushableBuildConfigs(g, osgraph.DefaultNamer)
27-
if e, a := 1, len(markers); e != a {
27+
if e, a := 2, len(markers); e != a {
2828
t.Fatalf("expected %v, got %v", e, a)
2929
}
3030

@@ -33,9 +33,10 @@ func TestUnpushableBuild(t *testing.T) {
3333
}
3434

3535
actualBC := osgraph.GetTopLevelContainerNode(g, markers[0].Node)
36-
expectedBC := g.Find(osgraph.UniqueName("BuildConfig|example/ruby-hello-world"))
37-
if e, a := expectedBC.ID(), actualBC.ID(); e != a {
38-
t.Errorf("expected %v, got %v", e, a)
36+
expectedBC1 := g.Find(osgraph.UniqueName("BuildConfig|example/ruby-hello-world"))
37+
expectedBC2 := g.Find(osgraph.UniqueName("BuildConfig|example/ruby-hello-world-2"))
38+
if e1, e2, a := expectedBC1.ID(), expectedBC2.ID(), actualBC.ID(); e1 != a && e2 != a {
39+
t.Errorf("expected either %v or %v, got %v", e1, e2, a)
3940
}
4041

4142
actualIST := markers[0].RelatedNodes[0]
@@ -202,7 +203,7 @@ func TestPendingImageStreamTag(t *testing.T) {
202203
g = g.Subgraph(nodeFn, edgeFn)
203204

204205
markers := FindPendingTags(g, osgraph.DefaultNamer)
205-
if e, a := 1, len(markers); e != a {
206+
if e, a := 2, len(markers); e != a {
206207
t.Fatalf("expected %v, got %v", e, a)
207208
}
208209

‎pkg/build/graph/helpers.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,13 @@ func defaultNamespace(value, defaultValue string) string {
8282
return value
8383
}
8484

85-
// BuildConfigForTag returns the buildConfig that points to the provided imageStreamTag.
86-
// TODO: Handle multiple buildconfigs pointing to the same tag.
87-
func BuildConfigForTag(g osgraph.Graph, istag graph.Node) *buildgraph.BuildConfigNode {
85+
// BuildConfigsForTag returns the buildConfig that points to the provided imageStreamTag.
86+
func BuildConfigsForTag(g osgraph.Graph, istag graph.Node) []*buildgraph.BuildConfigNode {
87+
bcs := []*buildgraph.BuildConfigNode{}
8888
for _, bcNode := range g.PredecessorNodesByEdgeKind(istag, BuildOutputEdgeKind) {
89-
return bcNode.(*buildgraph.BuildConfigNode)
89+
bcs = append(bcs, bcNode.(*buildgraph.BuildConfigNode))
9090
}
91-
return nil
91+
return bcs
9292
}
9393

9494
// GetLatestBuild returns the latest build for the provided buildConfig.

‎pkg/deploy/graph/analysis/dc.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ func ictMarker(g osgraph.Graph, f osgraph.Namer, dcNode *deploygraph.DeploymentC
6363
}
6464
}
6565

66-
if bcNode := buildedges.BuildConfigForTag(g, istNode); bcNode != nil {
66+
bcNodes := buildedges.BuildConfigsForTag(g, istNode)
67+
for _, bcNode := range bcNodes {
6768
// Avoid warning for the dc image trigger in case there is a build in flight.
6869
if latestBuild := buildedges.GetLatestBuild(g, bcNode); latestBuild != nil && !buildutil.IsBuildComplete(latestBuild.Build) {
6970
return nil

0 commit comments

Comments
 (0)