On 8 Jun 2017, at 10:31, Jason Kenny wrote:
James,
to help clarify what we are doing. There currently is an issue in
which autest cannot correct test certain builds of ATS. The issue
comes from the need to point to a build and copy/hard link bits in to
a sandbox directory to run the test in a controlled environment. Todo
this we use TS_ROOT and information from traffic_layout to setup
directories in the sandbox so when we run the
traffic_server/manger/top it will look in the sandbox area for
information such as the config files, or as the place to output log
files. The problem that has been found at Yahoo/Apple/some other
companies is that certain layouts use absolute path ( Honestly some of
them use messed up paths...) So for example the layout opt has this
layout structure:...mandir: ${prefix}/share/man
sysconfdir: /etc${prefix}
...
I would argue that sysconfdir should be ${prefix}/etc, however that is
not the point here.
The reason config.layout exists is because no-one can agree on the same
locations for everything :)
The issue is that cases like this prevent us from testing ATS
correctly as we point to locations outside the sandbox. GIven the
TS_ROOT as it is design is broken in certain cases, the fix for this
is to change autest to set exact paths for the path like sysconfdir.
This prevents the problems from happening and should allow custom
layouts to works. The code removed was some code ( I have no idea why
it was there, and from your response seems to be dead code as is) that
prevented us from modifying the sysconfdir, as it assumed we should be
using it only with some feature/command line option.
The code that was removed was checking that the -D option was not given
unless the "-C config_verify” was also given. If you look at the way
the -D option is handled, it sets the “conf_dir” value which is
*only* used in cmd_verify() in
“Layout::get()->update_sysconfdir(conf_dir)”. It doesn’t do
anything at all in the absence of the "-C config_verify” option.
This is why I asked about the removal, because removing the check does
not fix any of the problems you describe AFAICT. So I wonder why the
code was removed, and what problem that removal fixed.
To change the config directory, you can set the PROXY_CONFIG_CONFIG_DIR
environment variable, which is supported and ought to work in all cases.
At the moment I have an intern making these changes in AuTest
extensions to get it work with the current trafficserver. The only
code changes I will try to push are item that are utterly broken from
a testability point of view.
GIven we get this working the second phase I would like to do is make
code changes to add some sort of TS_RUN_DIR value or something to
allow us to define layout as a run time value.
Experiment with traffic_layout a bit. You can alter most things in the
layout today.
For example:
$ PROXY_CONFIG_CONFIG_DIR=/one/etc PROXY_CONFIG_LOCAL_STATE_DIR=/two/run
PROXY_CONFIG_LOG_LOGFILE_DIR=/three/logs /opt/ats/bin/traffic_layout
PREFIX: /opt/ats
BINDIR: /opt/ats/bin
SYSCONFDIR: /one/etc
LIBDIR: /opt/ats/lib
LOGDIR: /three/logs
RUNTIMEDIR: /two/run
PLUGINDIR: /opt/ats/libexec/trafficserver
INCLUDEDIR: /opt/ats/include
SNAPSHOTDIR: /one/etc/snapshots
records.config: /one/etc/records.config
remap.config: /one/etc/remap.config
plugin.config: /one/etc/plugin.config
ssl_multicert.config: /one/etc/ssl_multicert.config
storage.config: /one/etc/storage.config
hosting.config: /one/etc/hosting.config
volume.config: /one/etc/volume.config
ip_allow.config: /one/etc/ip_allow.config
As you correctly note, traffic_ctl probably should learn how to deal
with a non-default PROXY_CONFIG_LOCAL_STATE_DIR
General idea here is that we could say something like:
:> traffic_server init ./
which would make a copy of the config file and state being used. Ideas
of having options like --full --data/partial to control how much to
copy or options like --layout to copy data in a different layout
structure are --default or --clean to get a clean set of
configurations files, etc... are idea that might be useful to have as
wellWhen we run trafficserver from the location it would use the
layout data. there might be an option we can add to tell
traffic_server to use the data in that location ( instead of the
default location)
This would greatly help testing, and would be nice for ops as one can
copy the state, make changes and see if an issue or feature works
without messing up the default setup by mistake. I am sure other use
case could be define to benefit form this as well. However I am not at
the point to propose a formal design for this.. but if you have ideas
about it.. would love to hear it
anyways I hope this clarifies the change and the motivation behind it.
Also as far as I know we don't use the -C option as well.. but maybe
Evil Dave knows if it is used at all. It does sound like a useful
item.
Jason
On Wednesday, June 7, 2017
10:18:29 PM CDT, James Peach
<jpe...@apache.org> wrote:
On Jun 7, 2017, at 6:15 PM, dragon512 <notificati...@github.com>
wrote:
@jpeach I don't know about the -D options. I did not test to see if
-D works or not ( I would assumed you guy had tested it already).
This patch enables the -D option in the generic option parsing, so I
assumed that you needed it :)
The code removed here is preventing a custom directory for
sysconfdir() to be used. (ie the test would fail as trafficserver
would exit with message assuming we are using some -D option.
This was added by Sudheer for Yahoo for the "-C config_verify”
option. AFAIK no-one except Yahoo uses this, and there are no tests.
Removing this allowed some testing work to run correctly.
OK so it sounds like it makes something work for you? In general I’m
not that confident that setting the config directory is fully working.
It worked enough for what Yahoo needed at the time, but it is hard to
say more than that.
At the moment we are still making tweaks to get AuTest to work with
layouts that have absolute paths ( ie they ignore TS_ROOT) so they
can work correct with the testing system. A number of companies have
complied about this issue. This is one step in few PR yet to come to
correct this issue. At the moment an intern is looking in to
traffic_ctrl and what we need to fix to get this to use the correct
paths.
The only case that I’m aware of is that there is no convenient way
to tell traffic_ctl which socket to use if you have a custom runtie
layout. That doesn’t seem hard to fix, though deciding on the right
way to specify the path deserves some though.
I can see about getting some "test" to validate the -D option works,
and if we all agree this is a supported option, see about documenting
it
AFAICT this option does nothing, so I don’t understand what started
working when you removed it.
