Can't add image with location=XXX when not in TTY

Bug #907906 reported by Brian Lamar
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Glance
Fix Released
Low
Brian Lamar
Diablo
Fix Released
Undecided
Unassigned

Bug Description

Currently supported with bin/glance:
1) glance add name=XXX location=YYY (from within a TTY)
2) glance add name=XXX < file_name (from within a TTY)
3) glance add name=XXX < file_name (when NOT in a TTY)

It does not support:
4) glance add name=XXX location=YYY (when NOT in a TTY)

This makes it very difficult to execute bin/glance add within a script (like one called from Chef). I suggest removing a single 'if' statement which prohibits this valid use case.

Brian Lamar (blamar)
Changed in glance:
assignee: nobody → Brian Lamar (blamar)
status: New → In Progress
importance: Undecided → Low
Brian Waldon (bcwaldon)
Changed in glance:
milestone: none → essex-3
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (master)

Reviewed: https://review.openstack.org/2566
Committed: http://github.com/openstack/glance/commit/1571cd8f83cb80f2c89bd226883d7f802119bfb2
Submitter: Jenkins
Branch: master

commit 1571cd8f83cb80f2c89bd226883d7f802119bfb2
Author: Brian Lamar <email address hidden>
Date: Thu Dec 22 14:22:48 2011 -0500

    Removed bin/glance's TTY detection.

    Fixes bug 907906

    The logic is sound: if location=XXX is specified, use that.
    If it's not specified, use sys.stdin to read image data. No need to
    error when location=XXX is specified and stdin just happens to be a TTY.

    Change-Id: I057312becad59b3dd7a71f94d25ebd032e1a7b52

Changed in glance:
status: In Progress → Fix Committed
Revision history for this message
Jay Pipes (jaypipes) wrote :

Adding a Location property AND image data (in form of redirect) is NOT a valid use case. It's one or the other... I'm a little stumped as to why this was pushed through, especially since the chef changes weren't correct anyway (used piping instead of redirection to place image data into glance CLI tool...

Revision history for this message
Brian Lamar (blamar) wrote :

Jay,

I'm not sure we're talking about the same thing here. I understand that specifying a Location property and image data is not a valid use case. However, the way glance was attempting to detect that situation was the incorrect bit.

If you look at the bug report, glance needs to be able to support:

`glance add name=XXX location=YYY`

*when NOT in a TTY*

When we were running "glance add name=XXX location=YYY" without any redirect or piping, glance was failing saying that we shouldn't be giving data AND a location parameter. However, as you can see from the command...no data was being piped in and nothing was being redirected into bin/glance. I was just trying to add an image with a location.

The problem was that Chef was running a bash script in a process WITHOUT a TTY, which is pretty common I'd imagine because creating a pseudo TTY would be a pain/unnecessary. Simply calling `os.isatty` wasn't the correct logic because that would mean you'd always need to be redirecting data when not in a TTY (right?).

The Chef script with adding images w/o data and with a "location" parameter haven't been purposed for merge yet, which might be adding to the confusion here, but I can show you what was failing if I'm not explaining it correctly here. I'm not an expert on TTYs, but I'd like to think this change was correct because "glance add name=XXX location=YYY" should be able to be run when not in a TTY-like environment.

Revision history for this message
Jay Pipes (jaypipes) wrote : Re: [Bug 907906] Re: Can't add image with location=XXX when not in TTY

Ah, I gotcha... OK, yes that makes much more sense. Sorry for the
confusion, man! :(

-jay

On Thu, Dec 29, 2011 at 11:48 AM, Brian Lamar <email address hidden> wrote:
> Jay,
>
> I'm not sure we're talking about the same thing here. I understand that
> specifying a Location property and image data is not a valid use case.
> However, the way glance was attempting to detect that situation was the
> incorrect bit.
>
> If you look at the bug report, glance needs to be able to support:
>
> `glance add name=XXX location=YYY`
>
> *when NOT in a TTY*
>
> When we were running "glance add name=XXX location=YYY" without any
> redirect or piping, glance was failing saying that we shouldn't be
> giving data AND a location parameter. However, as you can see from the
> command...no data was being piped in and nothing was being redirected
> into bin/glance. I was just trying to add an image with a location.
>
> The problem was that Chef was running a bash script in a process WITHOUT
> a TTY, which is pretty common I'd imagine because creating a pseudo TTY
> would be a pain/unnecessary. Simply calling `os.isatty` wasn't the
> correct logic because that would mean you'd always need to be
> redirecting data when not in a TTY (right?).
>
> The Chef script with adding images w/o data and with a "location"
> parameter haven't been purposed for merge yet, which might be adding to
> the confusion here, but I can show you what was failing if I'm not
> explaining it correctly here. I'm not an expert on TTYs, but I'd like to
> think this change was correct because "glance add name=XXX location=YYY"
> should be able to be run when not in a TTY-like environment.
>
> --
> You received this bug notification because you are subscribed to Glance.
> https://bugs.launchpad.net/bugs/907906
>
> Title:
>  Can't add image with location=XXX when not in TTY
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/glance/+bug/907906/+subscriptions

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/diablo)

Fix proposed to branch: stable/diablo
Review: https://review.openstack.org/2866

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/diablo)

Reviewed: https://review.openstack.org/2866
Committed: http://github.com/openstack/glance/commit/2b97b7e56172aead287e2f5248d55650e2e458cf
Submitter: Jenkins
Branch: stable/diablo

commit 2b97b7e56172aead287e2f5248d55650e2e458cf
Author: Brian Lamar <email address hidden>
Date: Thu Dec 22 14:22:48 2011 -0500

    Removed bin/glance's TTY detection.

    Fixes bug 907906

    The logic is sound: if location=XXX is specified, use that.
    If it's not specified, use sys.stdin to read image data. No need to
    error when location=XXX is specified and stdin just happens to be a TTY.

    (cherry picked from commit 1571cd8f83cb80f2c89bd226883d7f802119bfb2)

    Change-Id: I057312becad59b3dd7a71f94d25ebd032e1a7b52

tags: added: in-stable-diablo
Thierry Carrez (ttx)
Changed in glance:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in glance:
milestone: essex-3 → 2012.1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.