Open Stack

> -----Original Message-----
> From: Ben Nemec [mailto:openstack at 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 at 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".