RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov
Please review this simple change that makes the error message in
jtreg-ext/requires/VMProps.java conditional
on java property "vmprops.trace.verbose". W/o this condition an error
message is printed each time any jtreg
test is ran, including non-docker tests.

     JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
     Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/



======= Background and details:
VMProps.java is called for each test run of jtreg (be it a run
containing a singe test or multiple tests).
VMProps evaluates the @requires properties. Therefore, in case of this
bug, any time a user runs any jtreg VM test
on a machine w/o an operational docker engine, this error will pop up.
The fix makes printing of this error message conditional, off by
default. The message can be turend on
via a property vmprops.trace.verbose when detailed diagnostic info is
desired.



======== Testing:
Local testing:
   W/o docker present:
     jtreg <arbitrary-jtreg-test>
       No error message
     jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
       Detailed error message
     jtreg DockerBasicTest.java
       Test skipped

   With docker installed but no permissions:
     jtreg <arbitrary-jtreg-test>
       No error message
     jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
       Detailed error message
     jtreg DockerBasicTest.java
       Test skipped

   With docker installed and operational:
     jtreg <arbitrary-jtreg-test>
       No error message
     jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
       No error message
     jtreg DockerBasicTest.java
       Test passed

Testing via automated distributed test system:
   - tier1
   - docker tests
   In progress


Thank you,
Misha

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

David Holmes
Hi Misha,

I don't see anything else in VMProps that writes anything out so to me
the fix is to delete the System.err.println completely.

That said I did not look at this in the past but I feel the whole docker
test here is not really appropriate/suitable to always be run on
linux-x64. It has to exec another process and wait up to 10 seconds to
get a result! Can't this actually be done via a WhiteBox check once
Bob's container support updates are in place?

Thanks,
David

On 3/11/2017 7:03 AM, mikhailo wrote:

> Please review this simple change that makes the error message in
> jtreg-ext/requires/VMProps.java conditional
> on java property "vmprops.trace.verbose". W/o this condition an error
> message is printed each time any jtreg
> test is ran, including non-docker tests.
>
>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>
>
>
> ======= Background and details:
> VMProps.java is called for each test run of jtreg (be it a run
> containing a singe test or multiple tests).
> VMProps evaluates the @requires properties. Therefore, in case of this
> bug, any time a user runs any jtreg VM test
> on a machine w/o an operational docker engine, this error will pop up.
> The fix makes printing of this error message conditional, off by
> default. The message can be turend on
> via a property vmprops.trace.verbose when detailed diagnostic info is
> desired.
>
>
>
> ======== Testing:
> Local testing:
>    W/o docker present:
>      jtreg <arbitrary-jtreg-test>
>        No error message
>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>        Detailed error message
>      jtreg DockerBasicTest.java
>        Test skipped
>
>    With docker installed but no permissions:
>      jtreg <arbitrary-jtreg-test>
>        No error message
>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>        Detailed error message
>      jtreg DockerBasicTest.java
>        Test skipped
>
>    With docker installed and operational:
>      jtreg <arbitrary-jtreg-test>
>        No error message
>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>        No error message
>      jtreg DockerBasicTest.java
>        Test passed
>
> Testing via automated distributed test system:
>    - tier1
>    - docker tests
>    In progress
>
>
> Thank you,
> Misha
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov
Hi David,

   Thank you for taking a look at this change.


On 11/02/2017 04:47 PM, David Holmes wrote:
> Hi Misha,
>
> I don't see anything else in VMProps that writes anything out so to me
> the fix is to delete the System.err.println completely.
Removal is an option, and seems a simpler one. However, we will loose
the diagnostic information.
On the other hand, if an issue arises on a specific host or system, I
can patch this file with extra trace statements, and diagnose the
failure that way.
If you think removing the print statement is a better option, and I do
not hear objections or other opinions, I can just remove the print
statement.

> That said I did not look at this in the past but I feel the whole
> docker test here is not really appropriate/suitable to always be run
> on linux-x64. It has to exec another process and wait up to 10 seconds
> to get a result! Can't this actually be done via a WhiteBox check once
> Bob's container support updates are in place?
Actually, WhiteBox is not necessary for such check. The ability to run
docker on a given host/node can be determined in a body of a jtreg test,
via a test utility. In fact, that was my original intent. When this
change was presented for a discussion, this aspect was debated and
reviewers have reached a consensus to do such check in VMProps.java. We
mainly discussed within VM SQE team. The main argument was that using
@requires is a more proper way of "skipping" the test(s) on unsupported
environments, rather then checking this in the body of the test and then
returning. When using @requires, the test execution system will report
the test as "not run". When skipping the test using "return" from the
body of a test, the execution system will report test as "ran" and
passed; jtreg does not yet support the "skipped" state.

Unfortunately, the way our "@requires" mechanism works is that it runs
each time prior to starting a test run, even if the test being run is
not concerned with a specific property. It was my concern that this
could be a burden on the test run startup, but I eventually agreed to
the opinion of other reviewers.

If you have a strong opinion or concern about this, let me know.
However, I am not sure what is the best process to use to overturn a
decision on a prior review or design discussion. I guess one option is
to file a bug or an RFE, and bring it up for a discussion.


For this change under review, if I do not hear opinions from other
reviewers, I can simply delete the print statement.


Thank you,
Misha

>
> Thanks,
> David
>
> On 3/11/2017 7:03 AM, mikhailo wrote:
>> Please review this simple change that makes the error message in
>> jtreg-ext/requires/VMProps.java conditional
>> on java property "vmprops.trace.verbose". W/o this condition an error
>> message is printed each time any jtreg
>> test is ran, including non-docker tests.
>>
>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>
>>
>>
>> ======= Background and details:
>> VMProps.java is called for each test run of jtreg (be it a run
>> containing a singe test or multiple tests).
>> VMProps evaluates the @requires properties. Therefore, in case of
>> this bug, any time a user runs any jtreg VM test
>> on a machine w/o an operational docker engine, this error will pop up.
>> The fix makes printing of this error message conditional, off by
>> default. The message can be turend on
>> via a property vmprops.trace.verbose when detailed diagnostic info is
>> desired.
>>
>>
>>
>> ======== Testing:
>> Local testing:
>>    W/o docker present:
>>      jtreg <arbitrary-jtreg-test>
>>        No error message
>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>        Detailed error message
>>      jtreg DockerBasicTest.java
>>        Test skipped
>>
>>    With docker installed but no permissions:
>>      jtreg <arbitrary-jtreg-test>
>>        No error message
>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>        Detailed error message
>>      jtreg DockerBasicTest.java
>>        Test skipped
>>
>>    With docker installed and operational:
>>      jtreg <arbitrary-jtreg-test>
>>        No error message
>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>        No error message
>>      jtreg DockerBasicTest.java
>>        Test passed
>>
>> Testing via automated distributed test system:
>>    - tier1
>>    - docker tests
>>    In progress
>>
>>
>> Thank you,
>> Misha
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

David Holmes
Hi Misha,

On 3/11/2017 10:22 AM, mikhailo wrote:

> Hi David,
>
>    Thank you for taking a look at this change.
>
>
> On 11/02/2017 04:47 PM, David Holmes wrote:
>> Hi Misha,
>>
>> I don't see anything else in VMProps that writes anything out so to me
>> the fix is to delete the System.err.println completely.
> Removal is an option, and seems a simpler one. However, we will loose
> the diagnostic information.
> On the other hand, if an issue arises on a specific host or system, I
> can patch this file with extra trace statements, and diagnose the
> failure that way.
> If you think removing the print statement is a better option, and I do
> not hear objections or other opinions, I can just remove the print
> statement.

Delete please :)

>> That said I did not look at this in the past but I feel the whole
>> docker test here is not really appropriate/suitable to always be run
>> on linux-x64. It has to exec another process and wait up to 10 seconds
>> to get a result! Can't this actually be done via a WhiteBox check once
>> Bob's container support updates are in place?
> Actually, WhiteBox is not necessary for such check. The ability to run
> docker on a given host/node can be determined in a body of a jtreg test,
> via a test utility. In fact, that was my original intent. When this
> change was presented for a discussion, this aspect was debated and
> reviewers have reached a consensus to do such check in VMProps.java. We
> mainly discussed within VM SQE team. The main argument was that using
> @requires is a more proper way of "skipping" the test(s) on unsupported
> environments, rather then checking this in the body of the test and then
> returning. When using @requires, the test execution system will report
> the test as "not run". When skipping the test using "return" from the
> body of a test, the execution system will report test as "ran" and
> passed; jtreg does not yet support the "skipped" state.
>
> Unfortunately, the way our "@requires" mechanism works is that it runs
> each time prior to starting a test run, even if the test being run is
> not concerned with a specific property. It was my concern that this
> could be a burden on the test run startup, but I eventually agreed to
> the opinion of other reviewers.
>
> If you have a strong opinion or concern about this, let me know.
> However, I am not sure what is the best process to use to overturn a
> decision on a prior review or design discussion. I guess one option is
> to file a bug or an RFE, and bring it up for a discussion.

Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using
WhiteBox connected to Bob's new container support code is better than
exec'ing a separate process (which I'm not even sure what it does).

>
> For this change under review, if I do not hear opinions from other
> reviewers, I can simply delete the print statement.

Sounds good to me.

Thanks,
David

>
> Thank you,
> Misha
>>
>> Thanks,
>> David
>>
>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>> Please review this simple change that makes the error message in
>>> jtreg-ext/requires/VMProps.java conditional
>>> on java property "vmprops.trace.verbose". W/o this condition an error
>>> message is printed each time any jtreg
>>> test is ran, including non-docker tests.
>>>
>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>
>>>
>>>
>>> ======= Background and details:
>>> VMProps.java is called for each test run of jtreg (be it a run
>>> containing a singe test or multiple tests).
>>> VMProps evaluates the @requires properties. Therefore, in case of
>>> this bug, any time a user runs any jtreg VM test
>>> on a machine w/o an operational docker engine, this error will pop up.
>>> The fix makes printing of this error message conditional, off by
>>> default. The message can be turend on
>>> via a property vmprops.trace.verbose when detailed diagnostic info is
>>> desired.
>>>
>>>
>>>
>>> ======== Testing:
>>> Local testing:
>>>    W/o docker present:
>>>      jtreg <arbitrary-jtreg-test>
>>>        No error message
>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>        Detailed error message
>>>      jtreg DockerBasicTest.java
>>>        Test skipped
>>>
>>>    With docker installed but no permissions:
>>>      jtreg <arbitrary-jtreg-test>
>>>        No error message
>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>        Detailed error message
>>>      jtreg DockerBasicTest.java
>>>        Test skipped
>>>
>>>    With docker installed and operational:
>>>      jtreg <arbitrary-jtreg-test>
>>>        No error message
>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>        No error message
>>>      jtreg DockerBasicTest.java
>>>        Test passed
>>>
>>> Testing via automated distributed test system:
>>>    - tier1
>>>    - docker tests
>>>    In progress
>>>
>>>
>>> Thank you,
>>> Misha
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov
Hi David,

Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/

Please see my comments below.

On 11/2/17, 6:02 PM, David Holmes wrote:

> Hi Misha,
>
> On 3/11/2017 10:22 AM, mikhailo wrote:
>> Hi David,
>>
>>    Thank you for taking a look at this change.
>>
>>
>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>> Hi Misha,
>>>
>>> I don't see anything else in VMProps that writes anything out so to
>>> me the fix is to delete the System.err.println completely.
>> Removal is an option, and seems a simpler one. However, we will loose
>> the diagnostic information.
>> On the other hand, if an issue arises on a specific host or system, I
>> can patch this file with extra trace statements, and diagnose the
>> failure that way.
>> If you think removing the print statement is a better option, and I
>> do not hear objections or other opinions, I can just remove the print
>> statement.
>
> Delete please :)
>
Done.

>>> That said I did not look at this in the past but I feel the whole
>>> docker test here is not really appropriate/suitable to always be run
>>> on linux-x64. It has to exec another process and wait up to 10
>>> seconds to get a result! Can't this actually be done via a WhiteBox
>>> check once Bob's container support updates are in place?
>> Actually, WhiteBox is not necessary for such check. The ability to
>> run docker on a given host/node can be determined in a body of a
>> jtreg test, via a test utility. In fact, that was my original intent.
>> When this change was presented for a discussion, this aspect was
>> debated and reviewers have reached a consensus to do such check in
>> VMProps.java. We mainly discussed within VM SQE team. The main
>> argument was that using @requires is a more proper way of "skipping"
>> the test(s) on unsupported environments, rather then checking this in
>> the body of the test and then returning. When using @requires, the
>> test execution system will report the test as "not run". When
>> skipping the test using "return" from the body of a test, the
>> execution system will report test as "ran" and passed; jtreg does not
>> yet support the "skipped" state.
>>
>> Unfortunately, the way our "@requires" mechanism works is that it
>> runs each time prior to starting a test run, even if the test being
>> run is not concerned with a specific property. It was my concern that
>> this could be a burden on the test run startup, but I eventually
>> agreed to the opinion of other reviewers.
>>
>> If you have a strong opinion or concern about this, let me know.
>> However, I am not sure what is the best process to use to overturn a
>> decision on a prior review or design discussion. I guess one option
>> is to file a bug or an RFE, and bring it up for a discussion.
>
> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using
> WhiteBox connected to Bob's new container support code is better than
> exec'ing a separate process (which I'm not even sure what it does).
I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps' from
VMProps.java to determine presence of docker engine is deemed too heavy"

Also, in current webrev, I though I could do something quick and easy to
address your concerns, before we discuss and agree on a final solution.
I have timed running "docker ps" on 2 different hosts, 30 times each. I
saw a spread between 20ms to 150ms, mostly centered around 30ms. Hence,
I reduced the timeout in the VMProps.java from 10 sec to 500ms. I
thought 500 ms gives enough time cushion for slower test nodes/hosts.

Let me know what you think. I can revert this part of the change or
reduce the timeout value a bit more.

Thank you,
Misha

>
>>
>> For this change under review, if I do not hear opinions from other
>> reviewers, I can simply delete the print statement.
>
> Sounds good to me.
>
> Thanks,
> David
>
>>
>> Thank you,
>> Misha
>>>
>>> Thanks,
>>> David
>>>
>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>> Please review this simple change that makes the error message in
>>>> jtreg-ext/requires/VMProps.java conditional
>>>> on java property "vmprops.trace.verbose". W/o this condition an
>>>> error message is printed each time any jtreg
>>>> test is ran, including non-docker tests.
>>>>
>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>
>>>>
>>>>
>>>> ======= Background and details:
>>>> VMProps.java is called for each test run of jtreg (be it a run
>>>> containing a singe test or multiple tests).
>>>> VMProps evaluates the @requires properties. Therefore, in case of
>>>> this bug, any time a user runs any jtreg VM test
>>>> on a machine w/o an operational docker engine, this error will pop up.
>>>> The fix makes printing of this error message conditional, off by
>>>> default. The message can be turend on
>>>> via a property vmprops.trace.verbose when detailed diagnostic info
>>>> is desired.
>>>>
>>>>
>>>>
>>>> ======== Testing:
>>>> Local testing:
>>>>    W/o docker present:
>>>>      jtreg <arbitrary-jtreg-test>
>>>>        No error message
>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>        Detailed error message
>>>>      jtreg DockerBasicTest.java
>>>>        Test skipped
>>>>
>>>>    With docker installed but no permissions:
>>>>      jtreg <arbitrary-jtreg-test>
>>>>        No error message
>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>        Detailed error message
>>>>      jtreg DockerBasicTest.java
>>>>        Test skipped
>>>>
>>>>    With docker installed and operational:
>>>>      jtreg <arbitrary-jtreg-test>
>>>>        No error message
>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>        No error message
>>>>      jtreg DockerBasicTest.java
>>>>        Test passed
>>>>
>>>> Testing via automated distributed test system:
>>>>    - tier1
>>>>    - docker tests
>>>>    In progress
>>>>
>>>>
>>>> Thank you,
>>>> Misha
>>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

David Holmes
Hi Misha,

On 4/11/2017 10:21 AM, Mikhailo Seledtsov wrote:

> Hi David,
>
> Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/
>
> Please see my comments below.
>
> On 11/2/17, 6:02 PM, David Holmes wrote:
>> Hi Misha,
>>
>> On 3/11/2017 10:22 AM, mikhailo wrote:
>>> Hi David,
>>>
>>>    Thank you for taking a look at this change.
>>>
>>>
>>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>>> Hi Misha,
>>>>
>>>> I don't see anything else in VMProps that writes anything out so to
>>>> me the fix is to delete the System.err.println completely.
>>> Removal is an option, and seems a simpler one. However, we will loose
>>> the diagnostic information.
>>> On the other hand, if an issue arises on a specific host or system, I
>>> can patch this file with extra trace statements, and diagnose the
>>> failure that way.
>>> If you think removing the print statement is a better option, and I
>>> do not hear objections or other opinions, I can just remove the print
>>> statement.
>>
>> Delete please :)
>>
> Done.
>>>> That said I did not look at this in the past but I feel the whole
>>>> docker test here is not really appropriate/suitable to always be run
>>>> on linux-x64. It has to exec another process and wait up to 10
>>>> seconds to get a result! Can't this actually be done via a WhiteBox
>>>> check once Bob's container support updates are in place?
>>> Actually, WhiteBox is not necessary for such check. The ability to
>>> run docker on a given host/node can be determined in a body of a
>>> jtreg test, via a test utility. In fact, that was my original intent.
>>> When this change was presented for a discussion, this aspect was
>>> debated and reviewers have reached a consensus to do such check in
>>> VMProps.java. We mainly discussed within VM SQE team. The main
>>> argument was that using @requires is a more proper way of "skipping"
>>> the test(s) on unsupported environments, rather then checking this in
>>> the body of the test and then returning. When using @requires, the
>>> test execution system will report the test as "not run". When
>>> skipping the test using "return" from the body of a test, the
>>> execution system will report test as "ran" and passed; jtreg does not
>>> yet support the "skipped" state.
>>>
>>> Unfortunately, the way our "@requires" mechanism works is that it
>>> runs each time prior to starting a test run, even if the test being
>>> run is not concerned with a specific property. It was my concern that
>>> this could be a burden on the test run startup, but I eventually
>>> agreed to the opinion of other reviewers.
>>>
>>> If you have a strong opinion or concern about this, let me know.
>>> However, I am not sure what is the best process to use to overturn a
>>> decision on a prior review or design discussion. I guess one option
>>> is to file a bug or an RFE, and bring it up for a discussion.
>>
>> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using
>> WhiteBox connected to Bob's new container support code is better than
>> exec'ing a separate process (which I'm not even sure what it does).
> I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps' from
> VMProps.java to determine presence of docker engine is deemed too heavy"
>
> Also, in current webrev, I though I could do something quick and easy to
> address your concerns, before we discuss and agree on a final solution.
> I have timed running "docker ps" on 2 different hosts, 30 times each. I
> saw a spread between 20ms to 150ms, mostly centered around 30ms. Hence,
> I reduced the timeout in the VMProps.java from 10 sec to 500ms. I
> thought 500 ms gives enough time cushion for slower test nodes/hosts.

I think a slowness would arise on systems that do not have docker
installed. If I time "docker ps" on my system I get

  > time docker ps
The program 'docker' is currently not installed.  To run 'docker' please
ask your administrator to install the package 'docker'

real    0m4.103s
user    0m0.044s
sys     0m0.036s

but the second call is much quicker:

  > time docker ps
The program 'docker' is currently not installed.  To run 'docker' please
ask your administrator to install the package 'docker'

real    0m0.078s
user    0m0.052s
sys     0m0.012s

It may well depend on the user's path. If "docker" is present early in
the path then it will be found and executed quickly, but if not present,
or if further down the path it may take a while to occur.

So I think the timeout change, as a short term proposal, may be more
risky and so not worthwhile.

I'd prefer to see the whole "docker ps" disappear altogether. Such an
approach is not scalable if there may be different container systems.

Thanks,
David

> Let me know what you think. I can revert this part of the change or
> reduce the timeout value a bit more.
>
> Thank you,
> Misha
>>
>>>
>>> For this change under review, if I do not hear opinions from other
>>> reviewers, I can simply delete the print statement.
>>
>> Sounds good to me.
>>
>> Thanks,
>> David
>>
>>>
>>> Thank you,
>>> Misha
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>> Please review this simple change that makes the error message in
>>>>> jtreg-ext/requires/VMProps.java conditional
>>>>> on java property "vmprops.trace.verbose". W/o this condition an
>>>>> error message is printed each time any jtreg
>>>>> test is ran, including non-docker tests.
>>>>>
>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>
>>>>>
>>>>>
>>>>> ======= Background and details:
>>>>> VMProps.java is called for each test run of jtreg (be it a run
>>>>> containing a singe test or multiple tests).
>>>>> VMProps evaluates the @requires properties. Therefore, in case of
>>>>> this bug, any time a user runs any jtreg VM test
>>>>> on a machine w/o an operational docker engine, this error will pop up.
>>>>> The fix makes printing of this error message conditional, off by
>>>>> default. The message can be turend on
>>>>> via a property vmprops.trace.verbose when detailed diagnostic info
>>>>> is desired.
>>>>>
>>>>>
>>>>>
>>>>> ======== Testing:
>>>>> Local testing:
>>>>>    W/o docker present:
>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>        No error message
>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>        Detailed error message
>>>>>      jtreg DockerBasicTest.java
>>>>>        Test skipped
>>>>>
>>>>>    With docker installed but no permissions:
>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>        No error message
>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>        Detailed error message
>>>>>      jtreg DockerBasicTest.java
>>>>>        Test skipped
>>>>>
>>>>>    With docker installed and operational:
>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>        No error message
>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>        No error message
>>>>>      jtreg DockerBasicTest.java
>>>>>        Test passed
>>>>>
>>>>> Testing via automated distributed test system:
>>>>>    - tier1
>>>>>    - docker tests
>>>>>    In progress
>>>>>
>>>>>
>>>>> Thank you,
>>>>> Misha
>>>>>
>>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov
Hi David,


On 11/05/2017 07:28 PM, David Holmes wrote:

> Hi Misha,
>
> On 4/11/2017 10:21 AM, Mikhailo Seledtsov wrote:
>> Hi David,
>>
>> Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/
>>
>> Please see my comments below.
>>
>> On 11/2/17, 6:02 PM, David Holmes wrote:
>>> Hi Misha,
>>>
>>> On 3/11/2017 10:22 AM, mikhailo wrote:
>>>> Hi David,
>>>>
>>>>    Thank you for taking a look at this change.
>>>>
>>>>
>>>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>>>> Hi Misha,
>>>>>
>>>>> I don't see anything else in VMProps that writes anything out so
>>>>> to me the fix is to delete the System.err.println completely.
>>>> Removal is an option, and seems a simpler one. However, we will
>>>> loose the diagnostic information.
>>>> On the other hand, if an issue arises on a specific host or system,
>>>> I can patch this file with extra trace statements, and diagnose the
>>>> failure that way.
>>>> If you think removing the print statement is a better option, and I
>>>> do not hear objections or other opinions, I can just remove the
>>>> print statement.
>>>
>>> Delete please :)
>>>
>> Done.
>>>>> That said I did not look at this in the past but I feel the whole
>>>>> docker test here is not really appropriate/suitable to always be
>>>>> run on linux-x64. It has to exec another process and wait up to 10
>>>>> seconds to get a result! Can't this actually be done via a
>>>>> WhiteBox check once Bob's container support updates are in place?
>>>> Actually, WhiteBox is not necessary for such check. The ability to
>>>> run docker on a given host/node can be determined in a body of a
>>>> jtreg test, via a test utility. In fact, that was my original
>>>> intent. When this change was presented for a discussion, this
>>>> aspect was debated and reviewers have reached a consensus to do
>>>> such check in VMProps.java. We mainly discussed within VM SQE team.
>>>> The main argument was that using @requires is a more proper way of
>>>> "skipping" the test(s) on unsupported environments, rather then
>>>> checking this in the body of the test and then returning. When
>>>> using @requires, the test execution system will report the test as
>>>> "not run". When skipping the test using "return" from the body of a
>>>> test, the execution system will report test as "ran" and passed;
>>>> jtreg does not yet support the "skipped" state.
>>>>
>>>> Unfortunately, the way our "@requires" mechanism works is that it
>>>> runs each time prior to starting a test run, even if the test being
>>>> run is not concerned with a specific property. It was my concern
>>>> that this could be a burden on the test run startup, but I
>>>> eventually agreed to the opinion of other reviewers.
>>>>
>>>> If you have a strong opinion or concern about this, let me know.
>>>> However, I am not sure what is the best process to use to overturn
>>>> a decision on a prior review or design discussion. I guess one
>>>> option is to file a bug or an RFE, and bring it up for a discussion.
>>>
>>> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using
>>> WhiteBox connected to Bob's new container support code is better
>>> than exec'ing a separate process (which I'm not even sure what it
>>> does).
>> I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps' from
>> VMProps.java to determine presence of docker engine is deemed too heavy"
>>
>> Also, in current webrev, I though I could do something quick and easy
>> to address your concerns, before we discuss and agree on a final
>> solution.
>> I have timed running "docker ps" on 2 different hosts, 30 times each.
>> I saw a spread between 20ms to 150ms, mostly centered around 30ms.
>> Hence, I reduced the timeout in the VMProps.java from 10 sec to
>> 500ms. I thought 500 ms gives enough time cushion for slower test
>> nodes/hosts.
>
> I think a slowness would arise on systems that do not have docker
> installed. If I time "docker ps" on my system I get
>
>  > time docker ps
> The program 'docker' is currently not installed.  To run 'docker'
> please ask your administrator to install the package 'docker'
>
> real    0m4.103s
> user    0m0.044s
> sys     0m0.036s
>
> but the second call is much quicker:
>
>  > time docker ps
> The program 'docker' is currently not installed.  To run 'docker'
> please ask your administrator to install the package 'docker'
>
> real    0m0.078s
> user    0m0.052s
> sys     0m0.012s
>
> It may well depend on the user's path. If "docker" is present early in
> the path then it will be found and executed quickly, but if not
> present, or if further down the path it may take a while to occur.
>
> So I think the timeout change, as a short term proposal, may be more
> risky and so not worthwhile.
>
> I'd prefer to see the whole "docker ps" disappear altogether. Such an
> approach is not scalable if there may be different container systems.
I have discussed this topic again with Igor and Leonid, and explained
your concerns.


We came up with alternative ideas:

1. VMProps.java will check the existence of docker on a given test
machine. We will look at "/bin/docker", "/usr/bin/docker",
"/usr/local/bin/docker"
     For now we only support Linux x64. If any of these files exist, we
return 'true' for a corresponding at-requires property, otherwise false.

2. In case someone wishes to specify a custom location for a docker
binary, we thought we could use a property for that:
      jdk.test.lib.docker.path=""
     If this property is specified, VMProps.java will check the docker
at that location.

3. No more "docker ps" execution in VMProps.java.

Please let me know what you think.



Thank you,
Misha

>
> Thanks,
> David
>
>> Let me know what you think. I can revert this part of the change or
>> reduce the timeout value a bit more.
>>
>> Thank you,
>> Misha
>>>
>>>>
>>>> For this change under review, if I do not hear opinions from other
>>>> reviewers, I can simply delete the print statement.
>>>
>>> Sounds good to me.
>>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> Thank you,
>>>> Misha
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>> Please review this simple change that makes the error message in
>>>>>> jtreg-ext/requires/VMProps.java conditional
>>>>>> on java property "vmprops.trace.verbose". W/o this condition an
>>>>>> error message is printed each time any jtreg
>>>>>> test is ran, including non-docker tests.
>>>>>>
>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>
>>>>>>
>>>>>>
>>>>>> ======= Background and details:
>>>>>> VMProps.java is called for each test run of jtreg (be it a run
>>>>>> containing a singe test or multiple tests).
>>>>>> VMProps evaluates the @requires properties. Therefore, in case of
>>>>>> this bug, any time a user runs any jtreg VM test
>>>>>> on a machine w/o an operational docker engine, this error will
>>>>>> pop up.
>>>>>> The fix makes printing of this error message conditional, off by
>>>>>> default. The message can be turend on
>>>>>> via a property vmprops.trace.verbose when detailed diagnostic
>>>>>> info is desired.
>>>>>>
>>>>>>
>>>>>>
>>>>>> ======== Testing:
>>>>>> Local testing:
>>>>>>    W/o docker present:
>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>        No error message
>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>        Detailed error message
>>>>>>      jtreg DockerBasicTest.java
>>>>>>        Test skipped
>>>>>>
>>>>>>    With docker installed but no permissions:
>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>        No error message
>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>        Detailed error message
>>>>>>      jtreg DockerBasicTest.java
>>>>>>        Test skipped
>>>>>>
>>>>>>    With docker installed and operational:
>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>        No error message
>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>        No error message
>>>>>>      jtreg DockerBasicTest.java
>>>>>>        Test passed
>>>>>>
>>>>>> Testing via automated distributed test system:
>>>>>>    - tier1
>>>>>>    - docker tests
>>>>>>    In progress
>>>>>>
>>>>>>
>>>>>> Thank you,
>>>>>> Misha
>>>>>>
>>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

David Holmes
Hi Misha,

This seems reasonable.

Thanks,
David

On 7/11/2017 8:42 AM, mikhailo wrote:

> Hi David,
>
>
> On 11/05/2017 07:28 PM, David Holmes wrote:
>> Hi Misha,
>>
>> On 4/11/2017 10:21 AM, Mikhailo Seledtsov wrote:
>>> Hi David,
>>>
>>> Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/
>>>
>>> Please see my comments below.
>>>
>>> On 11/2/17, 6:02 PM, David Holmes wrote:
>>>> Hi Misha,
>>>>
>>>> On 3/11/2017 10:22 AM, mikhailo wrote:
>>>>> Hi David,
>>>>>
>>>>>    Thank you for taking a look at this change.
>>>>>
>>>>>
>>>>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>>>>> Hi Misha,
>>>>>>
>>>>>> I don't see anything else in VMProps that writes anything out so
>>>>>> to me the fix is to delete the System.err.println completely.
>>>>> Removal is an option, and seems a simpler one. However, we will
>>>>> loose the diagnostic information.
>>>>> On the other hand, if an issue arises on a specific host or system,
>>>>> I can patch this file with extra trace statements, and diagnose the
>>>>> failure that way.
>>>>> If you think removing the print statement is a better option, and I
>>>>> do not hear objections or other opinions, I can just remove the
>>>>> print statement.
>>>>
>>>> Delete please :)
>>>>
>>> Done.
>>>>>> That said I did not look at this in the past but I feel the whole
>>>>>> docker test here is not really appropriate/suitable to always be
>>>>>> run on linux-x64. It has to exec another process and wait up to 10
>>>>>> seconds to get a result! Can't this actually be done via a
>>>>>> WhiteBox check once Bob's container support updates are in place?
>>>>> Actually, WhiteBox is not necessary for such check. The ability to
>>>>> run docker on a given host/node can be determined in a body of a
>>>>> jtreg test, via a test utility. In fact, that was my original
>>>>> intent. When this change was presented for a discussion, this
>>>>> aspect was debated and reviewers have reached a consensus to do
>>>>> such check in VMProps.java. We mainly discussed within VM SQE team.
>>>>> The main argument was that using @requires is a more proper way of
>>>>> "skipping" the test(s) on unsupported environments, rather then
>>>>> checking this in the body of the test and then returning. When
>>>>> using @requires, the test execution system will report the test as
>>>>> "not run". When skipping the test using "return" from the body of a
>>>>> test, the execution system will report test as "ran" and passed;
>>>>> jtreg does not yet support the "skipped" state.
>>>>>
>>>>> Unfortunately, the way our "@requires" mechanism works is that it
>>>>> runs each time prior to starting a test run, even if the test being
>>>>> run is not concerned with a specific property. It was my concern
>>>>> that this could be a burden on the test run startup, but I
>>>>> eventually agreed to the opinion of other reviewers.
>>>>>
>>>>> If you have a strong opinion or concern about this, let me know.
>>>>> However, I am not sure what is the best process to use to overturn
>>>>> a decision on a prior review or design discussion. I guess one
>>>>> option is to file a bug or an RFE, and bring it up for a discussion.
>>>>
>>>> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using
>>>> WhiteBox connected to Bob's new container support code is better
>>>> than exec'ing a separate process (which I'm not even sure what it
>>>> does).
>>> I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps' from
>>> VMProps.java to determine presence of docker engine is deemed too heavy"
>>>
>>> Also, in current webrev, I though I could do something quick and easy
>>> to address your concerns, before we discuss and agree on a final
>>> solution.
>>> I have timed running "docker ps" on 2 different hosts, 30 times each.
>>> I saw a spread between 20ms to 150ms, mostly centered around 30ms.
>>> Hence, I reduced the timeout in the VMProps.java from 10 sec to
>>> 500ms. I thought 500 ms gives enough time cushion for slower test
>>> nodes/hosts.
>>
>> I think a slowness would arise on systems that do not have docker
>> installed. If I time "docker ps" on my system I get
>>
>>  > time docker ps
>> The program 'docker' is currently not installed.  To run 'docker'
>> please ask your administrator to install the package 'docker'
>>
>> real    0m4.103s
>> user    0m0.044s
>> sys     0m0.036s
>>
>> but the second call is much quicker:
>>
>>  > time docker ps
>> The program 'docker' is currently not installed.  To run 'docker'
>> please ask your administrator to install the package 'docker'
>>
>> real    0m0.078s
>> user    0m0.052s
>> sys     0m0.012s
>>
>> It may well depend on the user's path. If "docker" is present early in
>> the path then it will be found and executed quickly, but if not
>> present, or if further down the path it may take a while to occur.
>>
>> So I think the timeout change, as a short term proposal, may be more
>> risky and so not worthwhile.
>>
>> I'd prefer to see the whole "docker ps" disappear altogether. Such an
>> approach is not scalable if there may be different container systems.
> I have discussed this topic again with Igor and Leonid, and explained
> your concerns.
>
>
> We came up with alternative ideas:
>
> 1. VMProps.java will check the existence of docker on a given test
> machine. We will look at "/bin/docker", "/usr/bin/docker",
> "/usr/local/bin/docker"
>      For now we only support Linux x64. If any of these files exist, we
> return 'true' for a corresponding at-requires property, otherwise false.
>
> 2. In case someone wishes to specify a custom location for a docker
> binary, we thought we could use a property for that:
>       jdk.test.lib.docker.path=""
>      If this property is specified, VMProps.java will check the docker
> at that location.
>
> 3. No more "docker ps" execution in VMProps.java.
>
> Please let me know what you think.
>
>
>
> Thank you,
> Misha
>
>>
>> Thanks,
>> David
>>
>>> Let me know what you think. I can revert this part of the change or
>>> reduce the timeout value a bit more.
>>>
>>> Thank you,
>>> Misha
>>>>
>>>>>
>>>>> For this change under review, if I do not hear opinions from other
>>>>> reviewers, I can simply delete the print statement.
>>>>
>>>> Sounds good to me.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>>
>>>>> Thank you,
>>>>> Misha
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>> Please review this simple change that makes the error message in
>>>>>>> jtreg-ext/requires/VMProps.java conditional
>>>>>>> on java property "vmprops.trace.verbose". W/o this condition an
>>>>>>> error message is printed each time any jtreg
>>>>>>> test is ran, including non-docker tests.
>>>>>>>
>>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ======= Background and details:
>>>>>>> VMProps.java is called for each test run of jtreg (be it a run
>>>>>>> containing a singe test or multiple tests).
>>>>>>> VMProps evaluates the @requires properties. Therefore, in case of
>>>>>>> this bug, any time a user runs any jtreg VM test
>>>>>>> on a machine w/o an operational docker engine, this error will
>>>>>>> pop up.
>>>>>>> The fix makes printing of this error message conditional, off by
>>>>>>> default. The message can be turend on
>>>>>>> via a property vmprops.trace.verbose when detailed diagnostic
>>>>>>> info is desired.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ======== Testing:
>>>>>>> Local testing:
>>>>>>>    W/o docker present:
>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>        No error message
>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>        Detailed error message
>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>        Test skipped
>>>>>>>
>>>>>>>    With docker installed but no permissions:
>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>        No error message
>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>        Detailed error message
>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>        Test skipped
>>>>>>>
>>>>>>>    With docker installed and operational:
>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>        No error message
>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>        No error message
>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>        Test passed
>>>>>>>
>>>>>>> Testing via automated distributed test system:
>>>>>>>    - tier1
>>>>>>>    - docker tests
>>>>>>>    In progress
>>>>>>>
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Misha
>>>>>>>
>>>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov
Thank you David,

I will make changes and post an updated webrev.


Misha

On 11/6/17, 5:34 PM, David Holmes wrote:

> Hi Misha,
>
> This seems reasonable.
>
> Thanks,
> David
>
> On 7/11/2017 8:42 AM, mikhailo wrote:
>> Hi David,
>>
>>
>> On 11/05/2017 07:28 PM, David Holmes wrote:
>>> Hi Misha,
>>>
>>> On 4/11/2017 10:21 AM, Mikhailo Seledtsov wrote:
>>>> Hi David,
>>>>
>>>> Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/
>>>>
>>>> Please see my comments below.
>>>>
>>>> On 11/2/17, 6:02 PM, David Holmes wrote:
>>>>> Hi Misha,
>>>>>
>>>>> On 3/11/2017 10:22 AM, mikhailo wrote:
>>>>>> Hi David,
>>>>>>
>>>>>>    Thank you for taking a look at this change.
>>>>>>
>>>>>>
>>>>>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>>>>>> Hi Misha,
>>>>>>>
>>>>>>> I don't see anything else in VMProps that writes anything out so
>>>>>>> to me the fix is to delete the System.err.println completely.
>>>>>> Removal is an option, and seems a simpler one. However, we will
>>>>>> loose the diagnostic information.
>>>>>> On the other hand, if an issue arises on a specific host or
>>>>>> system, I can patch this file with extra trace statements, and
>>>>>> diagnose the failure that way.
>>>>>> If you think removing the print statement is a better option, and
>>>>>> I do not hear objections or other opinions, I can just remove the
>>>>>> print statement.
>>>>>
>>>>> Delete please :)
>>>>>
>>>> Done.
>>>>>>> That said I did not look at this in the past but I feel the
>>>>>>> whole docker test here is not really appropriate/suitable to
>>>>>>> always be run on linux-x64. It has to exec another process and
>>>>>>> wait up to 10 seconds to get a result! Can't this actually be
>>>>>>> done via a WhiteBox check once Bob's container support updates
>>>>>>> are in place?
>>>>>> Actually, WhiteBox is not necessary for such check. The ability
>>>>>> to run docker on a given host/node can be determined in a body of
>>>>>> a jtreg test, via a test utility. In fact, that was my original
>>>>>> intent. When this change was presented for a discussion, this
>>>>>> aspect was debated and reviewers have reached a consensus to do
>>>>>> such check in VMProps.java. We mainly discussed within VM SQE
>>>>>> team. The main argument was that using @requires is a more proper
>>>>>> way of "skipping" the test(s) on unsupported environments, rather
>>>>>> then checking this in the body of the test and then returning.
>>>>>> When using @requires, the test execution system will report the
>>>>>> test as "not run". When skipping the test using "return" from the
>>>>>> body of a test, the execution system will report test as "ran"
>>>>>> and passed; jtreg does not yet support the "skipped" state.
>>>>>>
>>>>>> Unfortunately, the way our "@requires" mechanism works is that it
>>>>>> runs each time prior to starting a test run, even if the test
>>>>>> being run is not concerned with a specific property. It was my
>>>>>> concern that this could be a burden on the test run startup, but
>>>>>> I eventually agreed to the opinion of other reviewers.
>>>>>>
>>>>>> If you have a strong opinion or concern about this, let me know.
>>>>>> However, I am not sure what is the best process to use to
>>>>>> overturn a decision on a prior review or design discussion. I
>>>>>> guess one option is to file a bug or an RFE, and bring it up for
>>>>>> a discussion.
>>>>>
>>>>> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think
>>>>> using WhiteBox connected to Bob's new container support code is
>>>>> better than exec'ing a separate process (which I'm not even sure
>>>>> what it does).
>>>> I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps'
>>>> from VMProps.java to determine presence of docker engine is deemed
>>>> too heavy"
>>>>
>>>> Also, in current webrev, I though I could do something quick and
>>>> easy to address your concerns, before we discuss and agree on a
>>>> final solution.
>>>> I have timed running "docker ps" on 2 different hosts, 30 times
>>>> each. I saw a spread between 20ms to 150ms, mostly centered around
>>>> 30ms. Hence, I reduced the timeout in the VMProps.java from 10 sec
>>>> to 500ms. I thought 500 ms gives enough time cushion for slower
>>>> test nodes/hosts.
>>>
>>> I think a slowness would arise on systems that do not have docker
>>> installed. If I time "docker ps" on my system I get
>>>
>>> > time docker ps
>>> The program 'docker' is currently not installed.  To run 'docker'
>>> please ask your administrator to install the package 'docker'
>>>
>>> real    0m4.103s
>>> user    0m0.044s
>>> sys     0m0.036s
>>>
>>> but the second call is much quicker:
>>>
>>> > time docker ps
>>> The program 'docker' is currently not installed.  To run 'docker'
>>> please ask your administrator to install the package 'docker'
>>>
>>> real    0m0.078s
>>> user    0m0.052s
>>> sys     0m0.012s
>>>
>>> It may well depend on the user's path. If "docker" is present early
>>> in the path then it will be found and executed quickly, but if not
>>> present, or if further down the path it may take a while to occur.
>>>
>>> So I think the timeout change, as a short term proposal, may be more
>>> risky and so not worthwhile.
>>>
>>> I'd prefer to see the whole "docker ps" disappear altogether. Such
>>> an approach is not scalable if there may be different container
>>> systems.
>> I have discussed this topic again with Igor and Leonid, and explained
>> your concerns.
>>
>>
>> We came up with alternative ideas:
>>
>> 1. VMProps.java will check the existence of docker on a given test
>> machine. We will look at "/bin/docker", "/usr/bin/docker",
>> "/usr/local/bin/docker"
>>      For now we only support Linux x64. If any of these files exist,
>> we return 'true' for a corresponding at-requires property, otherwise
>> false.
>>
>> 2. In case someone wishes to specify a custom location for a docker
>> binary, we thought we could use a property for that:
>>       jdk.test.lib.docker.path=""
>>      If this property is specified, VMProps.java will check the
>> docker at that location.
>>
>> 3. No more "docker ps" execution in VMProps.java.
>>
>> Please let me know what you think.
>>
>>
>>
>> Thank you,
>> Misha
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> Let me know what you think. I can revert this part of the change or
>>>> reduce the timeout value a bit more.
>>>>
>>>> Thank you,
>>>> Misha
>>>>>
>>>>>>
>>>>>> For this change under review, if I do not hear opinions from
>>>>>> other reviewers, I can simply delete the print statement.
>>>>>
>>>>> Sounds good to me.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>>
>>>>>> Thank you,
>>>>>> Misha
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>>> Please review this simple change that makes the error message
>>>>>>>> in jtreg-ext/requires/VMProps.java conditional
>>>>>>>> on java property "vmprops.trace.verbose". W/o this condition an
>>>>>>>> error message is printed each time any jtreg
>>>>>>>> test is ran, including non-docker tests.
>>>>>>>>
>>>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ======= Background and details:
>>>>>>>> VMProps.java is called for each test run of jtreg (be it a run
>>>>>>>> containing a singe test or multiple tests).
>>>>>>>> VMProps evaluates the @requires properties. Therefore, in case
>>>>>>>> of this bug, any time a user runs any jtreg VM test
>>>>>>>> on a machine w/o an operational docker engine, this error will
>>>>>>>> pop up.
>>>>>>>> The fix makes printing of this error message conditional, off
>>>>>>>> by default. The message can be turend on
>>>>>>>> via a property vmprops.trace.verbose when detailed diagnostic
>>>>>>>> info is desired.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ======== Testing:
>>>>>>>> Local testing:
>>>>>>>>    W/o docker present:
>>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>>        No error message
>>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>        Detailed error message
>>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>>        Test skipped
>>>>>>>>
>>>>>>>>    With docker installed but no permissions:
>>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>>        No error message
>>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>        Detailed error message
>>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>>        Test skipped
>>>>>>>>
>>>>>>>>    With docker installed and operational:
>>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>>        No error message
>>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>        No error message
>>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>>        Test passed
>>>>>>>>
>>>>>>>> Testing via automated distributed test system:
>>>>>>>>    - tier1
>>>>>>>>    - docker tests
>>>>>>>>    In progress
>>>>>>>>
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Misha
>>>>>>>>
>>>>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Bob Vandette
In reply to this post by Mikhailo Seledtsov

> On Nov 6, 2017, at 5:42 PM, mikhailo <[hidden email]> wrote:
>
> Hi David,
>
>
> On 11/05/2017 07:28 PM, David Holmes wrote:
>> Hi Misha,
>>
>> On 4/11/2017 10:21 AM, Mikhailo Seledtsov wrote:
>>> Hi David,
>>>
>>> Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/
>>>
>>> Please see my comments below.
>>>
>>> On 11/2/17, 6:02 PM, David Holmes wrote:
>>>> Hi Misha,
>>>>
>>>> On 3/11/2017 10:22 AM, mikhailo wrote:
>>>>> Hi David,
>>>>>
>>>>>    Thank you for taking a look at this change.
>>>>>
>>>>>
>>>>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>>>>> Hi Misha,
>>>>>>
>>>>>> I don't see anything else in VMProps that writes anything out so to me the fix is to delete the System.err.println completely.
>>>>> Removal is an option, and seems a simpler one. However, we will loose the diagnostic information.
>>>>> On the other hand, if an issue arises on a specific host or system, I can patch this file with extra trace statements, and diagnose the failure that way.
>>>>> If you think removing the print statement is a better option, and I do not hear objections or other opinions, I can just remove the print statement.
>>>>
>>>> Delete please :)
>>>>
>>> Done.
>>>>>> That said I did not look at this in the past but I feel the whole docker test here is not really appropriate/suitable to always be run on linux-x64. It has to exec another process and wait up to 10 seconds to get a result! Can't this actually be done via a WhiteBox check once Bob's container support updates are in place?
>>>>> Actually, WhiteBox is not necessary for such check. The ability to run docker on a given host/node can be determined in a body of a jtreg test, via a test utility. In fact, that was my original intent. When this change was presented for a discussion, this aspect was debated and reviewers have reached a consensus to do such check in VMProps.java. We mainly discussed within VM SQE team. The main argument was that using @requires is a more proper way of "skipping" the test(s) on unsupported environments, rather then checking this in the body of the test and then returning. When using @requires, the test execution system will report the test as "not run". When skipping the test using "return" from the body of a test, the execution system will report test as "ran" and passed; jtreg does not yet support the "skipped" state.
>>>>>
>>>>> Unfortunately, the way our "@requires" mechanism works is that it runs each time prior to starting a test run, even if the test being run is not concerned with a specific property. It was my concern that this could be a burden on the test run startup, but I eventually agreed to the opinion of other reviewers.
>>>>>
>>>>> If you have a strong opinion or concern about this, let me know. However, I am not sure what is the best process to use to overturn a decision on a prior review or design discussion. I guess one option is to file a bug or an RFE, and bring it up for a discussion.
>>>>
>>>> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using WhiteBox connected to Bob's new container support code is better than exec'ing a separate process (which I'm not even sure what it does).
>>> I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps' from VMProps.java to determine presence of docker engine is deemed too heavy"
>>>
>>> Also, in current webrev, I though I could do something quick and easy to address your concerns, before we discuss and agree on a final solution.
>>> I have timed running "docker ps" on 2 different hosts, 30 times each. I saw a spread between 20ms to 150ms, mostly centered around 30ms. Hence, I reduced the timeout in the VMProps.java from 10 sec to 500ms. I thought 500 ms gives enough time cushion for slower test nodes/hosts.
>>
>> I think a slowness would arise on systems that do not have docker installed. If I time "docker ps" on my system I get
>>
>>  > time docker ps
>> The program 'docker' is currently not installed.  To run 'docker' please ask your administrator to install the package 'docker'
>>
>> real    0m4.103s
>> user    0m0.044s
>> sys     0m0.036s
>>
>> but the second call is much quicker:
>>
>>  > time docker ps
>> The program 'docker' is currently not installed.  To run 'docker' please ask your administrator to install the package 'docker'
>>
>> real    0m0.078s
>> user    0m0.052s
>> sys     0m0.012s
>>
>> It may well depend on the user's path. If "docker" is present early in the path then it will be found and executed quickly, but if not present, or if further down the path it may take a while to occur.
>>
>> So I think the timeout change, as a short term proposal, may be more risky and so not worthwhile.
>>
>> I'd prefer to see the whole "docker ps" disappear altogether. Such an approach is not scalable if there may be different container systems.
> I have discussed this topic again with Igor and Leonid, and explained your concerns.
>
>
> We came up with alternative ideas:
>
> 1. VMProps.java will check the existence of docker on a given test machine. We will look at "/bin/docker", "/usr/bin/docker", "/usr/local/bin/docker"
>     For now we only support Linux x64. If any of these files exist, we return 'true' for a corresponding at-requires property, otherwise false.
>
> 2. In case someone wishes to specify a custom location for a docker binary, we thought we could use a property for that:
>      jdk.test.lib.docker.path=""
>     If this property is specified, VMProps.java will check the docker at that location.
>
> 3. No more "docker ps" execution in VMProps.java.
>
> Please let me know what you think.

Just having checking for the existence of /bin/docker doesn’t means that it’s properly configured for use by any test process.
There’s permission setup, possible proxy setup etc.   We had issues with our internal testing systems not having the tools we’ve
needed and it’s a pain to have to go to each system in order to clear the way to successfully run all the tests.

This is probably breaking new ground, but can we cache the result of “docker ps” in /tmp in order to eliminate the performance
overhead for all but the first time a new user runs these tests?

Bob.

>
>
>
> Thank you,
> Misha
>
>>
>> Thanks,
>> David
>>
>>> Let me know what you think. I can revert this part of the change or reduce the timeout value a bit more.
>>>
>>> Thank you,
>>> Misha
>>>>
>>>>>
>>>>> For this change under review, if I do not hear opinions from other reviewers, I can simply delete the print statement.
>>>>
>>>> Sounds good to me.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>>
>>>>> Thank you,
>>>>> Misha
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>> Please review this simple change that makes the error message in jtreg-ext/requires/VMProps.java conditional
>>>>>>> on java property "vmprops.trace.verbose". W/o this condition an error message is printed each time any jtreg
>>>>>>> test is ran, including non-docker tests.
>>>>>>>
>>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ======= Background and details:
>>>>>>> VMProps.java is called for each test run of jtreg (be it a run containing a singe test or multiple tests).
>>>>>>> VMProps evaluates the @requires properties. Therefore, in case of this bug, any time a user runs any jtreg VM test
>>>>>>> on a machine w/o an operational docker engine, this error will pop up.
>>>>>>> The fix makes printing of this error message conditional, off by default. The message can be turend on
>>>>>>> via a property vmprops.trace.verbose when detailed diagnostic info is desired.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ======== Testing:
>>>>>>> Local testing:
>>>>>>>    W/o docker present:
>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>        No error message
>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>        Detailed error message
>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>        Test skipped
>>>>>>>
>>>>>>>    With docker installed but no permissions:
>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>        No error message
>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>        Detailed error message
>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>        Test skipped
>>>>>>>
>>>>>>>    With docker installed and operational:
>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>        No error message
>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>        No error message
>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>        Test passed
>>>>>>>
>>>>>>> Testing via automated distributed test system:
>>>>>>>    - tier1
>>>>>>>    - docker tests
>>>>>>>    In progress
>>>>>>>
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Misha
>>>>>>>
>>>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov


On 11/07/2017 09:04 AM, Bob Vandette wrote:

>> On Nov 6, 2017, at 5:42 PM, mikhailo <[hidden email]> wrote:
>>
>> Hi David,
>>
>>
>> On 11/05/2017 07:28 PM, David Holmes wrote:
>>> Hi Misha,
>>>
>>> On 4/11/2017 10:21 AM, Mikhailo Seledtsov wrote:
>>>> Hi David,
>>>>
>>>> Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/
>>>>
>>>> Please see my comments below.
>>>>
>>>> On 11/2/17, 6:02 PM, David Holmes wrote:
>>>>> Hi Misha,
>>>>>
>>>>> On 3/11/2017 10:22 AM, mikhailo wrote:
>>>>>> Hi David,
>>>>>>
>>>>>>     Thank you for taking a look at this change.
>>>>>>
>>>>>>
>>>>>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>>>>>> Hi Misha,
>>>>>>>
>>>>>>> I don't see anything else in VMProps that writes anything out so to me the fix is to delete the System.err.println completely.
>>>>>> Removal is an option, and seems a simpler one. However, we will loose the diagnostic information.
>>>>>> On the other hand, if an issue arises on a specific host or system, I can patch this file with extra trace statements, and diagnose the failure that way.
>>>>>> If you think removing the print statement is a better option, and I do not hear objections or other opinions, I can just remove the print statement.
>>>>> Delete please :)
>>>>>
>>>> Done.
>>>>>>> That said I did not look at this in the past but I feel the whole docker test here is not really appropriate/suitable to always be run on linux-x64. It has to exec another process and wait up to 10 seconds to get a result! Can't this actually be done via a WhiteBox check once Bob's container support updates are in place?
>>>>>> Actually, WhiteBox is not necessary for such check. The ability to run docker on a given host/node can be determined in a body of a jtreg test, via a test utility. In fact, that was my original intent. When this change was presented for a discussion, this aspect was debated and reviewers have reached a consensus to do such check in VMProps.java. We mainly discussed within VM SQE team. The main argument was that using @requires is a more proper way of "skipping" the test(s) on unsupported environments, rather then checking this in the body of the test and then returning. When using @requires, the test execution system will report the test as "not run". When skipping the test using "return" from the body of a test, the execution system will report test as "ran" and passed; jtreg does not yet support the "skipped" state.
>>>>>>
>>>>>> Unfortunately, the way our "@requires" mechanism works is that it runs each time prior to starting a test run, even if the test being run is not concerned with a specific property. It was my concern that this could be a burden on the test run startup, but I eventually agreed to the opinion of other reviewers.
>>>>>>
>>>>>> If you have a strong opinion or concern about this, let me know. However, I am not sure what is the best process to use to overturn a decision on a prior review or design discussion. I guess one option is to file a bug or an RFE, and bring it up for a discussion.
>>>>> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using WhiteBox connected to Bob's new container support code is better than exec'ing a separate process (which I'm not even sure what it does).
>>>> I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps' from VMProps.java to determine presence of docker engine is deemed too heavy"
>>>>
>>>> Also, in current webrev, I though I could do something quick and easy to address your concerns, before we discuss and agree on a final solution.
>>>> I have timed running "docker ps" on 2 different hosts, 30 times each. I saw a spread between 20ms to 150ms, mostly centered around 30ms. Hence, I reduced the timeout in the VMProps.java from 10 sec to 500ms. I thought 500 ms gives enough time cushion for slower test nodes/hosts.
>>> I think a slowness would arise on systems that do not have docker installed. If I time "docker ps" on my system I get
>>>
>>>   > time docker ps
>>> The program 'docker' is currently not installed.  To run 'docker' please ask your administrator to install the package 'docker'
>>>
>>> real    0m4.103s
>>> user    0m0.044s
>>> sys     0m0.036s
>>>
>>> but the second call is much quicker:
>>>
>>>   > time docker ps
>>> The program 'docker' is currently not installed.  To run 'docker' please ask your administrator to install the package 'docker'
>>>
>>> real    0m0.078s
>>> user    0m0.052s
>>> sys     0m0.012s
>>>
>>> It may well depend on the user's path. If "docker" is present early in the path then it will be found and executed quickly, but if not present, or if further down the path it may take a while to occur.
>>>
>>> So I think the timeout change, as a short term proposal, may be more risky and so not worthwhile.
>>>
>>> I'd prefer to see the whole "docker ps" disappear altogether. Such an approach is not scalable if there may be different container systems.
>> I have discussed this topic again with Igor and Leonid, and explained your concerns.
>>
>>
>> We came up with alternative ideas:
>>
>> 1. VMProps.java will check the existence of docker on a given test machine. We will look at "/bin/docker", "/usr/bin/docker", "/usr/local/bin/docker"
>>      For now we only support Linux x64. If any of these files exist, we return 'true' for a corresponding at-requires property, otherwise false.
>>
>> 2. In case someone wishes to specify a custom location for a docker binary, we thought we could use a property for that:
>>       jdk.test.lib.docker.path=""
>>      If this property is specified, VMProps.java will check the docker at that location.
>>
>> 3. No more "docker ps" execution in VMProps.java.
>>
>> Please let me know what you think.
> Just having checking for the existence of /bin/docker doesn’t means that it’s properly configured for use by any test process.
> There’s permission setup, possible proxy setup etc.   We had issues with our internal testing systems not having the tools we’ve
> needed and it’s a pain to have to go to each system in order to clear the way to successfully run all the tests.
Yes, I agree with Bob, it is important to check that the docker is
runnable by the test user or test agent.
Given this, I propose to check 'docker ps' with a small timeout, say
300ms. However, instead of doing 'docker ps', I will do the '<full
path>/docker ps' with the docker binary that I have found on a given system.
This way we run 'docker ps' ONLY if we found it on the system. And, we
give a process runner an exact location of the binary which will
eliminate a search on the PATH, which in turn allows us to make wait
timeout smaller, hence reducing impact/cost on VMProps evaluation.

Does it work for you Bob? David, are you OK with this approach?

>
> This is probably breaking new ground, but can we cache the result of “docker ps” in /tmp in order to eliminate the performance
> overhead for all but the first time a new user runs these tests?
I try to keep this solution simple. If you and David approve the
solution above, I will implement and test it. If we find any problems,
either during my testing or during test production, we could look into
caching mechanisms.


Thank you,
Misha
>
> Bob.

>>
>>
>> Thank you,
>> Misha
>>
>>> Thanks,
>>> David
>>>
>>>> Let me know what you think. I can revert this part of the change or reduce the timeout value a bit more.
>>>>
>>>> Thank you,
>>>> Misha
>>>>>> For this change under review, if I do not hear opinions from other reviewers, I can simply delete the print statement.
>>>>> Sounds good to me.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Thank you,
>>>>>> Misha
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>>> Please review this simple change that makes the error message in jtreg-ext/requires/VMProps.java conditional
>>>>>>>> on java property "vmprops.trace.verbose". W/o this condition an error message is printed each time any jtreg
>>>>>>>> test is ran, including non-docker tests.
>>>>>>>>
>>>>>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>>       Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ======= Background and details:
>>>>>>>> VMProps.java is called for each test run of jtreg (be it a run containing a singe test or multiple tests).
>>>>>>>> VMProps evaluates the @requires properties. Therefore, in case of this bug, any time a user runs any jtreg VM test
>>>>>>>> on a machine w/o an operational docker engine, this error will pop up.
>>>>>>>> The fix makes printing of this error message conditional, off by default. The message can be turend on
>>>>>>>> via a property vmprops.trace.verbose when detailed diagnostic info is desired.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ======== Testing:
>>>>>>>> Local testing:
>>>>>>>>     W/o docker present:
>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>         No error message
>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>         Detailed error message
>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>         Test skipped
>>>>>>>>
>>>>>>>>     With docker installed but no permissions:
>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>         No error message
>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>         Detailed error message
>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>         Test skipped
>>>>>>>>
>>>>>>>>     With docker installed and operational:
>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>         No error message
>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>         No error message
>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>         Test passed
>>>>>>>>
>>>>>>>> Testing via automated distributed test system:
>>>>>>>>     - tier1
>>>>>>>>     - docker tests
>>>>>>>>     In progress
>>>>>>>>
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Misha
>>>>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Bob Vandette

> On Nov 7, 2017, at 2:25 PM, mikhailo <[hidden email]> wrote:
>
>
>
> On 11/07/2017 09:04 AM, Bob Vandette wrote:
>>> On Nov 6, 2017, at 5:42 PM, mikhailo <[hidden email]> wrote:
>>>
>>> Hi David,
>>>
>>>
>>> On 11/05/2017 07:28 PM, David Holmes wrote:
>>>> Hi Misha,
>>>>
>>>> On 4/11/2017 10:21 AM, Mikhailo Seledtsov wrote:
>>>>> Hi David,
>>>>>
>>>>> Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/
>>>>>
>>>>> Please see my comments below.
>>>>>
>>>>> On 11/2/17, 6:02 PM, David Holmes wrote:
>>>>>> Hi Misha,
>>>>>>
>>>>>> On 3/11/2017 10:22 AM, mikhailo wrote:
>>>>>>> Hi David,
>>>>>>>
>>>>>>>    Thank you for taking a look at this change.
>>>>>>>
>>>>>>>
>>>>>>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>>>>>>> Hi Misha,
>>>>>>>>
>>>>>>>> I don't see anything else in VMProps that writes anything out so to me the fix is to delete the System.err.println completely.
>>>>>>> Removal is an option, and seems a simpler one. However, we will loose the diagnostic information.
>>>>>>> On the other hand, if an issue arises on a specific host or system, I can patch this file with extra trace statements, and diagnose the failure that way.
>>>>>>> If you think removing the print statement is a better option, and I do not hear objections or other opinions, I can just remove the print statement.
>>>>>> Delete please :)
>>>>>>
>>>>> Done.
>>>>>>>> That said I did not look at this in the past but I feel the whole docker test here is not really appropriate/suitable to always be run on linux-x64. It has to exec another process and wait up to 10 seconds to get a result! Can't this actually be done via a WhiteBox check once Bob's container support updates are in place?
>>>>>>> Actually, WhiteBox is not necessary for such check. The ability to run docker on a given host/node can be determined in a body of a jtreg test, via a test utility. In fact, that was my original intent. When this change was presented for a discussion, this aspect was debated and reviewers have reached a consensus to do such check in VMProps.java. We mainly discussed within VM SQE team. The main argument was that using @requires is a more proper way of "skipping" the test(s) on unsupported environments, rather then checking this in the body of the test and then returning. When using @requires, the test execution system will report the test as "not run". When skipping the test using "return" from the body of a test, the execution system will report test as "ran" and passed; jtreg does not yet support the "skipped" state.
>>>>>>>
>>>>>>> Unfortunately, the way our "@requires" mechanism works is that it runs each time prior to starting a test run, even if the test being run is not concerned with a specific property. It was my concern that this could be a burden on the test run startup, but I eventually agreed to the opinion of other reviewers.
>>>>>>>
>>>>>>> If you have a strong opinion or concern about this, let me know. However, I am not sure what is the best process to use to overturn a decision on a prior review or design discussion. I guess one option is to file a bug or an RFE, and bring it up for a discussion.
>>>>>> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using WhiteBox connected to Bob's new container support code is better than exec'ing a separate process (which I'm not even sure what it does).
>>>>> I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps' from VMProps.java to determine presence of docker engine is deemed too heavy"
>>>>>
>>>>> Also, in current webrev, I though I could do something quick and easy to address your concerns, before we discuss and agree on a final solution.
>>>>> I have timed running "docker ps" on 2 different hosts, 30 times each. I saw a spread between 20ms to 150ms, mostly centered around 30ms. Hence, I reduced the timeout in the VMProps.java from 10 sec to 500ms. I thought 500 ms gives enough time cushion for slower test nodes/hosts.
>>>> I think a slowness would arise on systems that do not have docker installed. If I time "docker ps" on my system I get
>>>>
>>>>  > time docker ps
>>>> The program 'docker' is currently not installed.  To run 'docker' please ask your administrator to install the package 'docker'
>>>>
>>>> real    0m4.103s
>>>> user    0m0.044s
>>>> sys     0m0.036s
>>>>
>>>> but the second call is much quicker:
>>>>
>>>>  > time docker ps
>>>> The program 'docker' is currently not installed.  To run 'docker' please ask your administrator to install the package 'docker'
>>>>
>>>> real    0m0.078s
>>>> user    0m0.052s
>>>> sys     0m0.012s
>>>>
>>>> It may well depend on the user's path. If "docker" is present early in the path then it will be found and executed quickly, but if not present, or if further down the path it may take a while to occur.
>>>>
>>>> So I think the timeout change, as a short term proposal, may be more risky and so not worthwhile.
>>>>
>>>> I'd prefer to see the whole "docker ps" disappear altogether. Such an approach is not scalable if there may be different container systems.
>>> I have discussed this topic again with Igor and Leonid, and explained your concerns.
>>>
>>>
>>> We came up with alternative ideas:
>>>
>>> 1. VMProps.java will check the existence of docker on a given test machine. We will look at "/bin/docker", "/usr/bin/docker", "/usr/local/bin/docker"
>>>     For now we only support Linux x64. If any of these files exist, we return 'true' for a corresponding at-requires property, otherwise false.
>>>
>>> 2. In case someone wishes to specify a custom location for a docker binary, we thought we could use a property for that:
>>>      jdk.test.lib.docker.path=""
>>>     If this property is specified, VMProps.java will check the docker at that location.
>>>
>>> 3. No more "docker ps" execution in VMProps.java.
>>>
>>> Please let me know what you think.
>> Just having checking for the existence of /bin/docker doesn’t means that it’s properly configured for use by any test process.
>> There’s permission setup, possible proxy setup etc.   We had issues with our internal testing systems not having the tools we’ve
>> needed and it’s a pain to have to go to each system in order to clear the way to successfully run all the tests.
> Yes, I agree with Bob, it is important to check that the docker is runnable by the test user or test agent.
> Given this, I propose to check 'docker ps' with a small timeout, say 300ms. However, instead of doing 'docker ps', I will do the '<full path>/docker ps' with the docker binary that I have found on a given system.
> This way we run 'docker ps' ONLY if we found it on the system. And, we give a process runner an exact location of the binary which will eliminate a search on the PATH, which in turn allows us to make wait timeout smaller, hence reducing impact/cost on VMProps evaluation.
>
> Does it work for you Bob? David, are you OK with this approach?

I think this is an improvement and I’m ok with this approach but I don’t see that it will save that much time.  
Starting docker takes  a while (worse the first time).  I’ve seen 1sec startup times.  Let see what David thinks
of the /tmp caching idea.  You’d have to put the user that found and successfully ran docker and have to deal
with multi-process write access to the file.

Bob.


>
>>
>> This is probably breaking new ground, but can we cache the result of “docker ps” in /tmp in order to eliminate the performance
>> overhead for all but the first time a new user runs these tests?
> I try to keep this solution simple. If you and David approve the solution above, I will implement and test it. If we find any problems, either during my testing or during test production, we could look into caching mechanisms.
>
>
> Thank you,
> Misha
>>
>> Bob.
>
>>>
>>>
>>> Thank you,
>>> Misha
>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Let me know what you think. I can revert this part of the change or reduce the timeout value a bit more.
>>>>>
>>>>> Thank you,
>>>>> Misha
>>>>>>> For this change under review, if I do not hear opinions from other reviewers, I can simply delete the print statement.
>>>>>> Sounds good to me.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> Thank you,
>>>>>>> Misha
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>>>> Please review this simple change that makes the error message in jtreg-ext/requires/VMProps.java conditional
>>>>>>>>> on java property "vmprops.trace.verbose". W/o this condition an error message is printed each time any jtreg
>>>>>>>>> test is ran, including non-docker tests.
>>>>>>>>>
>>>>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ======= Background and details:
>>>>>>>>> VMProps.java is called for each test run of jtreg (be it a run containing a singe test or multiple tests).
>>>>>>>>> VMProps evaluates the @requires properties. Therefore, in case of this bug, any time a user runs any jtreg VM test
>>>>>>>>> on a machine w/o an operational docker engine, this error will pop up.
>>>>>>>>> The fix makes printing of this error message conditional, off by default. The message can be turend on
>>>>>>>>> via a property vmprops.trace.verbose when detailed diagnostic info is desired.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ======== Testing:
>>>>>>>>> Local testing:
>>>>>>>>>    W/o docker present:
>>>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>>>        No error message
>>>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>        Detailed error message
>>>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>>>        Test skipped
>>>>>>>>>
>>>>>>>>>    With docker installed but no permissions:
>>>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>>>        No error message
>>>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>        Detailed error message
>>>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>>>        Test skipped
>>>>>>>>>
>>>>>>>>>    With docker installed and operational:
>>>>>>>>>      jtreg <arbitrary-jtreg-test>
>>>>>>>>>        No error message
>>>>>>>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>        No error message
>>>>>>>>>      jtreg DockerBasicTest.java
>>>>>>>>>        Test passed
>>>>>>>>>
>>>>>>>>> Testing via automated distributed test system:
>>>>>>>>>    - tier1
>>>>>>>>>    - docker tests
>>>>>>>>>    In progress
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Misha
>>>>>>>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov


On 11/07/2017 11:47 AM, Bob Vandette wrote:

>> On Nov 7, 2017, at 2:25 PM, mikhailo <[hidden email]> wrote:
>>
>>
>>
>> On 11/07/2017 09:04 AM, Bob Vandette wrote:
>>>> On Nov 6, 2017, at 5:42 PM, mikhailo <[hidden email]> wrote:
>>>>
>>>> Hi David,
>>>>
>>>>
>>>> On 11/05/2017 07:28 PM, David Holmes wrote:
>>>>> Hi Misha,
>>>>>
>>>>> On 4/11/2017 10:21 AM, Mikhailo Seledtsov wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> Updated webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.01/
>>>>>>
>>>>>> Please see my comments below.
>>>>>>
>>>>>> On 11/2/17, 6:02 PM, David Holmes wrote:
>>>>>>> Hi Misha,
>>>>>>>
>>>>>>> On 3/11/2017 10:22 AM, mikhailo wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>>     Thank you for taking a look at this change.
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/02/2017 04:47 PM, David Holmes wrote:
>>>>>>>>> Hi Misha,
>>>>>>>>>
>>>>>>>>> I don't see anything else in VMProps that writes anything out so to me the fix is to delete the System.err.println completely.
>>>>>>>> Removal is an option, and seems a simpler one. However, we will loose the diagnostic information.
>>>>>>>> On the other hand, if an issue arises on a specific host or system, I can patch this file with extra trace statements, and diagnose the failure that way.
>>>>>>>> If you think removing the print statement is a better option, and I do not hear objections or other opinions, I can just remove the print statement.
>>>>>>> Delete please :)
>>>>>>>
>>>>>> Done.
>>>>>>>>> That said I did not look at this in the past but I feel the whole docker test here is not really appropriate/suitable to always be run on linux-x64. It has to exec another process and wait up to 10 seconds to get a result! Can't this actually be done via a WhiteBox check once Bob's container support updates are in place?
>>>>>>>> Actually, WhiteBox is not necessary for such check. The ability to run docker on a given host/node can be determined in a body of a jtreg test, via a test utility. In fact, that was my original intent. When this change was presented for a discussion, this aspect was debated and reviewers have reached a consensus to do such check in VMProps.java. We mainly discussed within VM SQE team. The main argument was that using @requires is a more proper way of "skipping" the test(s) on unsupported environments, rather then checking this in the body of the test and then returning. When using @requires, the test execution system will report the test as "not run". When skipping the test using "return" from the body of a test, the execution system will report test as "ran" and passed; jtreg does not yet support the "skipped" state.
>>>>>>>>
>>>>>>>> Unfortunately, the way our "@requires" mechanism works is that it runs each time prior to starting a test run, even if the test being run is not concerned with a specific property. It was my concern that this could be a burden on the test run startup, but I eventually agreed to the opinion of other reviewers.
>>>>>>>>
>>>>>>>> If you have a strong opinion or concern about this, let me know. However, I am not sure what is the best process to use to overturn a decision on a prior review or design discussion. I guess one option is to file a bug or an RFE, and bring it up for a discussion.
>>>>>>> Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using WhiteBox connected to Bob's new container support code is better than exec'ing a separate process (which I'm not even sure what it does).
>>>>>> I have filed a bug: "JDK-8190730 - [TESTBUG] Calling 'docker ps' from VMProps.java to determine presence of docker engine is deemed too heavy"
>>>>>>
>>>>>> Also, in current webrev, I though I could do something quick and easy to address your concerns, before we discuss and agree on a final solution.
>>>>>> I have timed running "docker ps" on 2 different hosts, 30 times each. I saw a spread between 20ms to 150ms, mostly centered around 30ms. Hence, I reduced the timeout in the VMProps.java from 10 sec to 500ms. I thought 500 ms gives enough time cushion for slower test nodes/hosts.
>>>>> I think a slowness would arise on systems that do not have docker installed. If I time "docker ps" on my system I get
>>>>>
>>>>>   > time docker ps
>>>>> The program 'docker' is currently not installed.  To run 'docker' please ask your administrator to install the package 'docker'
>>>>>
>>>>> real    0m4.103s
>>>>> user    0m0.044s
>>>>> sys     0m0.036s
>>>>>
>>>>> but the second call is much quicker:
>>>>>
>>>>>   > time docker ps
>>>>> The program 'docker' is currently not installed.  To run 'docker' please ask your administrator to install the package 'docker'
>>>>>
>>>>> real    0m0.078s
>>>>> user    0m0.052s
>>>>> sys     0m0.012s
>>>>>
>>>>> It may well depend on the user's path. If "docker" is present early in the path then it will be found and executed quickly, but if not present, or if further down the path it may take a while to occur.
>>>>>
>>>>> So I think the timeout change, as a short term proposal, may be more risky and so not worthwhile.
>>>>>
>>>>> I'd prefer to see the whole "docker ps" disappear altogether. Such an approach is not scalable if there may be different container systems.
>>>> I have discussed this topic again with Igor and Leonid, and explained your concerns.
>>>>
>>>>
>>>> We came up with alternative ideas:
>>>>
>>>> 1. VMProps.java will check the existence of docker on a given test machine. We will look at "/bin/docker", "/usr/bin/docker", "/usr/local/bin/docker"
>>>>      For now we only support Linux x64. If any of these files exist, we return 'true' for a corresponding at-requires property, otherwise false.
>>>>
>>>> 2. In case someone wishes to specify a custom location for a docker binary, we thought we could use a property for that:
>>>>       jdk.test.lib.docker.path=""
>>>>      If this property is specified, VMProps.java will check the docker at that location.
>>>>
>>>> 3. No more "docker ps" execution in VMProps.java.
>>>>
>>>> Please let me know what you think.
>>> Just having checking for the existence of /bin/docker doesn’t means that it’s properly configured for use by any test process.
>>> There’s permission setup, possible proxy setup etc.   We had issues with our internal testing systems not having the tools we’ve
>>> needed and it’s a pain to have to go to each system in order to clear the way to successfully run all the tests.
>> Yes, I agree with Bob, it is important to check that the docker is runnable by the test user or test agent.
>> Given this, I propose to check 'docker ps' with a small timeout, say 300ms. However, instead of doing 'docker ps', I will do the '<full path>/docker ps' with the docker binary that I have found on a given system.
>> This way we run 'docker ps' ONLY if we found it on the system. And, we give a process runner an exact location of the binary which will eliminate a search on the PATH, which in turn allows us to make wait timeout smaller, hence reducing impact/cost on VMProps evaluation.
>>
>> Does it work for you Bob? David, are you OK with this approach?
> I think this is an improvement and I’m ok with this approach but I don’t see that it will save that much time.
> Starting docker takes  a while (worse the first time).  I’ve seen 1sec startup times.  Let see what David thinks
> of the /tmp caching idea.  You’d have to put the user that found and successfully ran docker and have to deal
> with multi-process write access to the file.
OK. There is another option that will work well, but requires one extra
input from user. We could designate a property that would state whether
a host is docker capable. When user wishes to execute docker tests
locally she/he will set the property to true. When executed via Oracle
automated test system, the system will set this property to true on
docker capable test nodes. All that VMProps will do is check the
property, so no docker ps any more, not even searching for docker on a
known locations.

As for naming, I can propose this: jdk.test.lib.docker.capable.node.
Alternative suggestions of names welcome.

This makes the check clear, simple and reliable. What do you think?


Thank you,
Misha


>
> Bob.
>
>
>>> This is probably breaking new ground, but can we cache the result of “docker ps” in /tmp in order to eliminate the performance
>>> overhead for all but the first time a new user runs these tests?
>> I try to keep this solution simple. If you and David approve the solution above, I will implement and test it. If we find any problems, either during my testing or during test production, we could look into caching mechanisms.
>>
>>
>> Thank you,
>> Misha
>>> Bob.
>>>>
>>>> Thank you,
>>>> Misha
>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Let me know what you think. I can revert this part of the change or reduce the timeout value a bit more.
>>>>>>
>>>>>> Thank you,
>>>>>> Misha
>>>>>>>> For this change under review, if I do not hear opinions from other reviewers, I can simply delete the print statement.
>>>>>>> Sounds good to me.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Misha
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>>>>> Please review this simple change that makes the error message in jtreg-ext/requires/VMProps.java conditional
>>>>>>>>>> on java property "vmprops.trace.verbose". W/o this condition an error message is printed each time any jtreg
>>>>>>>>>> test is ran, including non-docker tests.
>>>>>>>>>>
>>>>>>>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>>>>       Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ======= Background and details:
>>>>>>>>>> VMProps.java is called for each test run of jtreg (be it a run containing a singe test or multiple tests).
>>>>>>>>>> VMProps evaluates the @requires properties. Therefore, in case of this bug, any time a user runs any jtreg VM test
>>>>>>>>>> on a machine w/o an operational docker engine, this error will pop up.
>>>>>>>>>> The fix makes printing of this error message conditional, off by default. The message can be turend on
>>>>>>>>>> via a property vmprops.trace.verbose when detailed diagnostic info is desired.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ======== Testing:
>>>>>>>>>> Local testing:
>>>>>>>>>>     W/o docker present:
>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>         No error message
>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>>         Detailed error message
>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>         Test skipped
>>>>>>>>>>
>>>>>>>>>>     With docker installed but no permissions:
>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>         No error message
>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>>         Detailed error message
>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>         Test skipped
>>>>>>>>>>
>>>>>>>>>>     With docker installed and operational:
>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>         No error message
>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>>         No error message
>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>         Test passed
>>>>>>>>>>
>>>>>>>>>> Testing via automated distributed test system:
>>>>>>>>>>     - tier1
>>>>>>>>>>     - docker tests
>>>>>>>>>>     In progress
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thank you,
>>>>>>>>>> Misha
>>>>>>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

David Holmes
Hi Misha,
On 8/11/2017 7:36 AM, mikhailo wrote:

>>> Does it work for you Bob? David, are you OK with this approach?
>> I think this is an improvement and I’m ok with this approach but I
>> don’t see that it will save that much time.
>> Starting docker takes  a while (worse the first time).  I’ve seen 1sec
>> startup times.  Let see what David thinks
>> of the /tmp caching idea.  You’d have to put the user that found and
>> successfully ran docker and have to deal
>> with multi-process write access to the file.
> OK. There is another option that will work well, but requires one extra
> input from user. We could designate a property that would state whether
> a host is docker capable. When user wishes to execute docker tests
> locally she/he will set the property to true. When executed via Oracle
> automated test system, the system will set this property to true on
> docker capable test nodes. All that VMProps will do is check the
> property, so no docker ps any more, not even searching for docker on a
> known locations.
>
> As for naming, I can propose this: jdk.test.lib.docker.capable.node.
> Alternative suggestions of names welcome.
>
> This makes the check clear, simple and reliable. What do you think?

This doesn't work if the intent is that we happen to add a
docker-enabled machine to a test pool. You can't selectively set the
property only when run on the right host (if you could then we'd only
run the tests on that host in the first place).

But you seemed to have moved from fixing this bug - 8189213 - to trying
to address the new RFE 8190730.

Thanks,
David

>
> Thank you,
> Misha
>
>
>>
>> Bob.
>>
>>
>>>> This is probably breaking new ground, but can we cache the result of
>>>> “docker ps” in /tmp in order to eliminate the performance
>>>> overhead for all but the first time a new user runs these tests?
>>> I try to keep this solution simple. If you and David approve the
>>> solution above, I will implement and test it. If we find any
>>> problems, either during my testing or during test production, we
>>> could look into caching mechanisms.
>>>
>>>
>>> Thank you,
>>> Misha
>>>> Bob.
>>>>>
>>>>> Thank you,
>>>>> Misha
>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> Let me know what you think. I can revert this part of the change
>>>>>>> or reduce the timeout value a bit more.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Misha
>>>>>>>>> For this change under review, if I do not hear opinions from
>>>>>>>>> other reviewers, I can simply delete the print statement.
>>>>>>>> Sounds good to me.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Misha
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>>>>>> Please review this simple change that makes the error message
>>>>>>>>>>> in jtreg-ext/requires/VMProps.java conditional
>>>>>>>>>>> on java property "vmprops.trace.verbose". W/o this condition
>>>>>>>>>>> an error message is printed each time any jtreg
>>>>>>>>>>> test is ran, including non-docker tests.
>>>>>>>>>>>
>>>>>>>>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>>>>>       Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ======= Background and details:
>>>>>>>>>>> VMProps.java is called for each test run of jtreg (be it a
>>>>>>>>>>> run containing a singe test or multiple tests).
>>>>>>>>>>> VMProps evaluates the @requires properties. Therefore, in
>>>>>>>>>>> case of this bug, any time a user runs any jtreg VM test
>>>>>>>>>>> on a machine w/o an operational docker engine, this error
>>>>>>>>>>> will pop up.
>>>>>>>>>>> The fix makes printing of this error message conditional, off
>>>>>>>>>>> by default. The message can be turend on
>>>>>>>>>>> via a property vmprops.trace.verbose when detailed diagnostic
>>>>>>>>>>> info is desired.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ======== Testing:
>>>>>>>>>>> Local testing:
>>>>>>>>>>>     W/o docker present:
>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>         No error message
>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>>>         Detailed error message
>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>         Test skipped
>>>>>>>>>>>
>>>>>>>>>>>     With docker installed but no permissions:
>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>         No error message
>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>>>         Detailed error message
>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>         Test skipped
>>>>>>>>>>>
>>>>>>>>>>>     With docker installed and operational:
>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>         No error message
>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>>>>>>>>>         No error message
>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>         Test passed
>>>>>>>>>>>
>>>>>>>>>>> Testing via automated distributed test system:
>>>>>>>>>>>     - tier1
>>>>>>>>>>>     - docker tests
>>>>>>>>>>>     In progress
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thank you,
>>>>>>>>>>> Misha
>>>>>>>>>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov


On 11/07/2017 06:01 PM, David Holmes wrote:

> Hi Misha,
> On 8/11/2017 7:36 AM, mikhailo wrote:
>>>> Does it work for you Bob? David, are you OK with this approach?
>>> I think this is an improvement and I’m ok with this approach but I
>>> don’t see that it will save that much time.
>>> Starting docker takes  a while (worse the first time).  I’ve seen
>>> 1sec startup times.  Let see what David thinks
>>> of the /tmp caching idea.  You’d have to put the user that found and
>>> successfully ran docker and have to deal
>>> with multi-process write access to the file.
>> OK. There is another option that will work well, but requires one
>> extra input from user. We could designate a property that would state
>> whether a host is docker capable. When user wishes to execute docker
>> tests locally she/he will set the property to true. When executed via
>> Oracle automated test system, the system will set this property to
>> true on docker capable test nodes. All that VMProps will do is check
>> the property, so no docker ps any more, not even searching for docker
>> on a known locations.
>>
>> As for naming, I can propose this: jdk.test.lib.docker.capable.node.
>> Alternative suggestions of names welcome.
>>
>> This makes the check clear, simple and reliable. What do you think?
>
> This doesn't work if the intent is that we happen to add a
> docker-enabled machine to a test pool. You can't selectively set the
> property only when run on the right host (if you could then we'd only
> run the tests on that host in the first place).
>
> But you seemed to have moved from fixing this bug - 8189213 - to
> trying to address the new RFE 8190730.
You are right. I got carried away with this discussion.

I should fix the "JDK-8189213 - [TESTBUG] Running jtreg tests on machine
without docker shows extra message" first, and then work on

"JDK-8190730 - [TESTBUG] Calling 'docker ps' from VMProps.java to
determine presence of docker engine is deemed too heavy".


So, the fix for this issue is simple, as we agreed before - remove the
message. Here is a webrev:
http://cr.openjdk.java.net/~mseledtsov/8189213.02/

May I count you as a Reviewer for JDK-8189213?


Thank you,
Misha


P.S. As for JDK-8190730, I will try to summarize the discussion in the
comments of the JBS issue.

>
> Thanks,
> David
>
>>
>> Thank you,
>> Misha
>>
>>
>>>
>>> Bob.
>>>
>>>
>>>>> This is probably breaking new ground, but can we cache the result
>>>>> of “docker ps” in /tmp in order to eliminate the performance
>>>>> overhead for all but the first time a new user runs these tests?
>>>> I try to keep this solution simple. If you and David approve the
>>>> solution above, I will implement and test it. If we find any
>>>> problems, either during my testing or during test production, we
>>>> could look into caching mechanisms.
>>>>
>>>>
>>>> Thank you,
>>>> Misha
>>>>> Bob.
>>>>>>
>>>>>> Thank you,
>>>>>> Misha
>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> Let me know what you think. I can revert this part of the
>>>>>>>> change or reduce the timeout value a bit more.
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Misha
>>>>>>>>>> For this change under review, if I do not hear opinions from
>>>>>>>>>> other reviewers, I can simply delete the print statement.
>>>>>>>>> Sounds good to me.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> Thank you,
>>>>>>>>>> Misha
>>>>>>>>>>> Thanks,
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>>>>>>> Please review this simple change that makes the error
>>>>>>>>>>>> message in jtreg-ext/requires/VMProps.java conditional
>>>>>>>>>>>> on java property "vmprops.trace.verbose". W/o this
>>>>>>>>>>>> condition an error message is printed each time any jtreg
>>>>>>>>>>>> test is ran, including non-docker tests.
>>>>>>>>>>>>
>>>>>>>>>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>>>>>>       Webrev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> ======= Background and details:
>>>>>>>>>>>> VMProps.java is called for each test run of jtreg (be it a
>>>>>>>>>>>> run containing a singe test or multiple tests).
>>>>>>>>>>>> VMProps evaluates the @requires properties. Therefore, in
>>>>>>>>>>>> case of this bug, any time a user runs any jtreg VM test
>>>>>>>>>>>> on a machine w/o an operational docker engine, this error
>>>>>>>>>>>> will pop up.
>>>>>>>>>>>> The fix makes printing of this error message conditional,
>>>>>>>>>>>> off by default. The message can be turend on
>>>>>>>>>>>> via a property vmprops.trace.verbose when detailed
>>>>>>>>>>>> diagnostic info is desired.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> ======== Testing:
>>>>>>>>>>>> Local testing:
>>>>>>>>>>>>     W/o docker present:
>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>         No error message
>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>         Detailed error message
>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>         Test skipped
>>>>>>>>>>>>
>>>>>>>>>>>>     With docker installed but no permissions:
>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>         No error message
>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>         Detailed error message
>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>         Test skipped
>>>>>>>>>>>>
>>>>>>>>>>>>     With docker installed and operational:
>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>         No error message
>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>         No error message
>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>         Test passed
>>>>>>>>>>>>
>>>>>>>>>>>> Testing via automated distributed test system:
>>>>>>>>>>>>     - tier1
>>>>>>>>>>>>     - docker tests
>>>>>>>>>>>>     In progress
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thank you,
>>>>>>>>>>>> Misha
>>>>>>>>>>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

David Holmes
On 9/11/2017 1:22 AM, mikhailo wrote:

>
>
> On 11/07/2017 06:01 PM, David Holmes wrote:
>> Hi Misha,
>> On 8/11/2017 7:36 AM, mikhailo wrote:
>>>>> Does it work for you Bob? David, are you OK with this approach?
>>>> I think this is an improvement and I’m ok with this approach but I
>>>> don’t see that it will save that much time.
>>>> Starting docker takes  a while (worse the first time).  I’ve seen
>>>> 1sec startup times.  Let see what David thinks
>>>> of the /tmp caching idea.  You’d have to put the user that found and
>>>> successfully ran docker and have to deal
>>>> with multi-process write access to the file.
>>> OK. There is another option that will work well, but requires one
>>> extra input from user. We could designate a property that would state
>>> whether a host is docker capable. When user wishes to execute docker
>>> tests locally she/he will set the property to true. When executed via
>>> Oracle automated test system, the system will set this property to
>>> true on docker capable test nodes. All that VMProps will do is check
>>> the property, so no docker ps any more, not even searching for docker
>>> on a known locations.
>>>
>>> As for naming, I can propose this: jdk.test.lib.docker.capable.node.
>>> Alternative suggestions of names welcome.
>>>
>>> This makes the check clear, simple and reliable. What do you think?
>>
>> This doesn't work if the intent is that we happen to add a
>> docker-enabled machine to a test pool. You can't selectively set the
>> property only when run on the right host (if you could then we'd only
>> run the tests on that host in the first place).
>>
>> But you seemed to have moved from fixing this bug - 8189213 - to
>> trying to address the new RFE 8190730.
> You are right. I got carried away with this discussion.
>
> I should fix the "JDK-8189213 - [TESTBUG] Running jtreg tests on machine
> without docker shows extra message" first, and then work on
>
> "JDK-8190730 - [TESTBUG] Calling 'docker ps' from VMProps.java to
> determine presence of docker engine is deemed too heavy".
>
>
> So, the fix for this issue is simple, as we agreed before - remove the
> message. Here is a webrev:
> http://cr.openjdk.java.net/~mseledtsov/8189213.02/
>
> May I count you as a Reviewer for JDK-8189213?

Yes - and this is a trivial change so only one review needed.

Thanks,
David

>
> Thank you,
> Misha
>
>
> P.S. As for JDK-8190730, I will try to summarize the discussion in the
> comments of the JBS issue.
>
>>
>> Thanks,
>> David
>>
>>>
>>> Thank you,
>>> Misha
>>>
>>>
>>>>
>>>> Bob.
>>>>
>>>>
>>>>>> This is probably breaking new ground, but can we cache the result
>>>>>> of “docker ps” in /tmp in order to eliminate the performance
>>>>>> overhead for all but the first time a new user runs these tests?
>>>>> I try to keep this solution simple. If you and David approve the
>>>>> solution above, I will implement and test it. If we find any
>>>>> problems, either during my testing or during test production, we
>>>>> could look into caching mechanisms.
>>>>>
>>>>>
>>>>> Thank you,
>>>>> Misha
>>>>>> Bob.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Misha
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>>> Let me know what you think. I can revert this part of the
>>>>>>>>> change or reduce the timeout value a bit more.
>>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Misha
>>>>>>>>>>> For this change under review, if I do not hear opinions from
>>>>>>>>>>> other reviewers, I can simply delete the print statement.
>>>>>>>>>> Sounds good to me.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>>> Thank you,
>>>>>>>>>>> Misha
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> David
>>>>>>>>>>>>
>>>>>>>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>>>>>>>> Please review this simple change that makes the error
>>>>>>>>>>>>> message in jtreg-ext/requires/VMProps.java conditional
>>>>>>>>>>>>> on java property "vmprops.trace.verbose". W/o this
>>>>>>>>>>>>> condition an error message is printed each time any jtreg
>>>>>>>>>>>>> test is ran, including non-docker tests.
>>>>>>>>>>>>>
>>>>>>>>>>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>>>>>>>       Webrev:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> ======= Background and details:
>>>>>>>>>>>>> VMProps.java is called for each test run of jtreg (be it a
>>>>>>>>>>>>> run containing a singe test or multiple tests).
>>>>>>>>>>>>> VMProps evaluates the @requires properties. Therefore, in
>>>>>>>>>>>>> case of this bug, any time a user runs any jtreg VM test
>>>>>>>>>>>>> on a machine w/o an operational docker engine, this error
>>>>>>>>>>>>> will pop up.
>>>>>>>>>>>>> The fix makes printing of this error message conditional,
>>>>>>>>>>>>> off by default. The message can be turend on
>>>>>>>>>>>>> via a property vmprops.trace.verbose when detailed
>>>>>>>>>>>>> diagnostic info is desired.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> ======== Testing:
>>>>>>>>>>>>> Local testing:
>>>>>>>>>>>>>     W/o docker present:
>>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>>         No error message
>>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>>         Detailed error message
>>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>>         Test skipped
>>>>>>>>>>>>>
>>>>>>>>>>>>>     With docker installed but no permissions:
>>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>>         No error message
>>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>>         Detailed error message
>>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>>         Test skipped
>>>>>>>>>>>>>
>>>>>>>>>>>>>     With docker installed and operational:
>>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>>         No error message
>>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>>         No error message
>>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>>         Test passed
>>>>>>>>>>>>>
>>>>>>>>>>>>> Testing via automated distributed test system:
>>>>>>>>>>>>>     - tier1
>>>>>>>>>>>>>     - docker tests
>>>>>>>>>>>>>     In progress
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>> Misha
>>>>>>>>>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

Mikhailo Seledtsov
Thank you,

Misha

On 11/8/17, 5:40 PM, David Holmes wrote:

> On 9/11/2017 1:22 AM, mikhailo wrote:
>>
>>
>> On 11/07/2017 06:01 PM, David Holmes wrote:
>>> Hi Misha,
>>> On 8/11/2017 7:36 AM, mikhailo wrote:
>>>>>> Does it work for you Bob? David, are you OK with this approach?
>>>>> I think this is an improvement and I’m ok with this approach but I
>>>>> don’t see that it will save that much time.
>>>>> Starting docker takes  a while (worse the first time).  I’ve seen
>>>>> 1sec startup times.  Let see what David thinks
>>>>> of the /tmp caching idea.  You’d have to put the user that found
>>>>> and successfully ran docker and have to deal
>>>>> with multi-process write access to the file.
>>>> OK. There is another option that will work well, but requires one
>>>> extra input from user. We could designate a property that would
>>>> state whether a host is docker capable. When user wishes to execute
>>>> docker tests locally she/he will set the property to true. When
>>>> executed via Oracle automated test system, the system will set this
>>>> property to true on docker capable test nodes. All that VMProps
>>>> will do is check the property, so no docker ps any more, not even
>>>> searching for docker on a known locations.
>>>>
>>>> As for naming, I can propose this:
>>>> jdk.test.lib.docker.capable.node. Alternative suggestions of names
>>>> welcome.
>>>>
>>>> This makes the check clear, simple and reliable. What do you think?
>>>
>>> This doesn't work if the intent is that we happen to add a
>>> docker-enabled machine to a test pool. You can't selectively set the
>>> property only when run on the right host (if you could then we'd
>>> only run the tests on that host in the first place).
>>>
>>> But you seemed to have moved from fixing this bug - 8189213 - to
>>> trying to address the new RFE 8190730.
>> You are right. I got carried away with this discussion.
>>
>> I should fix the "JDK-8189213 - [TESTBUG] Running jtreg tests on
>> machine without docker shows extra message" first, and then work on
>>
>> "JDK-8190730 - [TESTBUG] Calling 'docker ps' from VMProps.java to
>> determine presence of docker engine is deemed too heavy".
>>
>>
>> So, the fix for this issue is simple, as we agreed before - remove
>> the message. Here is a webrev:
>> http://cr.openjdk.java.net/~mseledtsov/8189213.02/
>>
>> May I count you as a Reviewer for JDK-8189213?
>
> Yes - and this is a trivial change so only one review needed.
>
> Thanks,
> David
>
>>
>> Thank you,
>> Misha
>>
>>
>> P.S. As for JDK-8190730, I will try to summarize the discussion in
>> the comments of the JBS issue.
>>
>>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> Thank you,
>>>> Misha
>>>>
>>>>
>>>>>
>>>>> Bob.
>>>>>
>>>>>
>>>>>>> This is probably breaking new ground, but can we cache the
>>>>>>> result of “docker ps” in /tmp in order to eliminate the performance
>>>>>>> overhead for all but the first time a new user runs these tests?
>>>>>> I try to keep this solution simple. If you and David approve the
>>>>>> solution above, I will implement and test it. If we find any
>>>>>> problems, either during my testing or during test production, we
>>>>>> could look into caching mechanisms.
>>>>>>
>>>>>>
>>>>>> Thank you,
>>>>>> Misha
>>>>>>> Bob.
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Misha
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> Let me know what you think. I can revert this part of the
>>>>>>>>>> change or reduce the timeout value a bit more.
>>>>>>>>>>
>>>>>>>>>> Thank you,
>>>>>>>>>> Misha
>>>>>>>>>>>> For this change under review, if I do not hear opinions
>>>>>>>>>>>> from other reviewers, I can simply delete the print statement.
>>>>>>>>>>> Sounds good to me.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>>> Thank you,
>>>>>>>>>>>> Misha
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> David
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>>>>>>>>>>>>> Please review this simple change that makes the error
>>>>>>>>>>>>>> message in jtreg-ext/requires/VMProps.java conditional
>>>>>>>>>>>>>> on java property "vmprops.trace.verbose". W/o this
>>>>>>>>>>>>>> condition an error message is printed each time any jtreg
>>>>>>>>>>>>>> test is ran, including non-docker tests.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>>>>>>>>>>>>       Webrev:
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ======= Background and details:
>>>>>>>>>>>>>> VMProps.java is called for each test run of jtreg (be it
>>>>>>>>>>>>>> a run containing a singe test or multiple tests).
>>>>>>>>>>>>>> VMProps evaluates the @requires properties. Therefore, in
>>>>>>>>>>>>>> case of this bug, any time a user runs any jtreg VM test
>>>>>>>>>>>>>> on a machine w/o an operational docker engine, this error
>>>>>>>>>>>>>> will pop up.
>>>>>>>>>>>>>> The fix makes printing of this error message conditional,
>>>>>>>>>>>>>> off by default. The message can be turend on
>>>>>>>>>>>>>> via a property vmprops.trace.verbose when detailed
>>>>>>>>>>>>>> diagnostic info is desired.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ======== Testing:
>>>>>>>>>>>>>> Local testing:
>>>>>>>>>>>>>>     W/o docker present:
>>>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>>>         No error message
>>>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>>>         Detailed error message
>>>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>>>         Test skipped
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>     With docker installed but no permissions:
>>>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>>>         No error message
>>>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>>>         Detailed error message
>>>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>>>         Test skipped
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>     With docker installed and operational:
>>>>>>>>>>>>>>       jtreg <arbitrary-jtreg-test>
>>>>>>>>>>>>>>         No error message
>>>>>>>>>>>>>>       jtreg -Dvmprops.trace.verbose=true
>>>>>>>>>>>>>> <arbitrary-jtreg-test>
>>>>>>>>>>>>>>         No error message
>>>>>>>>>>>>>>       jtreg DockerBasicTest.java
>>>>>>>>>>>>>>         Test passed
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Testing via automated distributed test system:
>>>>>>>>>>>>>>     - tier1
>>>>>>>>>>>>>>     - docker tests
>>>>>>>>>>>>>>     In progress
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>>> Misha
>>>>>>>>>>>>>>
>>>>
>>