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

2018-04-19 Thread Baptiste Mathus
Just submitted the PR as draft as https://github.com/jenkinsci/jep/pull/91

Feedback still more than welcome.

Cheers

2018-04-11 18:55 GMT+02:00 Baptiste Mathus :

> Good point. I just addressed it in the proposal and the Essentials tests.
>
> The proposal is updated: https://github.com/batmat/jep/tree/JENKINS-50294/
> jep/
> 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.
>
> 2018-04-11 15:48 GMT+02:00 Jesse Glick :
>
>> On Wed, Apr 11, 2018 at 9:05 AM, James Nord  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`).
>>
>> --
>> 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+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/ms
>> gid/jenkinsci-dev/CANfRfr3iufgV6J5eSf72tYk87ZXE%3DfBWk-
>> d3WTfuME_0jmrBTA%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS5Y5b%2B2u7cVvwtpfYot6UpmOpQCDG3NbR%3D3UKz%2BhDo8pg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


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

2018-04-11 Thread Baptiste Mathus
Good point. I just addressed it in the proposal and the Essentials tests.

The proposal is updated:
https://github.com/batmat/jep/tree/JENKINS-50294/jep/
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.

2018-04-11 15:48 GMT+02:00 Jesse Glick :

> On Wed, Apr 11, 2018 at 9:05 AM, James Nord  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`).
>
> --
> 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+unsubscr...@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.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS4_KqMHRYRz4kEUuRRSYPeDUan%3DKyy01JwJpLHeiTcJ_g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


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

2018-04-11 Thread Jesse Glick
On Wed, Apr 11, 2018 at 9:05 AM, James Nord  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`).

-- 
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+unsubscr...@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.
For more options, visit https://groups.google.com/d/optout.


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

2018-04-11 Thread James Nord

>
> 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


-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/38b68e1c-c962-4cd0-947d-a0228672598f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


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

2018-04-10 Thread Liam Newman
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  wrote:

> 2018-04-10 16:33 GMT+02:00 R. Tyler Croy :
>
>> (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+unsubscr...@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.
>>
> --
> 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+unsubscr...@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
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CAA0qCNxaUqBC6ckV-BE9qn%3DabQpoZqKMTE3oQ%3Di%3DGpqHkWG6Tw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


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

2018-04-10 Thread Jesse Glick
On Tue, Apr 10, 2018 at 10:33 AM, R. Tyler Croy  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.

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr0df4JRZiyr0hEgHxzvTyw0XL3JDk1OMRgZdHnyuGzQcw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


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

2018-04-10 Thread R. Tyler Croy
(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.




-- 
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+unsubscr...@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.


signature.asc
Description: PGP signature


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

2018-04-10 Thread Jesse Glick
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.

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr2U160HAC7VTSe%2B4UXz2TMokTmBnxFVNhbwD-CtZG_%2BZA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


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

2018-04-10 Thread Baptiste Mathus
Sorry, the right link is
https://github.com/batmat/jep/tree/JENKINS-50294/jep/

2018-04-10 14:09 GMT+02:00 Baptiste Mathus :

> Hello everyone,
>
> Just published the draft of the JEP for the "Essentials Instance Client
> Health Checking", it is ready for, and warmly welcoming, review:
>
> https://github.com/batmat/jep/blob/JENKINS-50294/README.adoc
>
> 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!
>
> PS: context is https://issues.jenkins-ci.org/browse/JENKINS-50294
>

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS5_CWd5nfzXoivoy%3D%3DoUrDMwc-6KQ%2B9Ji_%3DJ7zmPZmMEw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


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

2018-04-10 Thread Baptiste Mathus
Hello everyone,

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

https://github.com/batmat/jep/blob/JENKINS-50294/README.adoc

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!

PS: context is https://issues.jenkins-ci.org/browse/JENKINS-50294

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS55c6LXzLOx1wyW0h%3D2%2Br0DyQejLesbNUKP3u-VNF6VRQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.