[Essentials] Instance Health Checking JEP: ready for first review

38 views
Skip to first unread message

Baptiste Mathus

unread,
Apr 10, 2018, 8:09:40 AM4/10/18
to Jenkins Developers
Hello everyone,

Just published the draft of the JEP for the "Essentials Instance Client Health Checking", it is ready for, and warmly welcoming, review:


To sum up, this will be the component used to decide if we trigger a rollback or not after we ran an upgrade. (in which case, we will roll back using jep-302).

Please answer with your feedback in this thread as much as possible.

Thank you!

Baptiste Mathus

unread,
Apr 10, 2018, 8:57:20 AM4/10/18
to Jenkins Developers

Jesse Glick

unread,
Apr 10, 2018, 10:07:42 AM4/10/18
to Jenkins Dev
I for one would find it easier to comment on a PR, but IIRC JEP
editors have discouraged this for some reason…?


“Healthness” is not a word—it is just “health”.


I would suggest `/metrics/evergreen/healthcheck` just return an empty
JSON document, or a simple “OK” marker by default—only listing things
that are _not_ healthy.

R. Tyler Croy

unread,
Apr 10, 2018, 10:33:54 AM4/10/18
to jenkin...@googlegroups.com
(replies inline)

On Tue, 10 Apr 2018, Jesse Glick wrote:

> I for one would find it easier to comment on a PR, but IIRC JEP
> editors have discouraged this for some reason????
>
>
> ???Healthness??? is not a word???it is just ???health???.
>
>
> I would suggest `/metrics/evergreen/healthcheck` just return an empty
> JSON document, or a simple ???OK??? marker by default???only listing things
> that are _not_ healthy.

It was my understanding that this format was simply the Dropwizard healthcheck
format.

Do you have some reasoning for this you could share? Otherwise this seems like
a bit of personal preference to me, but I'm sure there's some logic I could be
missing here.




signature.asc

Jesse Glick

unread,
Apr 10, 2018, 11:03:10 AM4/10/18
to Jenkins Dev
On Tue, Apr 10, 2018 at 10:33 AM, R. Tyler Croy <ty...@monkeypox.org> wrote:
> Do you have some reasoning for this you could share?

Merely that “nothing in particular is broken” is not news, so there is
not much a client needs to know.

Baptiste Mathus

unread,
Apr 10, 2018, 3:41:04 PM4/10/18
to Jenkins Developers
2018-04-10 16:33 GMT+02:00 R. Tyler Croy <ty...@monkeypox.org>:
(replies inline)

On Tue, 10 Apr 2018, Jesse Glick wrote:

> I for one would find it easier to comment on a PR, but IIRC JEP
> editors have discouraged this for some reason????
>
>
> ???Healthness??? is not a word???it is just ???health???.
>
>
> I would suggest `/metrics/evergreen/healthcheck` just return an empty
> JSON document, or a simple ???OK??? marker by default???only listing things
> that are _not_ healthy.

It was my understanding that this format was simply the Dropwizard healthcheck
format.

Yes. Exactly. Also, though TBH I didn't test it myself yet, I expect one can contribute healthchecks, and they'd appear there.

That "OK" idea is already somehow available if one enables the `canPing` configuration field. Then `curl`ing the URL will reply with just PONG and http-200 IIRC. But I felt this was unnecessary to enable that too.
 

Do you have some reasoning for this you could share? Otherwise this seems like
a bit of personal preference to me, but I'm sure there's some logic I could be
missing here.




--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/20180410143346.discdbodxgiudmfz%40grape.lasagna.io.
For more options, visit https://groups.google.com/d/optout.

Liam Newman

unread,
Apr 10, 2018, 7:47:28 PM4/10/18
to jenkin...@googlegroups.com
Jesse, 
Thanks for the reminder about that point around commenting.

As an editor, I have no objections to having pre-draft discussions in whatever way contributors see fit.   

As a sponsor of JEP-1, it is important to me that when a sponsor submits a JEP for "Approval as Draft", that feedback they get on _that_ PR focuses on issues that would prevent the JEP being approved as draft rather than on design or other questions.  I'll add a note about this to  https://github.com/jenkinsci/jep/pull/76 .  

-Liam




On Tue, Apr 10, 2018 at 12:41 PM Baptiste Mathus <m...@batmat.net> wrote:
2018-04-10 16:33 GMT+02:00 R. Tyler Croy <ty...@monkeypox.org>:
(replies inline)

On Tue, 10 Apr 2018, Jesse Glick wrote:

> I for one would find it easier to comment on a PR, but IIRC JEP
> editors have discouraged this for some reason????
>
>
> ???Healthness??? is not a word???it is just ???health???.
>
>
> I would suggest `/metrics/evergreen/healthcheck` just return an empty
> JSON document, or a simple ???OK??? marker by default???only listing things
> that are _not_ healthy.

It was my understanding that this format was simply the Dropwizard healthcheck
format.

Yes. Exactly. Also, though TBH I didn't test it myself yet, I expect one can contribute healthchecks, and they'd appear there.

That "OK" idea is already somehow available if one enables the `canPing` configuration field. Then `curl`ing the URL will reply with just PONG and http-200 IIRC. But I felt this was unnecessary to enable that too.

Do you have some reasoning for this you could share? Otherwise this seems like
a bit of personal preference to me, but I'm sure there's some logic I could be
missing here.




--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS6FcSp_faJL5C26-xESdUrSQvw%3D40%2BGVxf-SkWM5TOvQQ%40mail.gmail.com.

James Nord

unread,
Apr 11, 2018, 9:05:01 AM4/11/18
to Jenkins Developers

Login URL

We check that:

  • it is reachable,

  • and returns a 200 HTTP status code.


I think that is potentially a bad idea.  Whilst currently the /login page returns a 200 it should only do this if you are not logged in and are using an AbstractPasswordBasedSecurityRealm based SecurityReakm.  For a non username password provider (google/SAML etc) IMO it should redirect to the actual login url AuthRealm  (link).

I consider the current behaviour of showing a username/password form a bug if you are either logged in or are not using something that can actually authenticate with a username and password. 

Regards

/James


Jesse Glick

unread,
Apr 11, 2018, 9:48:23 AM4/11/18
to Jenkins Dev
On Wed, Apr 11, 2018 at 9:05 AM, James Nord <james...@gmail.com> wrote:
> Whilst currently the /login page
> returns a 200 it should only do this if you are not logged in and are using
> an AbstractPasswordBasedSecurityRealm based SecurityRealm.

Good point, this is not a good choice of page for a general-purpose
anonymous HTML rendering check. I would suggest `/instance-identity/`
(from `IdentityRootAction/index.jelly`). You could also use `/whoAmI/`
(from `WhoAmI/index.jelly`).

Baptiste Mathus

unread,
Apr 11, 2018, 12:56:17 PM4/11/18
to Jenkins Developers
Good point. I just addressed it in the proposal and the Essentials tests.

FTR, the associated PR on Essentials side: https://github.com/jenkins-infra/evergreen/pull/52

I chose to go with /instance-identity, because I thought it might become useful in the near future to extract/check the instance-identity, so it looked like a better guess than /whoAmI where we always get the page for anonymous.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr3iufgV6J5eSf72tYk87ZXE%3DfBWk-d3WTfuME_0jmrBTA%40mail.gmail.com.

Baptiste Mathus

unread,
Apr 19, 2018, 11:34:06 AM4/19/18
to Jenkins Developers
Just submitted the PR as draft as https://github.com/jenkinsci/jep/pull/91

Feedback still more than welcome.

Cheers
Reply all
Reply to author
Forward
0 new messages