testDependency dir under C:/Users/jenkins/ should not be removed on windows

[AswathySK]

3rd party libs under C:/Users/jenkins/testDependency on Windows are pre-staged . However, this testDependency dir gets removed by this cleanup script since it removes patterns starting with test* in C:/Users/jenkins/ directory for windows machines.Due to missing pre-staged jars, the test jobs have to download the 3rd party jar at runtimes. The often cause test jobs failure because of unstable networks.

Helpful links:
https://github.ibm.com/runtimes/infrastructure/issues/9395

[AswathySK]

@keithc-ca , @pshipton can you please take a look at this PR?

[keithc-ca]

This change is not only for Windows, the title and description should not suggest otherwise.

[AswathySK]

This change is not only for Windows, the title and description should not suggest otherwise.

@keithc-ca , This issue is only for windows machines because in the script there is an additional portion for windows systems where they cleanup the /cygdrive/c/Users/jenkins/ directory where the testDependency is usually saved:

  cleanDirsStr += " ${buildWorkspace}/../../"
  cleanDirsStr += cleanDirs.join(" ${buildWorkspace}/../../")

Inorder to prevent this we should stop test* directories getting removed only from /cygdrive/c/Users/jenkins/ directory in windows.

This step could be a fix instead of the one I have made in this PR:

 if (nodeLabels.contains('sw.os.windows')) {
      // test resources
      cleanDirsStr += " ${buildWorkspace}/../../"
      cleanDirsStr += cleanDirs.join(" ${buildWorkspace}/../../")
      // shared classes cache
      cleanDirsStr += " ${buildWorkspace}/../../javasharedresources /tmp/javasharedresources /temp/javasharedresources"                                     
      cleanDirsStr = cleanDirsStr.replaceAll("${buildWorkspace}/../../test\\*", "")
                                    }

This way other test* directories will be removed from tmp for other OS and for windows testDependency directory wont be removed in /cygdrive/c/Users/jenkins/ directory.

[keithc-ca]

Could we not arrange that testDependency is someplace (outside the workspace) so it won't be deleted?

[AswathySK]

Could we not arrange that testDependency is someplace (outside the workspace) so it won't be deleted?

@llxia what are your thoughts on this?

[llxia]

testDependency dir is outside the Jenkins workspace. This clean-up job deletes files outside the workspace.

[AswathySK]

@llxia , Only in windows the cleanup job is clearing directories under /cygdrive/c/Users/jenkins/. Is this step required in windows?
Does the tests create directories apart from testDependencies in /cygdrive/c/Users/jenkins/ directory which should be removed?

[keithc-ca]

Ok, testDependency is outside the Jenkins workspace: shouldn't the cleanup job confine itself to the workspace? Then there would be no concern about removing files in testDependency that we want to keep.

[llxia]

Only in windows the cleanup job is clearing directories under /cygdrive/c/Users/jenkins/. Is this step required in windows?

I will leave @AdamBrousseau to comment on this one.

Does the tests create directories apart from testDependencies in /cygdrive/c/Users/jenkins/ directory which should be removed?

We have a separate clean-up job to periodically delete the test pipeline pre-staged files. This Cleanup-Nodes.groovy does not need to worry about any pre-staged files from the test pipeline under /cygdrive/c/Users/Jenkins/

shouldn't the cleanup job confine itself to the workspace?

I will try to answer this one. The purpose of Cleanup-Nodes.groovy is to remove any leftover files outside the workspace (i.e., from individual users, test jobs, etc). It tries to clean /tmp, /temp, /cores, etc.

[AdamBrousseau]

Only in windows the cleanup job is clearing directories under /cygdrive/c/Users/jenkins/. Is this step required in windows?

We can turn it off and keep an eye on it. At some point we noticed there was stuff in $HOME on Windows so we added it as a cleanup location.

Many tests create artifacts in $TEMP=. The cleanWs() command does not handle these artifacts because it is outside of the $WORKSPACE. Other than TEMP and HOME (on Windows), we also remove everything in workspace dir in case there are jobs that failed (or forgot) to cleanup after themselves.

To resolve this PR can we not just change the code to explicitly exclude removing the testDependency dir?

[keithc-ca]

change the code to explicitly exclude removing the testDependency dir

That was my request.

[AswathySK]

@keithc-ca , The change has been made.
@AdamBrousseau, who can I approach to make a list of dirs/files which will get created in jenkins directories after running tests. I want to understand what all should be maintained and what should be removed from the home dir. For example, is jck_root something which we should not delete?

[AdamBrousseau]

@AdamBrousseau, who can I approach to make a list of dirs/files which will get created in jenkins directories after running tests. I want to understand what all should be maintained and what should be removed from the home dir. For example, is jck_root something which we should not delete?

There should be nothing created by test in jenkins user's HOME dir. There is plenty created in /tmp as we know but we do not know enough about all the tests to know what test creates what artifact(s). @pshipton @llxia can confirm.
The main directories in HOME should be bootjdks, jck_root, openjdk_cache, workspace, and openj9_resources (only on z/os). I don't think we can take a blanket approach of deleting everything except certain directories though. That is too risky. We need to maintain a list of specific items we know are generated and not always cleaned up automatically and only remove those.

[keithc-ca]

The grep part of the pipeline should be grep -v '/testDependency$'.

[keithc-ca]

This, or cleanDirs has to change; as is, this still includes ${buildWorkspace}/../../test* which will remove the testDependency directory.

[AswathySK]

@keithc-ca cleanDirsStr = cleanDirsStr.replaceAll("${buildWorkspace}/../../test\\*", "") this line would exclude removing test* items from ${buildWorkspace}/../../ directory.

[keithc-ca]

Yes, I missed that line.

[keithc-ca]

I suggest a better approach would be to add 'test*' to cleanDirs only for non-Windows nodes.

[AswathySK]

@keithc-ca ,
I think even in windows we are required to clean test* dirs in /tmp directory. The only exclusion is for the testDependency dir. Is that not right @llxia @AdamBrousseau?

[keithc-ca]

So, are we agreed that we should change those jobs to use a different name than testDependency, say to external or support?

[AswathySK]

@llxia what should be the action taken?

[llxia]

@smlambert we are going to change testDependency to externalDependency at https://github.com/adoptium/aqa-tests/blob/2e34bbfe9931be699284120c04acf9a42680f71e/buildenv/jenkins/JenkinsfileBase#L108-L109. Do you have any objections?

FYI @annaibm @LongyuZhang

[smlambert]

Thanks @llxia. It will be good to raise an aqa-tests issue for awareness for others that the folder name will be changed to better support automatic cleaning of test material, while maintaining the benefit of preloading test dependencies. Other than communicating the change and checking that any references to that folder are accounted for, I have no objections.

[llxia]

adoptium/aqa-tests#5675 is created