RFR(M): 8189762: [TESTBUG] Create tests for JDK-8146115 container awareness and resource configuration

[Bob Vandette]

Do you still need this function?  I don’t see it being used.

  84     public static String read(String fileName) {
  85         String path = CGROUP_CPUSET_PATH + fileName;
  86         try {
  87             return readLineFromFile(path);
  88         } catch (Exception e) {
  89             System.err.println("Exception reading " + path);
  90             System.err.println("Exception: " + e);
  91         }
  92 
  93         return null;
  94     }

Bob.

> On Nov 9, 2017, at 2:15 PM, Mikhailo Seledtsov <mikhailo.seledtsov at oracle.com> wrote:
> 
> Here is a differential webrev:
>    http://cr.openjdk.java.net/~mseledtsov/8189762.01-02/webrev/
> 
> Misha
> 
> On 11/8/17, 5:26 PM, Mikhailo Seledtsov wrote:
>> Here is an update webrev:
>>    http://cr.openjdk.java.net/~mseledtsov/8189762.02/
>> I also tried to generate a diff between 01 and 02, but could not. My script does not seem to work any more, or I am missing something.
>> Igor, please let me know if you have scripts or command line to generate incremental webrev (I committed my initial changes locally, then applied the new changes on top).
>> 
>> Summary for updated webrev.02:
>> - implemented review feedback from Bob and Igor
>> - added @driver to all tests, since the actual testing happens in the child docker process; the main is just a test driver
>> - configured docker directory for exclusive access by jtreg tests, since there are potential and actual issues when running these tests concurrently
>> - modified test groups to add a hotspot_docker test group, and to exclude docker testing from lower tiers
>>    (once the infra is ready, I plan to add this execution initially at higher tiers; first will run it for a while as custom runs)
>> 
>> My next steps:
>>  - rebase to new jdk tree
>>  - update/merge
>>  - retest
>> 
>> Thank you,
>> Misha
>> 
>> 
>> On 11/6/17, 9:20 PM, Mikhailo Seledtsov wrote:
>>> Hi Igor,
>>> 
>>>  Thank you for reviewing the code.
>>> 
>>> On 11/6/17, 4:31 PM, Igor Ignatyev wrote:
>>>> Hi Misha,
>>>> 
>>>> I have several comments:
>>>> 1. whitebox.cpp : WB_IsContainerized has an incorrect signature, it should be WB_IsContainerized(JNIEnv*, jobject)
>>> Fixed
>>>> 2. CPUSetsReader.java : listToString can be rewritten using stream api as list.stream().limit(maxCount).map(Object::toString).collect(Collectors.joining(","))
>>> Thank you. I will try it out.
>>>> 3. in several files, I have noticed some problems w/ indents which make code quite hard to read, for example
>>>>>   73         Common.run( Common.newOpts(imageName)
>>>>>   74              .addDockerOpts("--memory", valueToSet))
>>>>>   75             .shouldMatch("Memory Limit is:.*" + expectedTraceValue);
>>>> or
>>>>>   96         DockerRunOptions opts = Common.newOpts(imageName, "AttemptOOM")
>>>>>   97             .addDockerOpts("--memory", dockerMemLimit, "--memory-swap", dockerMemLimit);
>>>>>   98             opts.classParams.add("" + sizeToAllocInMb);
>>> OK. I will unwind these expressions, and make sure indentation looks good.
>>>> 4. TestCPUSets.java : it's better to use Assert.assertEquals(parts.length, 2) than Asserts.assertTrue(parts.length == 2) as the former will also print the actual value of parts.length. so you won't need to have L#103
>>> OK
>>>> 5. Test*: there is no need to have parentheses in '@requires (docker.support)'
>>> OK
>>>> 6. Test*: ClassFileInstaller doesn't have to be run by JDK under test, so you can use '@run driver' to run it
>>> Will fix.
>>>> 7. TestCPUSets.java : L#72, to get a proper new line symbol you should use %n in format string. also System.out::printf seems to be more popular in our code than System.out::format
>>> OK.
>>>> 8. TestCPUSets.java: shouldn't checkResult assert that we found lineMarker?
>>> Oops. Missed it. Will add code to check it.
>>>> 9. Test*: would you consider adding .shouldHaveExitValue(0) to all Common.run calls which aren't expected to fail, e.g. all test* in TestCPUAwareness, testMemorySoftLimit and testMemoryLimit in TestMemoryAwareness
>>> Common.run() already checks for .shouldHaveExitValue(0) after running the container.
>>> 
>>> 
>>> I will apply feedback from Bob and you, and will post a new webrev.
>>> 
>>> 
>>> Thank you,
>>> Misha
>>>> 
>>>> Thanks,
>>>> -- Igor
>>>> 
>>>>> On Nov 1, 2017, at 8:11 PM, mikhailo<mikhailo.seledtsov at oracle.com>  wrote:
>>>>> 
>>>>> Please review these tests that were developed to test JVM's container awareness in Docker environment.
>>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8189762
>>>>>     Webrev:
>>>>>       Tests: http://cr.openjdk.java.net/~mseledtsov/8189762.00/
>>>>>       WB API: http://cr.openjdk.java.net/~mseledtsov/8189762.00.whitebox/
>>>>>     Testing:
>>>>>         1. Locally: Linux-x64, docker engine version: 17.06.2-ce
>>>>>            Ran the developed tests via jtreg
>>>>>            Pass
>>>>> 
>>>>>         2. Automated testing system - run these tests
>>>>>            In progress
>>>>> 
>>>>> Thank you,
>>>>> Misha
>>>>>