From 9fda0dc815f042272c7c2b8ae18424cf1b6505e8 Mon Sep 17 00:00:00 2001
From: Hugh Saunders <hugh@wherenow.org>
Date: Mon, 3 Jun 2013 15:24:51 +0100
Subject: [PATCH] Fix problem where image data is not read from a pipe.

For image-updae and image-create commands, glanceclient attempts to
determine whether image data should be uploaded based on the presence
of data on stdin. Unforunately it is difficult to determine if data is
available, especially when standard in is from a pipe.

This is especially problematic for update operations, where data must
only be uploaded if the image is in queued state. For example data may
be uploaded when the user only wants to rename an image, but the rename
will be rejected because data cannot be uploaded to an unqueued image.

This patch removes the check that attempts to determine if data is
available to read as it didn't work for pipes. It also re-introduces a
check for image state in the update operation, so that glanceclient only
attempts to read data if the image being updated is in queued state.

The image state check is part of the original patchset that was removed
so the patchset could have a single focus [1]

This patch also removes a test for handling empty stdin, and adds a test
for reading stdin from a pipe.

[1] https://review.openstack.org/#/c/27536/3/glanceclient/v1/shell.py

Fixes: bug 1184566
Related to: bug 1173044

Change-Id: I8d37f6412a0bf9ca21cbd75cde6a4d5a174e5545
---
 glanceclient/v1/shell.py |  7 +++---
 tests/v1/test_shell.py   | 46 +++++++++++++++++++++++++---------------
 2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/glanceclient/v1/shell.py b/glanceclient/v1/shell.py
index 31d9f9fa..ecfc70bd 100644
--- a/glanceclient/v1/shell.py
+++ b/glanceclient/v1/shell.py
@@ -121,12 +121,12 @@ def _set_data_field(fields, args):
             # (3) no image data provided:
             #     glance ...
             try:
-                stat_result = os.fstat(0)
+                os.fstat(0)
             except OSError:
                 # (1) stdin is not valid (closed...)
                 fields['data'] = None
                 return
-            if not sys.stdin.isatty() and stat_result.st_size != 0:
+            if not sys.stdin.isatty():
                 # (2) image data is provided through standard input
                 if msvcrt:
                     msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY)
@@ -294,7 +294,8 @@ def do_image_update(gc, args):
     UPDATE_PARAMS = glanceclient.v1.images.UPDATE_PARAMS
     fields = dict(filter(lambda x: x[0] in UPDATE_PARAMS, fields.items()))
 
-    _set_data_field(fields, args)
+    if image.status == 'queued':
+        _set_data_field(fields, args)
 
     image = gc.images.update(image, purge_props=args.purge_props, **fields)
     _image_show(image, args.human_readable)
diff --git a/tests/v1/test_shell.py b/tests/v1/test_shell.py
index a478b8c0..88ad3e39 100644
--- a/tests/v1/test_shell.py
+++ b/tests/v1/test_shell.py
@@ -18,6 +18,7 @@
 import argparse
 import json
 import os
+import subprocess
 import tempfile
 import testtools
 
@@ -385,23 +386,8 @@ class ShellStdinHandlingTests(testtools.TestCase):
             or self.collected_args[1]['data'] is None
         )
 
-    def test_image_update_empty_stdin(self):
-        """Supply glanceclient with an open but empty stdin, and perform an
-        image update to an active image. Glanceclient should not attempt to
-        read stdin.
-        """
-
-        self._do_update()
-
-        self.assertTrue(
-            'data' not in self.collected_args[1]
-            or self.collected_args[1]['data'] is None
-        )
-
-    def test_image_update_data_is_read(self):
-        """Ensure that data is read from stdin when image is in queued
-        status and data is available.
-        """
+    def test_image_update_data_is_read_from_file(self):
+        """Ensure that data is read from a file."""
 
         try:
 
@@ -417,6 +403,8 @@ class ShellStdinHandlingTests(testtools.TestCase):
 
             self.assertTrue('data' in self.collected_args[1])
             self.assertIsInstance(self.collected_args[1]['data'], file)
+            self.assertEqual(self.collected_args[1]['data'].read(),
+                             'Some Data')
 
         finally:
             try:
@@ -424,3 +412,27 @@ class ShellStdinHandlingTests(testtools.TestCase):
                 os.remove(f.name)
             except:
                 pass
+
+    def test_image_update_data_is_read_from_pipe(self):
+        """Ensure that data is read from a pipe."""
+
+        try:
+
+            # NOTE(hughsaunders): Setup a pipe, duplicate it to stdin
+            # ensure it is read.
+            process = subprocess.Popen(['/bin/echo', 'Some Data'],
+                                       stdout=subprocess.PIPE)
+            os.dup2(process.stdout.fileno(), 0)
+
+            self._do_update('44d2c7e1-de4e-4612-8aa2-ba26610c444f')
+
+            self.assertTrue('data' in self.collected_args[1])
+            self.assertIsInstance(self.collected_args[1]['data'], file)
+            self.assertEqual(self.collected_args[1]['data'].read(),
+                             'Some Data\n')
+
+        finally:
+            try:
+                process.stdout.close()
+            except:
+                pass