Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update missing probe severity to info #11968

Conversation

juanvallejo
Copy link
Contributor

Fixes #10294

This patch changes the marker severity of missing probes (liveness and readiness) from osgraph.WarningSeverity to osgraph.InfoSeverity. It also adds a linebreak between Errors, Warnings, and Infos.

Example

$ oc status -v
In project default on server https://10.13.137.149:8443

http://www.pictre.org (svc/frontend)

https://idling-echo-reencrypt-default.router.default.svc.cluster.local (reencrypt) (svc/idling-echo)
http://idling-echo-default.router.default.svc.cluster.local
  dc/idling-echo deploys docker.io/openshift/node:latest
    deployment #1 deployed 2 days ago - 3 pods

svc/kubernetes - 172.30.0.1 ports 443, 53->8053, 53->8053

dc/test-deployment-config deploys docker.io/openshift/origin-pod:latest
  deployment #1 deployed 2 days ago - 1 pod

bc/test-buildconfig source builds git://github.com/openshift/ruby-hello-world.git on docker.io/centos/ruby-22-centos7:latest
  not built yet

Errors:
  * route/foo is routing traffic to svc/foo, but either the administrator has not installed a router or the router is not selecting this route.
    try: oc adm router -h
  * route/idling-echo is routing traffic to svc/idling-echo, but either the administrator has not installed a router or the router is not selecting this route.
    try: oc adm router -h
  * route/idling-echo-reencrypt is routing traffic to svc/idling-echo, but either the administrator has not installed a router or the router is not selecting this route.
    try: oc adm router -h
  * route/my-route is routing traffic to svc/frontend, but either the administrator has not installed a router or the router is not selecting this route.
    try: oc adm router -h

Warnings:
  * route/foo is supposed to route traffic to svc/foo but svc/foo doesn't exist.
  * route/idling-echo doesn't have a port specified and is routing traffic to svc/idling-echo which uses multiple ports.
  * route/idling-echo-reencrypt doesn't have a port specified and is routing traffic to svc/idling-echo which uses multiple ports.

Infos:
  * dc/idling-echo has no readiness probe to verify pods are ready to accept traffic or ensure deployment is successful.
    try: oc set probe dc/idling-echo --readiness ...
  * dc/idling-echo has no liveness probe to verify pods are still running.
    try: oc set probe dc/idling-echo --liveness ...
  * dc/test-deployment-config has no readiness probe to verify pods are ready to accept traffic or ensure deployment is successful.
    try: oc set probe dc/test-deployment-config --readiness ...
  * dc/test-deployment-config has no liveness probe to verify pods are still running.
    try: oc set probe dc/test-deployment-config --liveness ...

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

cc @jwforres @deads2k @fabianofranz

@juanvallejo juanvallejo force-pushed the jvallejo/update-missing-probe-severity-to-info branch from 9cdb35a to 3b232f2 Compare November 18, 2016 17:44
@juanvallejo
Copy link
Contributor Author

[test]

@jwforres
Copy link
Member

I would use "Info:" for the header not "Infos:" you wouldn't say
Informations, you would say Information

On Fri, Nov 18, 2016 at 12:42 PM, Juan Vallejo notifications@github.com
wrote:

Fixes #10294 #10294

This patch changes the marker severity of missing probes (liveness and
readiness) from osgraph.WarningSeverity to osgraph.InfoSeverity. It also
adds a linebreak between Errors, Warnings, and Infos.

Example

$ oc status -v
In project default on server https://10.13.137.149:8443
http://www.pictre.org (svc/frontend)
https://idling-echo-reencrypt-default.router.default.svc.cluster.local (reencrypt) (svc/idling-echo)http://idling-echo-default.router.default.svc.cluster.local
dc/idling-echo deploys docker.io/openshift/node:latest
deployment #1 deployed 2 days ago - 3 pods

svc/kubernetes - 172.30.0.1 ports 443, 53->8053, 53->8053

dc/test-deployment-config deploys docker.io/openshift/origin-pod:latest
deployment #1 deployed 2 days ago - 1 pod

bc/test-buildconfig source builds git://github.com/openshift/ruby-hello-world.git on docker.io/centos/ruby-22-centos7:latest
not built yet

Errors:

  • route/foo is routing traffic to svc/foo, but either the administrator has not installed a router or the router is not selecting this route.
    try: oc adm router -h
  • route/idling-echo is routing traffic to svc/idling-echo, but either the administrator has not installed a router or the router is not selecting this route.
    try: oc adm router -h
  • route/idling-echo-reencrypt is routing traffic to svc/idling-echo, but either the administrator has not installed a router or the router is not selecting this route.
    try: oc adm router -h
  • route/my-route is routing traffic to svc/frontend, but either the administrator has not installed a router or the router is not selecting this route.
    try: oc adm router -h

Warnings:

  • route/foo is supposed to route traffic to svc/foo but svc/foo doesn't exist.
  • route/idling-echo doesn't have a port specified and is routing traffic to svc/idling-echo which uses multiple ports.
  • route/idling-echo-reencrypt doesn't have a port specified and is routing traffic to svc/idling-echo which uses multiple ports.

Infos:

  • dc/idling-echo has no readiness probe to verify pods are ready to accept traffic or ensure deployment is successful.
    try: oc set probe dc/idling-echo --readiness ...
  • dc/idling-echo has no liveness probe to verify pods are still running.
    try: oc set probe dc/idling-echo --liveness ...
  • dc/test-deployment-config has no readiness probe to verify pods are ready to accept traffic or ensure deployment is successful.
    try: oc set probe dc/test-deployment-config --readiness ...
  • dc/test-deployment-config has no liveness probe to verify pods are still running.
    try: oc set probe dc/test-deployment-config --liveness ...

View details with 'oc describe /' or list everything with 'oc get all'.

cc @jwforres https://github.com/jwforres @deads2k
https://github.com/deads2k @fabianofranz

https://github.com/fabianofranz

You can view, comment on, or merge this pull request online at:

#11968
Commit Summary

  • update missing probe severity to info

File Changes

Patch Links:


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11968, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABZk7cWR5QexfCdPXb1HNiytBlSB7l0-ks5q_eOjgaJpZM4K2viZ
.

@juanvallejo juanvallejo force-pushed the jvallejo/update-missing-probe-severity-to-info branch from 3b232f2 to 6378793 Compare November 18, 2016 18:04
@juanvallejo
Copy link
Contributor Author

@jwforres

I would use "Info:" for the header not "Infos:" you wouldn't say
Informations, you would say Information

Done! Do you think Notices or something along those lines would sound better instead of Info?

@jwforres
Copy link
Member

Hmm that would probably be OK too, I don't have a strong opinion either way.

On Fri, Nov 18, 2016 at 1:05 PM, Juan Vallejo notifications@github.com
wrote:

@jwforres https://github.com/jwforres

I would use "Info:" for the header not "Infos:" you wouldn't say
Informations, you would say Information

Done! Do you think Notices or something along those lines would sound
better instead of Info?


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

@@ -304,6 +304,10 @@ func (d *ProjectStatusDescriber) Describe(namespace, name string) (string, error
warningMarkers := allMarkers.BySeverity(osgraph.WarningSeverity)
if len(warningMarkers) > 0 {
if d.Suggest {
// add linebreak between Errors list and Warnings list
if len(errorMarkers) > 0 {
fmt.Fprintln(out, "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, fmt.Fprintln(out)

if d.Suggest {
// add linebreak between Warnings list and Info List
if len(warningMarkers) > 0 || len(warningMarkers) > 0 {
fmt.Fprintln(out, "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@fabianofranz
Copy link
Member

LGTM, no strong opinion about the wording. Let me know when this is good to go.

@juanvallejo juanvallejo force-pushed the jvallejo/update-missing-probe-severity-to-info branch from 6378793 to 55bae56 Compare November 18, 2016 18:25
@juanvallejo
Copy link
Contributor Author

@fabianofranz thanks for the review, almost good to go, added a printMarkerSuggestions function to avoid duplicating code for Errors, Warnings, and Info

@juanvallejo juanvallejo force-pushed the jvallejo/update-missing-probe-severity-to-info branch from 0592719 to 106c43d Compare November 18, 2016 18:47
@juanvallejo
Copy link
Contributor Author

integration check flaked on #9203 re[test]

if len(infoMarkers) > 0 {
if d.Suggest {
// add linebreak between Warnings list and Info List
if len(warningMarkers) > 0 || len(warningMarkers) > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-check just in case. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) Thanks for catching that

@juanvallejo juanvallejo force-pushed the jvallejo/update-missing-probe-severity-to-info branch from 106c43d to 2ce9162 Compare November 18, 2016 21:34
@fabianofranz
Copy link
Member

LGTM [merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 2ce9162

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 2ce9162

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11572/) (Base Commit: f056a5b)

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 18, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11573/) (Base Commit: 271fff8) (Image: devenv-rhel7_5387)

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS

@openshift-bot openshift-bot merged commit 3b2bbe5 into openshift:master Nov 19, 2016
@juanvallejo juanvallejo deleted the jvallejo/update-missing-probe-severity-to-info branch November 21, 2016 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants