Openjdk-tests: Windows jdk16 test run hangs retrieving openjdk source

[adamfarley]

Describe the bug
A hang during a git ls-remote call to the jdk16 source repo.

The git ls-remote call appears to be the ant openjdk-tests/openjdk/build.xml's way of determining whether the jdk source repo we want is that of an upgrade jdk or not (jdk16.git vs jdk16.git) if the job's JDK_REPO parameter is empty.

However, sometimes we'll make the call, and the process will hang. This behaviour can be caused by ls-remote responding with a request for a username. The process stops there, and we then have a 9-hour wait until the jenkins job times out.

Problem: The build.xml script is not set up to perceive the login as a sign that the openjdk-jdk${JDK_VERSION}u.git repo is not accessible, and that the non-u repo "openjdk-jdk${JDK_VERSION}.git" repo should be used instead (as I interpret a restricted-access jdk source repo as a sign the repo is not ready to be used yet).

Solution: Somehow transform the request for a user ID into a failure that the build.xml can correctly interpret.

To Reproduce
Run a Windows grinder for jdk16 on any openjdk regression test target (e.g. jdk_foreign_native), specifying a jdk16 nightly jdk, but leaving the JDK_REPO parameter blank.

Expected behavior
I expect that either the user login should be interpreted as a failure immediately, or (second best) that a timeout should be set for the exec task running that command, with a timeout of more than a few seconds taken as evidence that the non-u repo should be used.

Screenshots

15:58:21  getOpenjdk:
15:58:23       [echo] git ls-remote -h https://github.com/AdoptOpenJDK/openjdk-jdk16u.git
Cancelling nested steps due to timeout
01:44:18  Sending interrupt signal to process

Additional context
Grinder has only run on test-azure-win2012r2-x64-2 so far, but this issue has been reproduced locally on 2 other machines.

[adamfarley]

Exploring build.xml options now. One idea is to add a timeout to the exec task and enable failure, but nest it in a trycatch block. Failure due to timeout or otherwise results in us setting the jdk location to the non-u repo and continuing.

[smlambert]

FYI @sophia-guo

My understanding of the build.xml code is that it calls ls-remote to check if the jdku repo exists, if not, assumes and tries jdk repo.

[adamfarley]

Yep. However, if the repo exists but has restricted access (e.g. if it's not ready for public use yet), we get a request for a username that we're not handling.

[adamfarley]

Ok, the "timeout" approach works, but the output may confuse folks. Will add some error messages.

EDIT: Yup, that looks much clearer. Removing the escape characters though, as that didn't work as expected.

18:47:04  getOpenjdk:

18:47:04       [echo] Using a git ls-remote command to determine if the jdk source repo has an available \'u\' version yet.
18:47:04       [echo] git ls-remote -h https://github.com/AdoptOpenJDK/openjdk-jdk16u.git

18:47:27       [exec] Username for 'https://github.com': 
18:47:27       [exec] Password for 'https://github.com': 
18:47:27       [echo] Command failed, or timed out waiting for a username due to access restrictions.
18:47:27       [echo] Setting repo name to the non-\'u\' version.
18:47:27       [echo] git clone --depth 1 -q -b dev https://github.com/AdoptOpenJDK/openjdk-jdk16.git

Still feels hacky tho. Is there a good way to detect a request for a username? Or perhaps a way to tell the git command to fail immediately if the username is requested? Investigating.

[adamfarley]

Ok, one website says setting GIT_TERMINAL_PROMPT=0 should cause an immediate failure if git wants to ask for a username and password. This works locally. Trying that in my sandboxed build.xml.

Also, I'll probably keep my timeout change so we can still run the git ls-remote command on versions of git older than when GIT_TERMINAL_PROMPT was implemented (which I believe is 2.3).

EDIT: The sandboxed build.xml works fine. Creating a PR.

[llxia]

Just to understand the problem, this only happens in Grinder when users do not specify JDK_REPO param and the default JDK_REPO happens to have restricted access? Does it only happen on windows?

[sophia-guo]

If the repo exists but has restricted access I would expect job exits immediately. This should only happen in grinder with no JDK_REPO passing in ( nightly builds pass in JDK_REPO and JDK_BRANCH explicitly). I believe that we expect that developers set JDK_REPO and JDK_BRANCH explicitly. The convoluted logic contained in openjdk-tests/openjdk/build.xml will be removed eventually https://github.com/AdoptOpenJDK/openjdk-tests/issues/1872#issue-653505029.

Going forward this case ( change from jdkversion.git to jdkversionu.git)should not happen anymore as openjdk/jdk has moved to github since jdk16 https://github.com/openjdk/jdk16? Hence no need to use git ls-remote command .

[adamfarley]

Lan: Correct (where "default" is ${jdk_version}u), and yes. We run the ls-remote command on other platforms too, but there we see immediate failure if we lack login credentials, e.g.

fatal: could not read Username for 'https://github.com': No such device or address

It seems only Windows has the problem where it asks the user for a username and hangs around forever waiting for it.

Sophia: I'd expect that too, but it appears this is not the case on Windows. If we lack access, it just asks for a username and waits around forever. I wouldn't say "expect" though, as we don't make the field mandatory in any clear way, we just hang when it isn't specified.

Removing the code eventually sounds good, but we'll still need to handle this scenario until then. Since my code is already written, I see no harm in including it, thus bringing Windows' grinder behaviour on jdk16 in line with behaviour on other platforms.

Also, I see you've linked to your own comment. I don't understand why.

[adamfarley]

PR created to handle the problem: https://github.com/AdoptOpenJDK/openjdk-tests/pull/2287

When the entire code block is removed entirely at some point in the future, this problem will go away anyway, but I still think it's good to get this change in because it prevents the issue until then.

[smlambert]

So if I understand correctly, this is a case where someone would have to be running a Grinder on jdk16+ on windows in order to encounter this issue, and you want to steam roll in a change for that narrow case instead of applying the workaround of filling in an extra field in the Grinder?

The fix for this should be what Sophia suggested, which is to say if jdk version is 16+ then assume there is no 'u'. Let's confirm that will be the case and jump directly to the real solution instead of the temporary retry fix for the narrow edge case.

[sophia-guo]

@adamfarley sorry, wrong link. Here it is the issue related https://github.com/AdoptOpenJDK/openjdk-tests/issues/1872#issue-653505029

[adamfarley]

This problem is only limited to jdk16 at the moment. Ultimately, it applies to every version of the jdk that doesn't have 'u' repo.

As for being a narrow case, this behaviour is only seen on Windows because all other platforms appear to do this already. This change is purely to bring the Windows behaviour in line with all the other platforms. And yes, it does prevent grinder users from having to remember to use a workaround that's only necessary on one platform.

Next up, steam-rolling. I announced this fix on both the #Testing Slack channel and this issue so that everyone involved with this issue would see it.

Then we have "assume there is no 'u' for 16+". I'll admit I didn't understand what Sophia meant by that, and I should have asked. Are you saying that for jdk16+, both here at Adopt and at upstream OpenJDK, there will be no update projects demarcated with a "u" after the version number?

[adamfarley]

Also, thanks for the link Sophia. :)

While making the setting of JDK_REPO strongly recommended seems a good idea, and making it necessary for pipelines could avoid ambiguity, I think that having automated JDK_REPO identification helps keep grinder runs as simple as possible.

[smlambert]

Apologies, steamrolling seems now like a very loaded term... by that I meant push in a change when there were several voices in the issue recommending a different approach (and given the issue the PR is intended to fix, no dire urgency to patch, but can spend a bit more time figuring out if the more final solution can be completed at this time).

[adamfarley]

Apology accepted. I've added "WIP" to the PR's title so we can debate further here without that PR getting merged by a 3rd party.

Is jdk16 (and up) not going to have an update project at all? At least, one represented by a 'u' on the end of the repo name?

[aahlenst]

Is jdk16 (and up) not going to have an update project at all? At least, one represented by a 'u' on the end of the repo name?

It already exists: https://github.com/openjdk/jdk16u

[adamfarley]

Thanks Andreas.

In that case, could @sophia-guo please clarify her stance?

[karianna]

Is jdk16 (and up) not going to have an update project at all? At least, one represented by a 'u' on the end of the repo name?

It already exists: https://github.com/openjdk/jdk16u

Note that it isn't really a 'valid' repo until after 16 gets released, all a bit backward I know :-(

[github-actions[bot]]

@karianna Build(s) successful.
Workflow Run ID: 578929466

[smlambert]

Updating here, based on the virtual meeting discussion with @adamfarley today.

What we did not know the answer to (and wanted to confirm, and now confirmed based on the existence of jdk16u), was whether the 16+ github repos will continue to use this naming pattern. It is also not very clear when that newly named repo gets created.

From the build pipelines, the openjdk repo / branch will be explicitly passed to test pipelines. The only time we need to 'guess' what openjdk repo you wish to use, is from Grinder jobs. It is preferred that people explicitly set it or use the default.

We will eventually remove the code in build.xml to try and figure out which repo to use, all that to be said, having seen the PR, I am not opposed to bringing it in, since the clean-up of build.xml is not imminent.

[github-actions[bot]]

@smlambert Build(s) successful.
Workflow Run ID: 578945848

[karianna]

Bot being over-excited here?

[github-actions[bot]]

@karianna Build(s) successful.
Workflow Run ID: 578958767

[smlambert]

https://github.com/AdoptOpenJDK/openjdk-tests/issues/2289

[github-actions[bot]]

@smlambert Build(s) successful.
Workflow Run ID: 578989831

[sophia-guo]

Going forward this case ( change from jdkversion.git to jdkversionu.git)should not happen anymore as openjdk/jdk has moved to github since jdk16 https://github.com/openjdk/jdk16?

@adamfarley I'm not sure about this as I'm using the question mark. I'm also not sure that if AdoptOpenJDK or Adoptium will
still do the mirror from mercurial as jdk moved from mercurial to github.

[github-actions[bot]]

@sophia-guo Build(s) successful.
Workflow Run ID: 579233921

[adamfarley]

PR is now merged. Closing issue, as we now have a workaround that should prevent this symptom until the code block gets removed entirely in the future.

Will mention it in the next call with Shelley, so she knows I'm not being sneaky. :)