Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
On 12/04/2014 05:41 AM, Clint Byrum wrote: What if the patch is reworked to leave the current trace-all-the-time mode in place, and we iterate on each script to make tracing conditional as we add proper logging? I have run [1] over patchset 15 to keep whatever was originally using -x tracing itself by default. I did not do this originally because it seems to me this list of files could be approximated with rand(), but it should maintain the status quo. -i [1] https://gist.github.com/ianw/71bbda9e6acc74ccd0fd ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
Hi > On 3 Dec 2014, at 18:41, Clint Byrum wrote: > > What if the patch is reworked to leave the current trace-all-the-time > mode in place, and we iterate on each script to make tracing conditional > as we add proper logging? +1 Cheers, -- Chris Jones ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
Excerpts from Chris Jones's message of 2014-12-03 02:47:30 -0800: > Hi > > I am very sympathetic to this view. We have a patch in hand that improves the > situation. We also have disagreement about the ideal situation. > > I +2'd Ian's patch because it makes things work better than they do now. If > we can arrive at an ideal solution later, great, but the more I think about > logging from a multitude of bash scripts, and tricks like XTRACE_FD, the more > I think it's crazy and we should just incrementally improve the non-trace > logging as a separate exercise, leaving working tracing for true debugging > situations. > Forgive me, I am not pushing for an ideal situation, but I don't want a regression. Running without -x right now has authors xtracing as a rule. Meaning that the moment this merges, the amount of output goes to almost nil compared to what it is now. Basically this is just more of the same OpenStack wrong-headed idea, "you have to run in DEBUG logging mode to be able to understand any issue". I'm totally willing to compromise on the ideal for something that is good enough, but I'm saying this is not good enough _if_ it turns off tracing for all scripts. What if the patch is reworked to leave the current trace-all-the-time mode in place, and we iterate on each script to make tracing conditional as we add proper logging? ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
Hi I am very sympathetic to this view. We have a patch in hand that improves the situation. We also have disagreement about the ideal situation. I +2'd Ian's patch because it makes things work better than they do now. If we can arrive at an ideal solution later, great, but the more I think about logging from a multitude of bash scripts, and tricks like XTRACE_FD, the more I think it's crazy and we should just incrementally improve the non-trace logging as a separate exercise, leaving working tracing for true debugging situations. Cheers, -- Chris Jones > On 3 Dec 2014, at 05:00, Ian Wienand wrote: > >> On 12/03/2014 09:30 AM, Clint Byrum wrote: >> I for one find the idea of printing every cp, cat, echo and ls command out >> rather frustratingly verbose when scanning logs from a normal run. > > I for one find this ongoing discussion over a flag whose own help says > "-x -- turn on tracing" not doing the blindly obvious thing of turning > on tracing and the seeming inability to reach to a conclusion on a > posted review over 3 months a troubling narrative for potential > consumers of diskimage-builder. > > -i > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
On 12/03/2014 09:30 AM, Clint Byrum wrote: I for one find the idea of printing every cp, cat, echo and ls command out rather frustratingly verbose when scanning logs from a normal run. I for one find this ongoing discussion over a flag whose own help says "-x -- turn on tracing" not doing the blindly obvious thing of turning on tracing and the seeming inability to reach to a conclusion on a posted review over 3 months a troubling narrative for potential consumers of diskimage-builder. -i ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
Excerpts from Ian Wienand's message of 2014-12-02 11:22:31 -0800: > On 12/02/2014 03:46 PM, Clint Byrum wrote: > > 1) Conform all o-r-c scripts to the logging standards we have in > > OpenStack, or write new standards for diskimage-builder and conform > > them to those standards. Abolish non-conditional xtrace in any script > > conforming to the standards. > > Honestly in the list of things that need doing in openstack, this must > be near the bottom. > > The whole reason I wrote this is because "disk-image-create -x ..." > doesn't do what any reasonable person expects it to; i.e. trace all > the scripts it starts. > > Having a way to trace execution of all d-i-b scripts is all that's > needed and gives sufficient detail to debug issues. Several developers have expressed their concern for an all-or-nothing approach to this. The concern is that when you turn off the "trace all" you lose the author-intended "info" level messages. I for one find the idea of printing every cp, cat, echo and ls command out rather frustratingly verbose when scanning logs from a normal run. Do I want it sometimes? YES, but it will actually hinder normal image building iteration if we only have a toggle of "all trace" or "no trace". ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
On 12/02/2014 03:46 PM, Clint Byrum wrote: 1) Conform all o-r-c scripts to the logging standards we have in OpenStack, or write new standards for diskimage-builder and conform them to those standards. Abolish non-conditional xtrace in any script conforming to the standards. Honestly in the list of things that need doing in openstack, this must be near the bottom. The whole reason I wrote this is because "disk-image-create -x ..." doesn't do what any reasonable person expects it to; i.e. trace all the scripts it starts. Having a way to trace execution of all d-i-b scripts is all that's needed and gives sufficient detail to debug issues. -i ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
Excerpts from James Slagle's message of 2014-11-28 11:27:20 -0800:
> On Thu, Nov 27, 2014 at 1:29 PM, Sullivan, Jon Paul
> wrote:
> >> -Original Message-
> >> From: Ben Nemec [mailto:openst...@nemebean.com]
> >> Sent: 26 November 2014 17:03
> >> To: OpenStack Development Mailing List (not for usage questions)
> >> Subject: Re: [openstack-dev] [diskimage-builder] Tracing levels for
> >> scripts (119023)
> >>
> >> On 11/25/2014 10:58 PM, Ian Wienand wrote:
> >> > Hi,
> >> >
> >> > My change [1] to enable a consistent tracing mechanism for the many
> >> > scripts diskimage-builder runs during its build seems to have hit a
> >> > stalemate.
> >> >
> >> > I hope we can agree that the current situation is not good. When
> >> > trying to develop with diskimage-builder, I find myself constantly
> >> > going and fiddling with "set -x" in various scripts, requiring me
> >> > re-running things needlessly as I try and trace what's happening.
> >> > Conversley some scripts set -x all the time and give output when you
> >> > don't want it.
> >> >
> >> > Now nodepool is using d-i-b more, it would be even nicer to have
> >> > consistency in the tracing so relevant info is captured in the image
> >> > build logs.
> >> >
> >> > The crux of the issue seems to be some disagreement between reviewers
> >> > over having a single "trace everything" flag or a more fine-grained
> >> > approach, as currently implemented after it was asked for in reviews.
> >> >
> >> > I must be honest, I feel a bit silly calling out essentially a
> >> > four-line patch here.
> >>
> >> My objections are documented in the review, but basically boil down to
> >> the fact that it's not a four line patch, it's a 500+ line patch that
> >> does essentially the same thing as:
> >>
> >> set +e
> >> set -x
> >> export SHELLOPTS
> >
> > I don't think this is true, as there are many more things in SHELLOPTS than
> > just xtrace. I think it is wrong to call the two approaches equivalent.
> >
> >>
> >> in disk-image-create. You do lose set -e in disk-image-create itself on
> >> debug runs because that's not something we can safely propagate,
> >> although we could work around that by unsetting it before calling hooks.
> >> FWIW I've used this method locally and it worked fine.
> >
> > So this does say that your alternative implementation has a difference from
> > the proposed one. And that the difference has a negative impact.
> >
> >>
> >> The only drawback is it doesn't allow the granularity of an if block in
> >> every script, but I don't personally see that as a particularly useful
> >> feature anyway. I would like to hear from someone who requested that
> >> functionality as to what their use case is and how they would define the
> >> different debug levels before we merge an intrusive patch that would
> >> need to be added to every single new script in dib or tripleo going
> >> forward.
> >
> > So currently we have boilerplate to be added to all new elements, and that
> > boilerplate is:
> >
> > set -eux
> > set -o pipefail
> >
> > This patch would change that boilerplate to:
> >
> > if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
> > set -x
> > fi
> > set -eu
> > set -o pipefail
> >
> > So it's adding 3 lines. It doesn't seem onerous, especially as most people
> > creating a new element will either copy an existing one or copy/paste the
> > header anyway.
> >
> > I think that giving control over what is effectively debug or non-debug
> > output is a desirable feature.
>
> I don't think it's debug vs non-debug. I think script writers that
> have explicitly used set -x previously have then operated under the
> assumption that they don't need to add any useful logging since it's
> running -x. In that case, this patch is actually harmful.
>
I believe James has hit the nail squarely on the head with the paragraph
above.
I propose a way forward for this:
1) Conform all o-r-c scripts to the logging standards we have in
OpenStack, or write new standards for diskimage-builder and conform
them to those standards. Abolish non-conditional xtrace in any script
conforming to the standards.
2) Once that is done, implement optional -x. I rather prefer the explicit
conditional set -x implementation over SHELLOPTS. As somebody else
pointed out, it feels like asking for unintended side-effects. But the
"how" is far less important than the "what" in this case, which step 1
will better define.
Anyone else have a better plan?
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
On 02/12/14 12:27, Ian Wienand wrote: > - pretty sure SHELLOPTS doesn't survive sudo, which might add another > layer of complication for users sudo is well-known to strip out all but a well-defined list of environment variables when you use it. sudo -E turns that off, but the configuration can still prohibit certain variables from propagating. Cheers, -- Steve "Why does everyone say 'Relax' when they're about to do something terrible?" - Ensign Harry Kim, USS Voyager ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
On 12/02/2014 04:25 AM, Ben Nemec wrote: 1) A specific reason SHELLOPTS can't be used. IMO leave this alone as it changes global behaviour at a low-level and that is a vector for unintended side-effects. Some thoughts: - We don't want tracing output of various well-known scripts that might run from /bin. - SHELLOPTS is read-only, so you have to "set -x; export SHELLOPTS" which means to turn it on for children you have to start tracing yourself. It's unintuitive and a bit weird. - Following from that, "DIB_DEBUG_TRACE=n disk-image-create" is the same as "disk-image-create -x" which is consistent. This can be useful for CI wrappers - pretty sure SHELLOPTS doesn't survive sudo, which might add another layer of complication for users - A known env variable can be usefully overloaded to signal to scripts not in bash rather than parsing SHELLOPTS I'm all for improving in this area, but before we make an intrusive change with an ongoing cost that won't work with anything not explicitly enabled for it, I want to make sure it's the right thing to do. As yet I'm not convinced. For "ongoing cost" -- I've rebased this about 15 times and there just isn't that much change in practice. In reality everyone copy-pastes another script to get started, so at least they'll copy-paste something consistent. That and dib-lint barfs if they don't. This makes "disk-image-create -x" do something actually useful by standardising the inconsistent existing defacto headers in all files. How is this *worse* than the status quo? -i ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
Okay, boiling my thoughts down further:
James's (valid, IMHO) concerns aside, I want to see one of two things
before I'm anything but -1 on this change:
1) A specific reason SHELLOPTS can't be used. Nobody has given me one
besides hand-wavy "it might not work" stuff. FTR, as I noted in my
previous message, the set -e thing can be easily addressed if we think
it necessary so I don't consider that a valid answer here.
Also, http://stackoverflow.com/questions/4325444/bash-recursive-xtrace
2) A specific use case that can only be addressed via this
implementation. I don't personally have one, but if someone does then
I'd like to hear it.
I'm all for improving in this area, but before we make an intrusive
change with an ongoing cost that won't work with anything not explicitly
enabled for it, I want to make sure it's the right thing to do. As yet
I'm not convinced.
-Ben
On 11/27/2014 12:29 PM, Sullivan, Jon Paul wrote:
>> -Original Message-
>> From: Ben Nemec [mailto:openst...@nemebean.com]
>> Sent: 26 November 2014 17:03
>> To: OpenStack Development Mailing List (not for usage questions)
>> Subject: Re: [openstack-dev] [diskimage-builder] Tracing levels for
>> scripts (119023)
>>
>> On 11/25/2014 10:58 PM, Ian Wienand wrote:
>>> Hi,
>>>
>>> My change [1] to enable a consistent tracing mechanism for the many
>>> scripts diskimage-builder runs during its build seems to have hit a
>>> stalemate.
>>>
>>> I hope we can agree that the current situation is not good. When
>>> trying to develop with diskimage-builder, I find myself constantly
>>> going and fiddling with "set -x" in various scripts, requiring me
>>> re-running things needlessly as I try and trace what's happening.
>>> Conversley some scripts set -x all the time and give output when you
>>> don't want it.
>>>
>>> Now nodepool is using d-i-b more, it would be even nicer to have
>>> consistency in the tracing so relevant info is captured in the image
>>> build logs.
>>>
>>> The crux of the issue seems to be some disagreement between reviewers
>>> over having a single "trace everything" flag or a more fine-grained
>>> approach, as currently implemented after it was asked for in reviews.
>>>
>>> I must be honest, I feel a bit silly calling out essentially a
>>> four-line patch here.
>>
>> My objections are documented in the review, but basically boil down to
>> the fact that it's not a four line patch, it's a 500+ line patch that
>> does essentially the same thing as:
>>
>> set +e
>> set -x
>> export SHELLOPTS
>
> I don't think this is true, as there are many more things in SHELLOPTS than
> just xtrace. I think it is wrong to call the two approaches equivalent.
>
>>
>> in disk-image-create. You do lose set -e in disk-image-create itself on
>> debug runs because that's not something we can safely propagate,
>> although we could work around that by unsetting it before calling hooks.
>> FWIW I've used this method locally and it worked fine.
>
> So this does say that your alternative implementation has a difference from
> the proposed one. And that the difference has a negative impact.
>
>>
>> The only drawback is it doesn't allow the granularity of an if block in
>> every script, but I don't personally see that as a particularly useful
>> feature anyway. I would like to hear from someone who requested that
>> functionality as to what their use case is and how they would define the
>> different debug levels before we merge an intrusive patch that would
>> need to be added to every single new script in dib or tripleo going
>> forward.
>
> So currently we have boilerplate to be added to all new elements, and that
> boilerplate is:
>
> set -eux
> set -o pipefail
>
> This patch would change that boilerplate to:
>
> if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
> set -x
> fi
> set -eu
> set -o pipefail
>
> So it's adding 3 lines. It doesn't seem onerous, especially as most people
> creating a new element will either copy an existing one or copy/paste the
> header anyway.
>
> I think that giving control over what is effectively debug or non-debug
> output is a desirable feature.
>
> We have a patch that implements that desirable feature.
>
> I don't see a compelling technical reason to reject that patch.
>
>>
>> -Ben
>>
>>
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
On Thu, Nov 27, 2014 at 1:29 PM, Sullivan, Jon Paul
wrote:
>> -Original Message-
>> From: Ben Nemec [mailto:openst...@nemebean.com]
>> Sent: 26 November 2014 17:03
>> To: OpenStack Development Mailing List (not for usage questions)
>> Subject: Re: [openstack-dev] [diskimage-builder] Tracing levels for
>> scripts (119023)
>>
>> On 11/25/2014 10:58 PM, Ian Wienand wrote:
>> > Hi,
>> >
>> > My change [1] to enable a consistent tracing mechanism for the many
>> > scripts diskimage-builder runs during its build seems to have hit a
>> > stalemate.
>> >
>> > I hope we can agree that the current situation is not good. When
>> > trying to develop with diskimage-builder, I find myself constantly
>> > going and fiddling with "set -x" in various scripts, requiring me
>> > re-running things needlessly as I try and trace what's happening.
>> > Conversley some scripts set -x all the time and give output when you
>> > don't want it.
>> >
>> > Now nodepool is using d-i-b more, it would be even nicer to have
>> > consistency in the tracing so relevant info is captured in the image
>> > build logs.
>> >
>> > The crux of the issue seems to be some disagreement between reviewers
>> > over having a single "trace everything" flag or a more fine-grained
>> > approach, as currently implemented after it was asked for in reviews.
>> >
>> > I must be honest, I feel a bit silly calling out essentially a
>> > four-line patch here.
>>
>> My objections are documented in the review, but basically boil down to
>> the fact that it's not a four line patch, it's a 500+ line patch that
>> does essentially the same thing as:
>>
>> set +e
>> set -x
>> export SHELLOPTS
>
> I don't think this is true, as there are many more things in SHELLOPTS than
> just xtrace. I think it is wrong to call the two approaches equivalent.
>
>>
>> in disk-image-create. You do lose set -e in disk-image-create itself on
>> debug runs because that's not something we can safely propagate,
>> although we could work around that by unsetting it before calling hooks.
>> FWIW I've used this method locally and it worked fine.
>
> So this does say that your alternative implementation has a difference from
> the proposed one. And that the difference has a negative impact.
>
>>
>> The only drawback is it doesn't allow the granularity of an if block in
>> every script, but I don't personally see that as a particularly useful
>> feature anyway. I would like to hear from someone who requested that
>> functionality as to what their use case is and how they would define the
>> different debug levels before we merge an intrusive patch that would
>> need to be added to every single new script in dib or tripleo going
>> forward.
>
> So currently we have boilerplate to be added to all new elements, and that
> boilerplate is:
>
> set -eux
> set -o pipefail
>
> This patch would change that boilerplate to:
>
> if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
> set -x
> fi
> set -eu
> set -o pipefail
>
> So it's adding 3 lines. It doesn't seem onerous, especially as most people
> creating a new element will either copy an existing one or copy/paste the
> header anyway.
>
> I think that giving control over what is effectively debug or non-debug
> output is a desirable feature.
I don't think it's debug vs non-debug. I think script writers that
have explicitly used set -x previously have then operated under the
assumption that they don't need to add any useful logging since it's
running -x. In that case, this patch is actually harmful.
>
> We have a patch that implements that desirable feature.
>
> I don't see a compelling technical reason to reject that patch.
I'm not specifically -2 on this patch based on the implementation.
It's more of the fact that I don't think this patch addresses the
problem in a meaningful way. The problem seems to be that dib either
logs too much or not enough information.
Also, it's a change to the current behavior that could be unexpected.
diskimage-builder has rather poor logging as-is. We don't use echo's
enough to actually say what's going on. Most script writers have just
relied on set -x to log everything explicitly, so there's no need to
echo or log any useful info. This patch turns off all tracing unless
specifically requested via $DIB_DEBUG_TRACE. Also, not all
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
> -Original Message-
> From: Ben Nemec [mailto:openst...@nemebean.com]
> Sent: 26 November 2014 17:03
> To: OpenStack Development Mailing List (not for usage questions)
> Subject: Re: [openstack-dev] [diskimage-builder] Tracing levels for
> scripts (119023)
>
> On 11/25/2014 10:58 PM, Ian Wienand wrote:
> > Hi,
> >
> > My change [1] to enable a consistent tracing mechanism for the many
> > scripts diskimage-builder runs during its build seems to have hit a
> > stalemate.
> >
> > I hope we can agree that the current situation is not good. When
> > trying to develop with diskimage-builder, I find myself constantly
> > going and fiddling with "set -x" in various scripts, requiring me
> > re-running things needlessly as I try and trace what's happening.
> > Conversley some scripts set -x all the time and give output when you
> > don't want it.
> >
> > Now nodepool is using d-i-b more, it would be even nicer to have
> > consistency in the tracing so relevant info is captured in the image
> > build logs.
> >
> > The crux of the issue seems to be some disagreement between reviewers
> > over having a single "trace everything" flag or a more fine-grained
> > approach, as currently implemented after it was asked for in reviews.
> >
> > I must be honest, I feel a bit silly calling out essentially a
> > four-line patch here.
>
> My objections are documented in the review, but basically boil down to
> the fact that it's not a four line patch, it's a 500+ line patch that
> does essentially the same thing as:
>
> set +e
> set -x
> export SHELLOPTS
I don't think this is true, as there are many more things in SHELLOPTS than
just xtrace. I think it is wrong to call the two approaches equivalent.
>
> in disk-image-create. You do lose set -e in disk-image-create itself on
> debug runs because that's not something we can safely propagate,
> although we could work around that by unsetting it before calling hooks.
> FWIW I've used this method locally and it worked fine.
So this does say that your alternative implementation has a difference from the
proposed one. And that the difference has a negative impact.
>
> The only drawback is it doesn't allow the granularity of an if block in
> every script, but I don't personally see that as a particularly useful
> feature anyway. I would like to hear from someone who requested that
> functionality as to what their use case is and how they would define the
> different debug levels before we merge an intrusive patch that would
> need to be added to every single new script in dib or tripleo going
> forward.
So currently we have boilerplate to be added to all new elements, and that
boilerplate is:
set -eux
set -o pipefail
This patch would change that boilerplate to:
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
set -x
fi
set -eu
set -o pipefail
So it's adding 3 lines. It doesn't seem onerous, especially as most people
creating a new element will either copy an existing one or copy/paste the
header anyway.
I think that giving control over what is effectively debug or non-debug output
is a desirable feature.
We have a patch that implements that desirable feature.
I don't see a compelling technical reason to reject that patch.
>
> -Ben
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Thanks,
Jon-Paul Sullivan ☺ Cloud Services - @hpcloud
Postal Address: Hewlett-Packard Galway Limited, Ballybrit Business Park, Galway.
Registered Office: Hewlett-Packard Galway Limited, 63-74 Sir John Rogerson's
Quay, Dublin 2.
Registered Number: 361933
The contents of this message and any attachments to it are confidential and may
be legally privileged. If you have received this message in error you should
delete it from your system immediately and advise the sender.
To any recipient of this message within HP, unless otherwise stated, you should
consider this message and attachments as "HP CONFIDENTIAL".
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [diskimage-builder] Tracing levels for scripts (119023)
On 11/25/2014 10:58 PM, Ian Wienand wrote: > Hi, > > My change [1] to enable a consistent tracing mechanism for the many > scripts diskimage-builder runs during its build seems to have hit a > stalemate. > > I hope we can agree that the current situation is not good. When > trying to develop with diskimage-builder, I find myself constantly > going and fiddling with "set -x" in various scripts, requiring me > re-running things needlessly as I try and trace what's happening. > Conversley some scripts set -x all the time and give output when you > don't want it. > > Now nodepool is using d-i-b more, it would be even nicer to have > consistency in the tracing so relevant info is captured in the image > build logs. > > The crux of the issue seems to be some disagreement between reviewers > over having a single "trace everything" flag or a more fine-grained > approach, as currently implemented after it was asked for in reviews. > > I must be honest, I feel a bit silly calling out essentially a > four-line patch here. My objections are documented in the review, but basically boil down to the fact that it's not a four line patch, it's a 500+ line patch that does essentially the same thing as: set +e set -x export SHELLOPTS in disk-image-create. You do lose set -e in disk-image-create itself on debug runs because that's not something we can safely propagate, although we could work around that by unsetting it before calling hooks. FWIW I've used this method locally and it worked fine. The only drawback is it doesn't allow the granularity of an if block in every script, but I don't personally see that as a particularly useful feature anyway. I would like to hear from someone who requested that functionality as to what their use case is and how they would define the different debug levels before we merge an intrusive patch that would need to be added to every single new script in dib or tripleo going forward. -Ben ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
