display build chains in oc status #9411

gabemontero · 2016-06-17T20:57:22Z

smarterclayton · 2016-06-18T16:22:30Z

pkg/cmd/cli/describe/projectstatus.go

@@ -233,6 +233,9 @@ func (d *ProjectStatusDescriber) Describe(namespace, name string) (string, error
 			printLines(out, indent, 0, describeMonopod(f, monopod.Pod)...)
 		}

+		bcPipelines, _ := graphview.AllImagePipelinesFromBuildConfig(g, graphview.IntSet{})


Isn't this redundant information from what we are already showing? I like the general idea, but I don't like displaying redundant info. Is a build chain useful to me? What about all the other inputs to the build chain? What do we do when there are multiple inputs to the same build (because of image sources)?

A few thoughts:

terminology

the subject of the associated issue - making the links between build configs obvious

the description that follows mentioned "build chains", but given what @smarterclayton points out, "build chains" implies more than the link between build configs

so, if making the link between build configs more obvious is something worth pursuing, back off the term "build chain" in any oc status output

location

I asked for opinions on whether to denote connection in a separate section (perhaps contributing to the redundancy) vs. tweaking the existing umbrella of output

so exploring the latter flavor a little, how would changing this output:

https://www.example.com (svc/frontend) dc/frontend deploys istag/origin-ruby-sample:latest <- bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with pipelineproject/ruby-base-image:latest not built yet deployment #1 waiting on image

To this output:

https://www.example.com (svc/frontend) dc/frontend deploys istag/origin-ruby-sample:latest <- bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with pipelineproject/ruby-base-image:latest (produced by bc/ruby-base) not built yet deployment #1 waiting on image

be something worth pursuing (and honor the intent of the associated issue)?

Isn't this redundant information from what we are already showing?

It wasn't before when I wrote up the bug. There was no way to relate to the two and when you got more than one in, it was very difficult to follow. I like the "produced by" text in @gabemontero's example.

Is a build chain useful to me?

The chain is useful when you're trying to kick a build by rebuilding its input image. It comes up for me when I'm trying to update base layers which contain utilities and prereqs. I have one image with fedora plus basic tools (everything I build runs on top of this), another image with specific prereqs (think: this app needs ffmpeg), and finally my top layer. I want to rebuild image two, not one or three. But the build I remember is for image three. This lets me track backwards.

What do we do when there are multiple inputs to the same build (because of image sources)?

Hmmm, that wasn't a thing when I was dealing with this last.

Yeah, Images got added to the BuildSource section of the BuildConfig as recently as late 2015 according to git blame. It is a peer member to items like the Git connection information for example in the BuildSource struct, and separate from the "base image" specified in the From field of the 3 classic BuildStrategy types.

It is a way to overlay files from additional images on top of any files produced by the "build" (overloaded term :-) ) performed by the s2i/docker/custom strategies.

There is a minor inspection of it in already oc status here, but specific images are not mentioned.

With that background, next set of thoughts:

leave any expansion of the exposure of Images to the trello card opened based on @smarterclayton 's call for a formal proposal on pipelines, alignment with console, etc.

in the short term, I'll refactor this pull to generate the "produce by..." output noted earlier; through either further discussion here or as 1) progresses, if such a change is deemed as an acceptable, short-term/tactical change, we can move toward merge; as such, I'll change this PR to WIP in the interim

ok, i've pushed the updates for "produced by ...."

pending comments on the specific changes, putting this PR on hold until https://trello.com/c/hHx4HrCG/966-integrate-pipeline-stages-into-oc-status gets going enough to determine for sure whether this addition to the existing output is acceptable.

smarterclayton · 2016-06-18T21:57:34Z

Have you discussed this with Jessica and how it fits into the consistent pipeline visual experience we want across both status and the UI for 1.3?

gabemontero · 2016-06-19T00:06:39Z

I have not - will talk to her.

On Saturday, June 18, 2016, Clayton Coleman notifications@github.com
wrote:

Have you discussed this with Jessica and how it fits into the consistent
pipeline visual experience we want across both status and the UI for 1.3?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#9411 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ADbadIu90y6Hz-oKzXGIeyncnWX6Wj4iks5qNGnWgaJpZM4I4uXr
.

smarterclayton · 2016-06-19T00:38:18Z

I'd like to see a proposal for how we're going to integrate pipeline stages into oc status (in combination with this, and also test deployments) before we implement something, and it definitely needs to align with the console visual direction.

gabemontero · 2016-06-19T00:55:29Z

OK, created https://trello.com/c/hHx4HrCG/966-integrate-pipeline-stages-into-oc-status to track.

@bparees FYI

deads2k · 2016-06-29T12:56:13Z

"produced by" works for me, though "from" is shorter.

@smarterclayton are you sure we should block this improvement on pipeline stages? This is helpful now and we don't guarantee stable output from oc status.

@gabemontero is there a reasonable way to detect that the image you're using was imported? If there's no build pointed to it and the image stream is set up for periodic imports, it seems reasonable to indicate that it triggers on a time based import.

gabemontero · 2016-06-29T21:32:42Z

I've pushed an addition that searches for scheduled imports and adds a (import scheduled) parenthetical to the output if no bc input chain exists and imports are scheduled.

smarterclayton · 2016-07-18T14:31:50Z

We can't keep adding things to oc status incrementally if we don't preserve room for the really important stuff coming down. Some of these things end up just being noise (we can't show everything in oc status) so I'm mostly looking for this to be "user driven" instead of "feature driven". Make the justification based on whether the user (both first time and experienced) must know this info, then work backwards. Can you paste representative latest example?

bparees · 2016-07-18T21:17:38Z

trying to mirror the web console overview in oc status seems like a mistake. to me, oc status is "tell me if there is anything obviously wrong w/ my configuration that i should look into".

(this comment applies to both the original build-chain request, and the pipeline representation).

we have oadm build-chain for showing build-chains. I'd be open to moving that command somewhere more natural for users, but oc status doesn't seem like the place.

smarterclayton · 2016-07-19T00:42:40Z

oc status is not really that. oc status is intended to be a summary of
what is going on right now in your project in a way that is relevant to a
CLI user, so it is the overview. It also gives you the summarization of
status, but it's not solely intended for that. So chains of builds need to
be succinctly represented, especially if they result in deployments.

On Mon, Jul 18, 2016 at 5:17 PM, Ben Parees notifications@github.com
wrote:

trying to mirror the web console overview in oc status seems like a
mistake. to me, oc status is "tell me if there is anything obviously wrong
w/ my configuration that i should look into".

(this comment applies to both the original build-chain request, and the
pipeline representation).

we have oadm build-chain for showing build-chains. I'd be open to moving
that command somewhere more natural for users, but oc status doesn't seem
like the place.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9411 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_pzn69tQ0SB0b-rDz_nOeH46hfpVwks5qW-10gaJpZM4I4uXr
.

bparees · 2016-07-19T03:08:39Z

oc status is not really that. oc status is intended to be a summary of
what is going on right now in your project in a way that is relevant to a
CLI user

do we have data that this is of value to users or requested by them? it's pretty hard to concisely and clearly represent that kind of information in a console form, particularly if your project is anything other than trivial (and if you're the kind of user who's creating trivial projects, I bet you're a user who's using the web console).

And it's only going to get worse(more complex to represent) with pipelines.

bparees · 2016-07-19T03:09:15Z

Maybe we can discuss this at tomorrow's CLI review (after discussing build commands). @fabianofranz @jwforres

deads2k · 2016-07-19T12:33:18Z

do we have data that this is of value to users or requested by them? it's pretty hard to concisely and clearly represent that kind of information in a console form, particularly if your project is anything other than trivial (and if you're the kind of user who's creating trivial projects, I bet you're a user who's using the web console).

"It's really hard to find and describe all the links between the various elements of my project" seems like an argument to provide the information, not an argument to give up and leave the linking to the user.

gabemontero · 2016-07-19T13:52:14Z

Re-posting a sample output per recent request:

https://www.example.com (svc/frontend)
  dc/frontend deploys istag/origin-ruby-sample:latest <-
    bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with pipelineproject/ruby-base-image:latest (produced by bc/ruby-base) 
      not built yet
    deployment #1 waiting on image

gabemontero · 2016-07-19T14:07:12Z

For the flavor where import of the builder image is scheduled:

https://www.example.com (svc/frontend)
  dc/frontend deploys istag/origin-ruby-sample:latest <-
    bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with pipelineproject/ruby-base-image:latest (import scheduled) 
      not built yet
    deployment #1 waiting on image

gabemontero · 2016-07-19T14:24:14Z

OK, per the discussion in CLI Design Review, I think we are ready to consider this for merging again.

IGTM comments were posted previously, but mergers, please re-review as you see fit, and then perhaps we can get the merge comment posted subsequently.

thanks

bparees · 2016-07-19T15:13:36Z

"It's really hard to find and describe all the links between the various elements of my project" seems like an argument to provide the information, not an argument to give up and leave the linking to the user.

no, it's an argument to use the right tool for the job, which in this case, imho, is not console text. I look forward to endless future debates about the right way to present this text(and how much text) to users when we get there.

gabemontero · 2016-07-25T13:29:08Z

bump @deads2k @bparees or @smarterclayton - any final reviews / are we good to merge ?

bparees · 2016-07-25T16:35:00Z

pkg/api/graph/graphview/image_pipeline.go

@@ -8,6 +8,7 @@ import (
 	osgraph "github.com/openshift/origin/pkg/api/graph"
 	buildedges "github.com/openshift/origin/pkg/build/graph"
 	buildgraph "github.com/openshift/origin/pkg/build/graph/nodes"
+	//imageapi "github.com/openshift/origin/pkg/image/api"


delete this if we don't need it anymore?

Deleted.

On Mon, Jul 25, 2016 at 12:35 PM, Ben Parees notifications@github.com
wrote:

In pkg/api/graph/graphview/image_pipeline.go
#9411 (comment):

@@ -8,6 +8,7 @@ import (
osgraph "github.com/openshift/origin/pkg/api/graph"
buildedges "github.com/openshift/origin/pkg/build/graph"
buildgraph "github.com/openshift/origin/pkg/build/graph/nodes"

//imageapi "github.com/openshift/origin/pkg/image/api"

delete this if we don't need it anymore?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openshift/origin/pull/9411/files/2eb378ef04518695d44ee2e03bcb9c38a7d94e0c#r72098002,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADbadPi76lgcWXjK-WBZZ66jizuWsXCwks5qZOW-gaJpZM4I4uXr
.

bparees · 2016-07-25T16:37:52Z

one nit and lgtm but i'd like @Kargakis or @deads2k to sign off.

smarterclayton · 2016-07-25T16:46:56Z

I land on David's side on "not giving up" on this data, and it's the assumption status is built under now, so the burden of creation will be here.

The output looks fine to me for now, but I haven't looked at the graph code.

…note input images have scheduled imports

gabemontero · 2016-08-05T14:30:52Z

@smarterclayton bump ... sounded like you wanted to look at the graph code but were good with the output
@deads2k I believe you had posted igtm, but just in case ...
@bparees fyi

gabemontero · 2016-08-09T13:20:32Z

@smarterclayton bump

smarterclayton · 2016-08-09T16:22:34Z

LGTM [merge]

smarterclayton · 2016-08-09T17:55:29Z

[merge] #9490
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_check/4736/

On Tue, Aug 9, 2016 at 1:52 PM, OpenShift Bot notifications@github.com
wrote:

continuous-integration/openshift-jenkins/merge FAILURE (
https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7678/)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9411 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_pzYD_F8qdxzAWCjt8byKAUZS8BJ9ks5qeL5igaJpZM4I4uXr
.

gabemontero · 2016-08-09T18:07:56Z

There was one failure in the conformance tests that looks like a prior flake (will open explicit flake if that proves to be the case ).

Investigating with a local repro attempt.

gabemontero · 2016-08-09T18:08:47Z

(Thanks @smarterclayton for rehitting the merge button in interim)

gabemontero · 2016-08-09T18:19:20Z

created flake https://ci.openshift.redhat.com/jenkins/failure-cause-management/57aa1e88e4b05a5281cce5b3

gabemontero · 2016-08-09T18:26:29Z

test passes in my local copy of this branch

openshift-bot · 2016-08-09T19:17:39Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7697/) (Image: devenv-rhel7_4790)

gabemontero · 2016-08-09T19:32:31Z

this time, the integration test suffered a panic in github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/client/unversioned.RetryOnConflict.func1 at /data/src/github.com/openshift/origin/_output/local/go/src/github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/client/unversioned/util.go:64

and the origin_check failed with:

FAILURE after 0.335s: test/cmd/migrate.sh:38: executing 'oc import-image --from=php:latest test:2 --confirm' expecting success: the command returned the wrong error code
There was no output from the command.
Standard error from the command:
Error from server: Operation cannot be fulfilled on imagestream "test": the image stream was updated from "5720" to "5721"

Bot unrelated to the oc status change.

bparees · 2016-08-09T20:39:47Z

@gabemontero do we have open flake issues for all 3 of those things? seems like we should.

gabemontero · 2016-08-09T21:09:55Z

@bparees Created https://ci.openshift.redhat.com/jenkins/failure-cause-management/57aa4522e4b05a5281cce649 for the import image issue on the origin_check

And created https://ci.openshift.redhat.com/jenkins/failure-cause-management/57aa465fe4b05a5281cce64d for the k8s panic.

Mind hitting the merge button one more time? thanks ...

bparees · 2016-08-09T21:14:44Z

now that we have flake issues for them, i don't :)
[merge]

openshift-bot · 2016-08-09T21:17:24Z

Evaluated for origin merge up to 3b70f9c

smarterclayton reviewed Jun 18, 2016
View reviewed changes

gabemontero changed the title ~~display build chains in oc status~~ WIP: display build chains in oc status Jun 20, 2016

gabemontero force-pushed the issue8338 branch from fdd446e to 0c64173 Compare June 20, 2016 20:23

gabemontero force-pushed the issue8338 branch from 0c64173 to fc052ce Compare June 29, 2016 21:29

gabemontero force-pushed the issue8338 branch 2 times, most recently from 55eba0f to 40595de Compare July 1, 2016 14:15

gabemontero force-pushed the issue8338 branch from 40595de to 2eb378e Compare July 18, 2016 13:50

gabemontero changed the title ~~WIP: display build chains in oc status~~ display build chains in oc status Jul 19, 2016

bparees reviewed Jul 25, 2016
View reviewed changes

display build config dependencies more explicitly in oc status; also …

3b70f9c

…note input images have scheduled imports

gabemontero force-pushed the issue8338 branch from 2eb378e to 3b70f9c Compare July 25, 2016 17:23

openshift-bot merged commit 92c4448 into openshift:master Aug 9, 2016

liggitt mentioned this pull request Aug 10, 2016

test suites fail to compile #10319

Closed

gabemontero deleted the issue8338 branch August 10, 2016 13:43

display build chains in oc status #9411

display build chains in oc status #9411

Conversation

gabemontero commented Jun 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarterclayton commented Jun 18, 2016

gabemontero commented Jun 19, 2016

smarterclayton commented Jun 19, 2016

gabemontero commented Jun 19, 2016

deads2k commented Jun 29, 2016

gabemontero commented Jun 29, 2016

smarterclayton commented Jul 18, 2016

bparees commented Jul 18, 2016

smarterclayton commented Jul 19, 2016

bparees commented Jul 19, 2016

bparees commented Jul 19, 2016

deads2k commented Jul 19, 2016

gabemontero commented Jul 19, 2016

gabemontero commented Jul 19, 2016

gabemontero commented Jul 19, 2016

bparees commented Jul 19, 2016

gabemontero commented Jul 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparees commented Jul 25, 2016

smarterclayton commented Jul 25, 2016

gabemontero commented Aug 5, 2016

gabemontero commented Aug 9, 2016

smarterclayton commented Aug 9, 2016

smarterclayton commented Aug 9, 2016

gabemontero commented Aug 9, 2016

gabemontero commented Aug 9, 2016

gabemontero commented Aug 9, 2016

gabemontero commented Aug 9, 2016

openshift-bot commented Aug 9, 2016 • edited Loading

gabemontero commented Aug 9, 2016

bparees commented Aug 9, 2016

gabemontero commented Aug 9, 2016

bparees commented Aug 9, 2016

openshift-bot commented Aug 9, 2016

openshift-bot commented Aug 9, 2016 •

edited

Loading