status: Add more details for a broken dc trigger by 0xmichalis · Pull Request #6224 · openshift/origin

0xmichalis · 2015-12-07T11:57:20Z

Builds on changes from #6078

0xmichalis · 2015-12-07T12:00:19Z

[vagrant@localhost sample-app]$ oc status
In project test on server https://localhost:8443

svc/database - 172.30.16.62:5434 -> 3306
  dc/database deploys docker.io/openshift/mysql-55-centos7:latest 
    #1 deployment running for 15 seconds - 1 pod

svc/doesntmatter - 172.30.93.6:5432 -> 8080
  exposed by route/etcd-cruft

svc/frontend - 172.30.6.25:5432 -> 8080
  dc/frontend deploys imagestreamtag/origin-ruby-sample:latest <-
    bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with test/ruby-22-centos7:latest 
      not built yet
    #1 deployment waiting on image or update
  exposed by route/route-edge

Warnings:
  * The image trigger for dc/frontend will have no effect because imagestreamtag/origin-ruby-sample:latest does not exist but bc/ruby-sample-build points to imagestreamtag/origin-ruby-sample:latest.
  * route/etcd-cruft has a TLS configuration but no termination type specified.

2 warnings identified, use 'oc status -v' to see details.

[vagrant@localhost sample-app]$ oc status -v
In project test on server https://localhost:8443

svc/database - 172.30.16.62:5434 -> 3306
  dc/database deploys docker.io/openshift/mysql-55-centos7:latest 
    #1 deployed 33 seconds ago - 1 pod

svc/doesntmatter - 172.30.93.6:5432 -> 8080
  exposed by route/etcd-cruft

svc/frontend - 172.30.6.25:5432 -> 8080
  dc/frontend deploys imagestreamtag/origin-ruby-sample:latest <-
    bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with test/ruby-22-centos7:latest 
      not built yet
    #1 deployment waiting on image or update
  exposed by route/route-edge

Warnings:
  * The image trigger for dc/frontend will have no effect because imagestreamtag/origin-ruby-sample:latest does not exist but bc/ruby-sample-build points to imagestreamtag/origin-ruby-sample:latest.
    try: oc start-build bc/ruby-sample-build
  * route/etcd-cruft has a TLS configuration but no termination type specified.
    try: oc patch route/etcd-cruft -p '{"spec":{"tls":{"termination":"<type>"}}}' (replace <type> with a valid termination type: edge, passthrough, reencrypt)

To see more, use 'oc describe <resource>/<name>'.
You can use 'oc get all' to see a list of other objects.

smarterclayton · 2015-12-07T21:57:09Z

Without -v I wouldn't expect to see the list of warnings

On Mon, Dec 7, 2015 at 7:00 AM, Michail Kargakis notifications@github.com
wrote:

[vagrant@localhost sample-app]$ oc status
In project test on server https://localhost:8443

svc/database - 172.30.16.62:5434 -> 3306
dc/database deploys docker.io/openshift/mysql-55-centos7:latest
#1 deployment running for 15 seconds - 1 pod

svc/doesntmatter - 172.30.93.6:5432 -> 8080
exposed by route/etcd-cruft

svc/frontend - 172.30.6.25:5432 -> 8080
dc/frontend deploys imagestreamtag/origin-ruby-sample:latest <-
bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with test/ruby-22-centos7:latest
not built yet
#1 deployment waiting on image or update
exposed by route/route-edge

Warnings:

The image trigger for dc/frontend will have no effect because imagestreamtag/origin-ruby-sample:latest does not exist but bc/ruby-sample-build points to imagestreamtag/origin-ruby-sample:latest.

route/etcd-cruft has a TLS configuration but no termination type specified.

2 warnings identified, use 'oc status -v' to see details.

[vagrant@localhost sample-app]$ oc status -v
In project test on server https://localhost:8443

svc/database - 172.30.16.62:5434 -> 3306
dc/database deploys docker.io/openshift/mysql-55-centos7:latest
#1 deployed 33 seconds ago - 1 pod

svc/doesntmatter - 172.30.93.6:5432 -> 8080
exposed by route/etcd-cruft

svc/frontend - 172.30.6.25:5432 -> 8080
dc/frontend deploys imagestreamtag/origin-ruby-sample:latest <-
bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with test/ruby-22-centos7:latest
not built yet
#1 deployment waiting on image or update
exposed by route/route-edge

Warnings:

The image trigger for dc/frontend will have no effect because imagestreamtag/origin-ruby-sample:latest does not exist but bc/ruby-sample-build points to imagestreamtag/origin-ruby-sample:latest.
try: oc start-build bc/ruby-sample-build

route/etcd-cruft has a TLS configuration but no termination type specified.
try: oc patch route/etcd-cruft -p '{"spec":{"tls":{"termination":""}}}' (replace with a valid termination type: edge, passthrough, reencrypt)

To see more, use 'oc describe /'.
You can use 'oc get all' to see a list of other objects.

—
Reply to this email directly or view it on GitHub
#6224 (comment).

deads2k · 2015-12-08T13:15:09Z

Without -v I wouldn't expect to see the list of warnings

Ok, can you describe the distinction between warnings and errors? Right now, warnings mean "this thing isn't behaving how you think it will". The two examples shown above (bad dc trigger and bad tls termination on route), do you see them as errors or warnings?

smarterclayton · 2015-12-09T18:13:02Z

Two axis:

Usability: only nag me about fixing things that I must fix, or should
fix. If I may not want to fix it, and you nag me, it devalues the nag and
makes all other errors harder to read.

Warning vs Error: An error is "this is broken and we have no doubt". A
warning is "this might be broken but we're not positive".

On Tue, Dec 8, 2015 at 8:15 AM, David Eads notifications@github.com wrote:

Without -v I wouldn't expect to see the list of warnings

Ok, can you describe the distinction between warnings and errors? Right
now, warnings mean "this thing isn't behaving how you think it will". The
two examples shown above (bad dc trigger and bad tls termination on route),
do you see them as errors or warnings?

—
Reply to this email directly or view it on GitHub
#6224 (comment).

0xmichalis · 2015-12-14T12:27:58Z

@deads2k @ironcladlou @smarterclayton PTAL, I have added a couple of more cases and suggestions for dc triggers. Fixing #6281 too

0xmichalis · 2015-12-14T13:10:44Z

Needs a new unit test and another fixed

0xmichalis · 2015-12-16T12:17:31Z

[test]

0xmichalis · 2015-12-16T12:25:26Z

@deads2k @ironcladlou this is ready for reviewing.

0xmichalis · 2015-12-16T13:16:53Z

#6065 flake

[test]

deads2k · 2015-12-16T13:25:51Z

pkg/deploy/graph/analysis/dc.go

Why wouldn't I attach this sort of marker to the build config, instead of the deployment config?

Right, I updated both this error and the warning re a bc with no builds. ptal so I can squash

deads2k · 2015-12-16T13:28:21Z

implementation looks fine, just not sure of where I'd want the marker. Seems like I'd want it on the build config.

deads2k · 2015-12-16T15:25:35Z

pkg/deploy/graph/analysis/dc.go

I mean, wouldn't it make more sense to move this entire check to pkg/build/graph/analysis and make the marker there? Both the error and the help make sense there. The other marker (TagNotAvailable), will still make sense too, but that way I'll get help when all I have are builds or chained builds.

Moved build related issues

0xmichalis · 2015-12-17T13:41:13Z

$ oc status -v
In project test on server https://localhost:8443

svc/database - 172.30.60.90:5434 -> 3306
  dc/database deploys docker.io/openshift/mysql-55-centos7:latest 
    #1 deployment running for 9 seconds

svc/frontend - 172.30.95.119:5432 -> 8080
  dc/frontend deploys imagestreamtag/origin-ruby-sample:latest <-
    bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with test/ruby-22-centos7:latest 
      #1 build running for 4 seconds
    #1 deployment waiting on image or update
  exposed by route/route-edge

Warnings:
  * imagestreamtag/origin-ruby-sample:latest needs to be imported or created by a build.
  * The image trigger for dc/frontend will have no effect until imagestreamtag/origin-ruby-sample:latest is imported or created by a build.

View details with 'oc describe <resource>/<name>' or list everything with 'oc get all'.

deads2k · 2015-12-17T13:46:30Z

pkg/build/graph/analysis/bc.go

Why don't I see this suggestion in your output?

I have a running build there. Will post two more (one with this and one with a failed build).

I have a running build there. Will post two more (one with this and one with a failed build).

Hmmm... I hadn't considered that. Think we should skip the warning if a build is in progress?

Sounds fine. How about the dc trigger warning? Should we detect that a build is running and print something like Waiting on build/%s to complete for triggering a new deployment

Sounds fine. How about the dc trigger warning? Should we detect that a build is running and print something like Waiting on build/%s to complete for triggering a new deployment

Yes. Your choice on this pull or another pull.

0xmichalis · 2015-12-17T13:50:22Z

So my build just failed

$ oc status -v
In project test on server https://localhost:8443

svc/database - 172.30.60.90:5434 -> 3306
  dc/database deploys docker.io/openshift/mysql-55-centos7:latest 
    #1 deployment running for 9 minutes

svc/frontend - 172.30.95.119:5432 -> 8080
  dc/frontend deploys imagestreamtag/origin-ruby-sample:latest <-
    bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with test/ruby-22-centos7:latest 
      #1 build failed 48 seconds ago - 5868780: Merge pull request #49 from bparees/bump_version (Ben Parees <bparees@users.noreply.github.com>)
    #1 deployment waiting on image or update
  exposed by route/route-edge

Errors:
  * build/ruby-sample-build-1 has failed.
    try: Inspect the build failure with 'oc logs build/ruby-sample-build-1'
Warnings:
  * The image trigger for dc/frontend will have no effect until imagestreamtag/origin-ruby-sample:latest is imported or created by a build.

View details with 'oc describe <resource>/<name>' or list everything with 'oc get all'.

I delete it and now I have none

[vagrant@localhost sample-app]$ oc delete builds --all
build "ruby-sample-build-1" deleted

[vagrant@localhost sample-app]$ oc status -v
In project test on server https://localhost:8443

svc/database - 172.30.60.90:5434 -> 3306
  dc/database deploys docker.io/openshift/mysql-55-centos7:latest 
    #1 deployment running for 10 minutes

svc/frontend - 172.30.95.119:5432 -> 8080
  dc/frontend deploys imagestreamtag/origin-ruby-sample:latest <-
    bc/ruby-sample-build builds https://github.com/openshift/ruby-hello-world.git with test/ruby-22-centos7:latest 
      not built yet
    #1 deployment waiting on image or update
  exposed by route/route-edge

Warnings:
  * imagestreamtag/origin-ruby-sample:latest needs to be imported or created by a build.
    try: oc start-build bc/ruby-sample-build
  * The image trigger for dc/frontend will have no effect until imagestreamtag/origin-ruby-sample:latest is imported or created by a build.

View details with 'oc describe <resource>/<name>' or list everything with 'oc get all'.

deads2k · 2015-12-17T13:59:25Z

@openshift/ui-review Adding more linkage checking and debugging help to oc status. I think the web console already does this.

deads2k · 2015-12-17T14:04:34Z

@stevekuznetsov take a pass?

stevekuznetsov · 2015-12-17T14:08:42Z

pkg/build/graph/analysis/bc.go

Why do we need to use this syntax? continue will jump to the next iteration of the for loop as-is.

Every time we hit a marker for an object, there is no need to continue inspecting it (assuming markers in the same unit are placed in order of precedence).

@stevekuznetsov you are right, the label was primarily used in other places where we are using nested loops. Will remove it here.

Every time we hit a marker for an object, there is no need to continue inspecting it (assuming markers in the same unit are placed in order of precedence).

I wouldn't expect a continue inside a switch to require the label. break would, but I don't think continue does.

stevekuznetsov · 2015-12-17T14:18:43Z

If all -v is doing is adding the try: messages to warnings already shown by oc status, it seems unnecessary, we might as well show both.

deads2k · 2015-12-17T14:22:43Z

If all -v is doing is adding the try: messages to warnings already shown by oc status, it seems unnecessary, we might as well show both.

eh, wrong jungle.

stevekuznetsov · 2015-12-17T14:23:10Z

pkg/build/graph/analysis/bc.go

Godoc with list of precedence would be nice here

stevekuznetsov · 2015-12-17T14:27:00Z

Two small nits, but otherwise LGTM.

@deads2k understood

0xmichalis · 2015-12-17T14:27:51Z

I have updated the build analysis to take into account {new,pending,running} builds and not warn for them. Have another look in FindPendingTags. Will add godoc

stevekuznetsov · 2015-12-17T14:33:22Z

pkg/build/graph/analysis/bc.go

Should we raise a warning here as well telling them to re-start the build?

When the build is complete we should have the tag populated so we should never hit this. Or even if we do, it's probably a race and we should do nothing I think.

deads2k · 2015-12-17T14:42:42Z

pkg/build/graph/analysis/bc.go

Shouldn't this return a *buildgraph.BuildConfigNode?

I don't really care about these helpers, will change to specific node kinds.

0xmichalis · 2015-12-17T14:43:06Z

Comments addressed, squashed

I will update the dc trigger warning in a follow-up

deads2k · 2015-12-17T14:46:19Z

pkg/build/graph/analysis/bc.go

nit: rename to latestBuild? That helps me remember which build we're attaching the marker to.

deads2k · 2015-12-17T15:11:01Z

Minor comments on types and anonymous inclusion. lgtm otherwise.

If there is no build running for an istag where the user has placed an ICT, suggest oc start-build or oc logs.

openshift-bot · 2015-12-17T16:05:38Z

Evaluated for origin test up to 7f1d28e

deads2k · 2015-12-17T16:10:24Z

lgtm [merge]

openshift-bot · 2015-12-17T16:15:40Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4397/) (Image: devenv-rhel7_2983)

openshift-bot · 2015-12-17T16:15:40Z

Evaluated for origin merge up to 7f1d28e

openshift-bot · 2015-12-17T17:07:02Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7914/)

Merged by openshift-bot

openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 11, 2015

0xmichalis changed the title ~~Promote info marker - suggest oc start-build~~ status: Add more details for a broken dc trigger Dec 14, 2015

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2015

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2015

deads2k reviewed Dec 16, 2015
View reviewed changes

deads2k reviewed Dec 17, 2015
View reviewed changes

stevekuznetsov reviewed Dec 17, 2015
View reviewed changes

deads2k reviewed Dec 17, 2015
View reviewed changes

status: Add more details for a broken dc trigger

7f1d28e

If there is no build running for an istag where the user has placed an ICT, suggest oc start-build or oc logs.

openshift-bot pushed a commit that referenced this pull request Dec 18, 2015

Merge pull request #6224 from kargakis/promote-info-marker

45d4849

Merged by openshift-bot

openshift-bot merged commit 45d4849 into openshift:master Dec 18, 2015

0xmichalis deleted the promote-info-marker branch December 18, 2015 10:31

Conversation

0xmichalis commented Dec 7, 2015

Uh oh!

0xmichalis commented Dec 7, 2015

Uh oh!

smarterclayton commented Dec 7, 2015

Uh oh!

deads2k commented Dec 8, 2015

Uh oh!

smarterclayton commented Dec 9, 2015

Uh oh!

0xmichalis commented Dec 14, 2015

Uh oh!

0xmichalis commented Dec 14, 2015

Uh oh!

0xmichalis commented Dec 16, 2015

Uh oh!

0xmichalis commented Dec 16, 2015

Uh oh!

0xmichalis commented Dec 16, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deads2k commented Dec 16, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xmichalis commented Dec 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xmichalis commented Dec 17, 2015

Uh oh!

deads2k commented Dec 17, 2015

Uh oh!

deads2k commented Dec 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevekuznetsov commented Dec 17, 2015

Uh oh!

deads2k commented Dec 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevekuznetsov commented Dec 17, 2015

Uh oh!

0xmichalis commented Dec 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xmichalis commented Dec 17, 2015

Uh oh!

Choose a reason for hiding this comment