Skip to content

Configure TMP for test nodes on Windows#39959

Merged
alpar-t merged 3 commits intoelastic:masterfrom
alpar-t:fix-clusterfomration-test-windows
Mar 13, 2019
Merged

Configure TMP for test nodes on Windows#39959
alpar-t merged 3 commits intoelastic:masterfrom
alpar-t:fix-clusterfomration-test-windows

Conversation

@alpar-t
Copy link
Copy Markdown
Contributor

@alpar-t alpar-t commented Mar 12, 2019

On windows TMP dir default to C:\Windows and startup fails with a permission error.
The reason we don't see this more is that typically windows has TMP defined in users environment, but with newenvironment: true this is not passed along.

Closes #39904.

This breaks on windows where TMP dir default to C:\Windows and startup
fails with a permission error.
I tried to create a tmp dir and pass in `TMP` env, but it lead to a
class not found error, and since testclusers is already independent of
the calling environment I stopped there.
@rjernst
Copy link
Copy Markdown
Member

rjernst commented Mar 12, 2019

The new environment is necessary; otherwise we pass through JAVA_HOME and always use that, even when we want to use the bundled jdk. I think we can solve this by passing through whatever env vars windows needs?

Fix incorrect argument for paths with spaces
@alpar-t
Copy link
Copy Markdown
Contributor Author

alpar-t commented Mar 13, 2019

Thanks @rjernst I was too quick on that one. I added TMP now, which required an additional fix since it happened to be a path with spaces.

@alpar-t alpar-t changed the title Don't start test nodes with a new environment Configure TMP for test nodes on Windows Mar 13, 2019
Copy link
Copy Markdown
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one optional suggestion

// Since we configure ant to run with a new environment above, we need to set this to a dir we have access to
File tmpDir = new File(node.baseDir, "tmp")
tmpDir.mkdirs()
env(key: "TMP", value: tmpDir.absolutePath)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we just pass through TMP? this would match the previous behavior where the environment was inherited

@alpar-t alpar-t merged commit 97562a8 into elastic:master Mar 13, 2019
@alpar-t alpar-t deleted the fix-clusterfomration-test-windows branch March 13, 2019 17:11
alpar-t added a commit that referenced this pull request Mar 13, 2019
This breaks on windows where TMP dir default to C:\Windows and startup
fails with a permission error.
I tried to create a tmp dir and pass in `TMP` env, but it lead to a
class not found error, and since testclusers is already independent of
the calling environment I stopped there.
alpar-t added a commit that referenced this pull request Mar 13, 2019
This breaks on windows where TMP dir default to C:\Windows and startup
fails with a permission error.
I tried to create a tmp dir and pass in `TMP` env, but it lead to a
class not found error, and since testclusers is already independent of
the calling environment I stopped there.
alpar-t added a commit to alpar-t/elasticsearch that referenced this pull request Mar 13, 2019
This breaks on windows where TMP dir default to C:\Windows and startup
fails with a permission error.
I tried to create a tmp dir and pass in `TMP` env, but it lead to a
class not found error, and since testclusers is already independent of
the calling environment I stopped there.
ebadyano added a commit to ebadyano/rally-teams that referenced this pull request Mar 14, 2019
After elasticsearch change, ${ES_TMPDIR} needs quotes around it.

Relates elastic/elasticsearch#39959
ebadyano added a commit to elastic/rally-teams that referenced this pull request Mar 14, 2019
After elasticsearch change, ${ES_TMPDIR} needs quotes around it.

Relates elastic/elasticsearch#39959
alpar-t added a commit to alpar-t/elasticsearch that referenced this pull request Mar 21, 2019
alpar-t added a commit that referenced this pull request Mar 26, 2019
* Revert "Configure TMP for test nodes on Windows (#39959)"
This reverts commit 97562a8.

* Configure a tmp dir without spaces
* Pass on TMP instead of changing it
alpar-t added a commit that referenced this pull request Mar 26, 2019
* Revert "Configure TMP for test nodes on Windows (#39959)"
This reverts commit 97562a8.

* Configure a tmp dir without spaces
* Pass on TMP instead of changing it
alpar-t added a commit that referenced this pull request Apr 4, 2019
* Revert "Configure TMP for test nodes on Windows (#39959)"
This reverts commit 97562a8.

* Configure a tmp dir without spaces
* Pass on TMP instead of changing it
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v7.0.0-rc1 v7.2.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integration tests fail on Windows

5 participants