You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/09/09 22:49:57 UTC
[GitHub] [trafficserver] bneradt opened a new pull request #7177: Create an explicit runroot.yaml for AuTests
bneradt opened a new pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177
The ATS install directory may have its own runroot.yaml, but we don't
want AuTest to use that. The user specified a sandbox via autest and we
want the tests to use that location as the runroot. To ensure this, this
patch has the trafficserver extension create its own runroot.yaml in the
sandbox directory which the Traffic Server process will find via the
TS_ROOT environment variable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a change in pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#discussion_r486739064
##########
File path: tests/gold_tests/autest-site/trafficserver.test.ext
##########
@@ -125,7 +136,6 @@ def MakeATSProcess(obj, name, command='traffic_server', select_ports=True,
#########################################################
# create subdirectories that need to exist (but are empty)
# log directory has to be created with correct permissions
- p.Setup.MakeDir(log_dir) # log directory has to be created
p.Setup.Chown(log_dir, "nobody", "nobody", ignore=True)
# covers ubuntu's unprivileged group
p.Setup.Chown(log_dir, "nobody", "nogroup", ignore=True)
Review comment:
Interesting idea, but I don't think that's an option for the test. The `verify` sub command requires it to be run with root privileges:
```
ERROR: To fix permission issues, root privilege is required.
Please run with sudo.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#issuecomment-697516296
> It is not perfect .. but it a step in the direction we want I believe
Just to capture this in case it's helpful for the future: traffic_layout should be able to create a fully functional runroot. Using the command, as we are with this change, should therefore make it possible to remove most or all of the sandbox setup we do for ATS in the trafficserver extension (creating the conf directory, the storage directory, etc.). That was what Jason was getting at in his request. But as we tried to apply these changes, we found that traffic_layout doesn't seem to be complete in what it sets up so, while it does remove some of the TS sandbox creation burden from the extension, it doesn't remove all of it.
As Jason says, the current patch does improve things. To make more progress we'll have to update traffic_layout in the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#issuecomment-702439869
Others have found that on beefier hardware this causes timing related issues for TS starting up in AuTest. This isn't a critical feature, so I'm simply going to revert this. If we need this for build related stuff, I'll revisit how to use a runroot.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt merged pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
bneradt merged pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] dragon512 commented on a change in pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
dragon512 commented on a change in pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#discussion_r486488690
##########
File path: tests/gold_tests/autest-site/trafficserver.test.ext
##########
@@ -125,7 +136,6 @@ def MakeATSProcess(obj, name, command='traffic_server', select_ports=True,
#########################################################
# create subdirectories that need to exist (but are empty)
# log directory has to be created with correct permissions
- p.Setup.MakeDir(log_dir) # log directory has to be created
p.Setup.Chown(log_dir, "nobody", "nobody", ignore=True)
# covers ubuntu's unprivileged group
p.Setup.Chown(log_dir, "nobody", "nogroup", ignore=True)
Review comment:
I think we can replace these Chown with one `traffic_layout verify --fix` command
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] dragon512 commented on a change in pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
dragon512 commented on a change in pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#discussion_r492835470
##########
File path: tests/gold_tests/autest-site/trafficserver.test.ext
##########
@@ -125,7 +136,6 @@ def MakeATSProcess(obj, name, command='traffic_server', select_ports=True,
#########################################################
# create subdirectories that need to exist (but are empty)
# log directory has to be created with correct permissions
- p.Setup.MakeDir(log_dir) # log directory has to be created
p.Setup.Chown(log_dir, "nobody", "nobody", ignore=True)
# covers ubuntu's unprivileged group
p.Setup.Chown(log_dir, "nobody", "nogroup", ignore=True)
Review comment:
I think we need to tweak something more
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] dragon512 commented on a change in pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
dragon512 commented on a change in pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#discussion_r492835470
##########
File path: tests/gold_tests/autest-site/trafficserver.test.ext
##########
@@ -125,7 +136,6 @@ def MakeATSProcess(obj, name, command='traffic_server', select_ports=True,
#########################################################
# create subdirectories that need to exist (but are empty)
# log directory has to be created with correct permissions
- p.Setup.MakeDir(log_dir) # log directory has to be created
p.Setup.Chown(log_dir, "nobody", "nobody", ignore=True)
# covers ubuntu's unprivileged group
p.Setup.Chown(log_dir, "nobody", "nogroup", ignore=True)
Review comment:
I think we need to tweak something more
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#issuecomment-689930977
[approve ci rat]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall commented on a change in pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#discussion_r488278766
##########
File path: tests/gold_tests/autest-site/trafficserver.test.ext
##########
@@ -125,7 +136,6 @@ def MakeATSProcess(obj, name, command='traffic_server', select_ports=True,
#########################################################
# create subdirectories that need to exist (but are empty)
# log directory has to be created with correct permissions
- p.Setup.MakeDir(log_dir) # log directory has to be created
p.Setup.Chown(log_dir, "nobody", "nobody", ignore=True)
# covers ubuntu's unprivileged group
p.Setup.Chown(log_dir, "nobody", "nogroup", ignore=True)
Review comment:
@dragon512 ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a change in pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#discussion_r486739064
##########
File path: tests/gold_tests/autest-site/trafficserver.test.ext
##########
@@ -125,7 +136,6 @@ def MakeATSProcess(obj, name, command='traffic_server', select_ports=True,
#########################################################
# create subdirectories that need to exist (but are empty)
# log directory has to be created with correct permissions
- p.Setup.MakeDir(log_dir) # log directory has to be created
p.Setup.Chown(log_dir, "nobody", "nobody", ignore=True)
# covers ubuntu's unprivileged group
p.Setup.Chown(log_dir, "nobody", "nogroup", ignore=True)
Review comment:
Interesting idea, but I don't think that's an option for the test. The `verify` sub command requires it to be run with root privileges:
```
ERROR: To fix permission issues, root privilege is required.
Please run with sudo.
```
##########
File path: tests/gold_tests/autest-site/trafficserver.test.ext
##########
@@ -125,7 +136,6 @@ def MakeATSProcess(obj, name, command='traffic_server', select_ports=True,
#########################################################
# create subdirectories that need to exist (but are empty)
# log directory has to be created with correct permissions
- p.Setup.MakeDir(log_dir) # log directory has to be created
p.Setup.Chown(log_dir, "nobody", "nobody", ignore=True)
# covers ubuntu's unprivileged group
p.Setup.Chown(log_dir, "nobody", "nogroup", ignore=True)
Review comment:
Interesting idea, but I don't think that's an option for the test. The `verify` sub command requires it to be run with root privileges:
```
ERROR: To fix permission issues, root privilege is required.
Please run with sudo.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on pull request #7177: Create an explicit runroot.yaml for AuTests
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7177:
URL: https://github.com/apache/trafficserver/pull/7177#issuecomment-689934957
[approve ci autest]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org