[TESTBUG] runtime/containers/docker/TestCPUAwareness.java failed in docker not supporting --cpus
Bob Vandette
bob.vandette at oracle.com
Sun Feb 3 18:24:55 UTC 2019
Sure. Can you send me a patch ready to hg import and I’ll submit for you.
Bob.
> On Feb 3, 2019, at 12:20 PM, Ao Qi <aoqi at loongson.cn> wrote:
>
> Hi,
>
> Bob Vandette and David Holmes have reviewed. Could someone help to sponsor?
>
> Thanks,
> Ao Qi
>
>> On Fri, Feb 1, 2019 at 6:52 PM Ao Qi <aoqi at loongson.cn> wrote:
>>
>>> On Fri, Feb 1, 2019 at 11:58 AM David Holmes <david.holmes at oracle.com> wrote:
>>>
>>> Reviewed.
>>>
>>
>> Thanks, David.
>>
>>> I assume you also need a sponsor?
>>
>> Yes :)
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> On 1/02/2019 11:56 am, Ao Qi wrote:
>>>> Thanks, Bob. You help a lot.
>>>>
>>>> Could someone help to review this small change?
>>>>
>>>>> On Fri, Feb 1, 2019 at 12:30 AM Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>
>>>>> I’m not a “R” reviewer but I’m ok with your change.
>>>>>
>>>>> Bob.
>>>>>
>>>>>
>>>>>> On Jan 31, 2019, at 11:00 AM, Ao Qi <aoqi at loongson.cn> wrote:
>>>>>>
>>>>>> I leave the change, and update copyright year:
>>>>>> http://cr.openjdk.java.net/~aoqi/8217597/webrev.02/ Could you help to
>>>>>> review this?
>>>>>>
>>>>>> Thanks,
>>>>>> Ao Qi
>>>>>>
>>>>>>> On Thu, Jan 31, 2019 at 10:08 PM Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>>>
>>>>>>> Yes, that does provide a bit more unique testing so leave the change as you had it.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Bob.
>>>>>>>
>>>>>>>> On Jan 28, 2019, at 8:41 PM, Ao Qi <aoqi at loongson.cn> wrote:
>>>>>>>>
>>>>>>>> Hi Bob,
>>>>>>>>
>>>>>>>> Thanks! I am not a containers expert and have one small question. The
>>>>>>>> maximum amount of --cpus is 4 (equivalent to both setting —cpu-period
>>>>>>>> and —cpu-quota) in the already existing test. Is it valuable to keep
>>>>>>>> testCpus(i, i) according to the max num of available CPUs? If not, I
>>>>>>>> would also prefer removing the lines. In addition, I think I forgot to
>>>>>>>> update the copyright year, it will be fixed in the next version of
>>>>>>>> wevrev.
>>>>>>>>
>>>>>>>>> On Mon, Jan 28, 2019 at 11:35 PM Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>> There is already a test that verifies —cpu-period and —cpu-quota.
>>>>>>>>> I would just remove these lines.
>>>>>>>>>
>>>>>>>>> 62 // leave one CPU for system and tools, otherwise this test may be unstable
>>>>>>>>> 63 int maxNrOfAvailableCpus = availableCPUs - 1;
>>>>>>>>> 64 for (int i=1; i < maxNrOfAvailableCpus; i = i * 2) {
>>>>>>>>> 65 testCpus(i, i);
>>>>>>>>> 66 }
>>>>>>>>>
>>>>>>>>> 129 private static void testCpus(int valueToSet, int expectedTraceValue) throws Exception {
>>>>>>>>> 130 Common.logNewTestCase("test cpus: " + valueToSet);
>>>>>>>>> 131 DockerRunOptions opts = Common.newOpts(imageName)
>>>>>>>>> 132 .addDockerOpts("--cpus", "" + valueToSet);
>>>>>>>>> 133 Common.run(opts)
>>>>>>>>> 134 .shouldMatch("active_processor_count.*" + expectedTraceValue);
>>>>>>>>> 135 }
>>>>>>>>>
>>>>>>>>> Bob.
>>>>>>>>>
>>>>>>>>> On Jan 28, 2019, at 10:22 AM, Ao Qi <aoqi at loongson.cn> wrote:
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Since —cpus is a shortcut for of setting both --cpu-period and
>>>>>>>>> --cpu-quota and the test is not intended to verify that docker works
>>>>>>>>> correctly, I did not check the docker version and just replaced
>>>>>>>>> setting --cpus with setting both --cpu-period and --cpu-quota. What do
>>>>>>>>> you think of this patch:
>>>>>>>>> http://cr.openjdk.java.net/~aoqi/8217597/webrev.01/ ?
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Ao Qi
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed, Jan 23, 2019 at 11:37 PM Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Since —cpus is just a shortcut for of setting both --cpu-period and --cpu-quota”, I’d
>>>>>>>>> be ok with removing this test. The tests are intended to test the container/cgroup
>>>>>>>>> configuration detection logic and not to verify that docker works correctly.
>>>>>>>>>
>>>>>>>>> An alternate solution would be to add version detection to the Docker test check in
>>>>>>>>> DockerTestUtils.java . We already exec “docker ps” to see if docker is available
>>>>>>>>> and enabled.
>>>>>>>>>
>>>>>>>>> % docker --version
>>>>>>>>> Docker version 17.03.1-ce, build 276fd32
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Bob.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Jan 22, 2019, at 10:19 PM, Ao Qi <aoqi at loongson.cn> wrote:
>>>>>>>>>
>>>>>>>>> On Wed, Jan 23, 2019 at 10:55 AM David Holmes <david.holmes at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 23/01/2019 11:58 am, Ao Qi wrote:
>>>>>>>>>
>>>>>>>>> Hi David,
>>>>>>>>>
>>>>>>>>> On Wed, Jan 23, 2019 at 5:24 AM David Holmes <david.holmes at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> cc'ing Bob as our containers expert ...
>>>>>>>>>
>>>>>>>>> On 23/01/2019 1:10 am, Ao Qi wrote:
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> --cpus is available in Docker 1.13 and higher [1], so
>>>>>>>>> runtime/containers/docker/TestCPUAwareness.java failed in docker which
>>>>>>>>> does not support --cpus.
>>>>>>>>>
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~aoqi/docker/webrev.00/
>>>>>>>>>
>>>>>>>>> This patch skips the test if --cpus is not supported. I tested
>>>>>>>>> runtime/containers/docker/TestCPUAwareness.java on a Fedora 25 (Docker
>>>>>>>>> version 1.12.6, build ae7d637/1.12.6, not supporting --cpus) and
>>>>>>>>> Ubuntu 16.04 (Docker version 17.03.2-ce, build f5ec1e2, supporting
>>>>>>>>> --cpus)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The patch causes the test to pass if launching Docker fails for any
>>>>>>>>> reason so that is not good.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I tested two versions of docker which does not support --cpus. Their
>>>>>>>>> exit values when using --cpus are 2 and 125, and outputs are:
>>>>>>>>>
>>>>>>>>> flag provided but not defined: --cpus
>>>>>>>>> See 'docker run --help'.
>>>>>>>>>
>>>>>>>>> and
>>>>>>>>>
>>>>>>>>> unknown flag: --cpus
>>>>>>>>> See 'docker run --help'.
>>>>>>>>>
>>>>>>>>> My initial thought was that the else condition of
>>>>>>>>> "output.getExitValue() == 0" should match the condition of "--cpus not
>>>>>>>>> supported". Firstly I used output.shouldMatch("docker run --help"),
>>>>>>>>> but I am not sure if all the docker version behaves this way when
>>>>>>>>> --cpus is not supported and "docker run --help" does not certainly
>>>>>>>>> indicate "--cpus not supported", so I removed the else condition.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think we need to try and find a way to clearly identifyt eh failing
>>>>>>>>> condition. Is there are "docker --version" we coudl check?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I will do more research. Checking docker version may be one option,
>>>>>>>>> and checking whether one option is support by docker may be also one
>>>>>>>>> option. I will try them later, while waiting if there are some other
>>>>>>>>> opinions :)
>>>>>>>>>
>>>>>>>>> I am not sure if this is a testbug, so I did not file it on JBS. In
>>>>>>>>> fact, I am not quite sure what kind of issue can be filed on JBS. Is
>>>>>>>>> there any guidance document?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Any/all issues can be filed on JBS. You don't need to pre-classify as a
>>>>>>>>> testbug, simple create an issue that a test is failing under specific
>>>>>>>>> conditions. Whomever works on the bug will then determine whether it is
>>>>>>>>> a testbug or product issue or something else. (We don't seem to have any
>>>>>>>>> docs on using JBS ...)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> What if the issue is not a bug or no body cares the issue? The issue
>>>>>>>>> will be open on JBS forever?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Possibly :) But each component team performs regular triage of the bugs
>>>>>>>>> that get filed and eventually things will be examined enough to see if
>>>>>>>>> they are indeed a bug, and if not they will be closed as not an issue.
>>>>>>>>> If a bug but low priority it may eventually get closed as "will not fix"
>>>>>>>>> just to keep the open bug count down.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I was a little afraid that filing issues that are not bugs or nobody
>>>>>>>>> cares would increase the workload of others and frustrate myself, so I
>>>>>>>>> was not sure what kind of issue should be filed. Now I basically
>>>>>>>>> clear, thanks David.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>> Thanks for your explanation, and I filed this issue on JBS:
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8217597
>>>>>>>>>
>>>>>>>>> In this case I'm not sure whether we require a docker version that
>>>>>>>>> supports --cpus, and the test should be skipped otherwise. Though
>>>>>>>>> ideally this would involve an explicit version check so we don't just
>>>>>>>>> pass if the docker process fails.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Ao Qi
>>>>>>>>>
>>>>>>>>> [1] https://docs.docker.com/config/containers/resource_constraints/#cpu
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
More information about the hotspot-runtime-dev
mailing list