Re: [openstack-dev] [ironic] support for various glance image reference formats - do we need them all?

2017-08-11 Thread Mark Goddard 
Got it, thanks for explaining.
Mark

On 11 August 2017 at 10:46, Pavlo Shchelokovskyy <
pshchelokovs...@mirantis.com> wrote:

> Hi Mark,
>
> I do not propose to remove handling of plain http image references
> altogether, just remove the code pieces in glance service utils that
> pretend to support such refs *for glance images*.
>
> This code is never reached exactly due to plain http links being
> recognized as such from the very begining, and thus they will use a
> different 'image service' (HttpImageService) that has no notion of glance
> api, its required auth etc.
>
> Cheers,
>
> On Fri, Aug 11, 2017 at 11:59 AM, Mark Goddard  wrote:
>
>> Hi Pavlo,
>>
>> #3 is used in Bifrost, where there is no Glance service but the default
>> driver is agent_ipmitool. The images are served by the local nginx service.
>> For example, taken from one ironic node:
>>
>> 'image_source':  u'http://10.41.253.100:8080/deployment_image.qcow2'
>>
>> Mark
>>
>> On 10 August 2017 at 08:20, Pavlo Shchelokovskyy <
>> pshchelokovs...@mirantis.com> wrote:
>>
>>> HI Dmitry,
>>>
>>> On Tue, Aug 8, 2017 at 7:13 PM, Dmitry Tantsur 
>>> wrote:
>>>
 Hi!

 Thanks for raising this.

 On 08/07/2017 02:47 PM, Pavlo Shchelokovskyy wrote:

> Hi all,
>
> currently our GlanceImageService seems to support several ways of
> defining a reference to glance image:
>
> 1) simple image UUID [0]
> 2) image UUID prefixed with 'glance://' protocol [1] (well, actually
> anything starting with 'glance://' and ending with '/')
> 3) full REST path to the image (as in 
> http:///v2/images/),
> also used to extract the glance API address [2]
>
> Support for #3 must surely be removed, as we are moving to API
> endpoint discovery from keystone catalog.
> Besides I am pretty sure this code path can not actually be reached
> currently, as the 'is_glance_image' will ignore such image refs and return
> False for those [3], and 'get_image_service' would also not return the
> GlanceImageService instance for such image refs [4].
> Hence the question - if it is unusable anyway now, should we execute
> the standard deprecation process when removing support for it or just
> remove right away?
>

 If unsure, always use the standard process ;) Unless somebody is ready
 to set up DevStack, and try it out, and prove that it's completely and
 hopelessly broken.
>>>
>>>
>>> Did just that [0] - hacked up our tempest configuration so that the
>>> 'http' url for whole disk image used for *HttpLink standalone tests leads
>>> to /v2/images/ [1].
>>> As I kind of expected, both *HttpLink standalone tests failed, right on
>>> validating of the deploy interface when trying to do a HEAD on that URL
>>> instead of 'glance image show', receiving 401 [2].
>>> On a side note, it seems our logging is insufficient in this parts, as I
>>> could not find any relevant logs in ironic-conductor even at debug level,
>>> all that's there are final RPC processing error logs from api.
>>>
>>> So I am quite confident that this code paths [3] in service_utils is a
>>> dead end indeed :-/
>>>
>>> [0] https://review.openstack.org/#/c/492184/
>>> [1] http://logs.openstack.org/84/492184/1/check/gate-ironic-
>>> dsvm-standalone-ubuntu-xenial/32ea5de/logs/tempest_conf.txt.gz
>>> [2] http://logs.openstack.org/84/492184/1/check/gate-ironic-
>>> dsvm-standalone-ubuntu-xenial/32ea5de/logs/testr_results.html.gz
>>> [3] https://github.com/openstack/ironic/blob/7e6ce7e78c4378f
>>> 2f86d085df61a26507a410738/ironic/common/glance_service/servi
>>> ce_utils.py#L159-L166
>>>
>>>


> As for the #1 and #2 I do not see any big difference between those,
> and proposed deprecating and eventually removing support for #2
> ('glance://' scheme) as part of [5]. Two people in review already raised a
> concern about removing such support.
>

 To be honest, I like glance:// more, as it makes it obvious where
 the image is coming from vs http://. I don't mind removing it too
 much, but I guess supporting it is not a lot of code, right?

>>>
>>> That's not too much burden indeed, as long as we actually do only that -
>>> as I said, right now it is more like "glance:///uuid"
>>>
>>>

> Thus I'd like to ask a broader audience - do we need support for
> glance image references in 'glance://*' form? Is it actually used
> somewhere? What are the benefits of having it besides simple UUID?
>
> [0] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
> ommon/glance_service/service_utils.py#n149
> [1] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
> ommon/glance_service/service_utils.py#n155
> [2] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
> ommon/glance_service/service_utils.py#n160
> [3]

Re: [openstack-dev] [ironic] support for various glance image reference formats - do we need them all?

2017-08-11 Thread Pavlo Shchelokovskyy 
Hi Mark,

I do not propose to remove handling of plain http image references
altogether, just remove the code pieces in glance service utils that
pretend to support such refs *for glance images*.

This code is never reached exactly due to plain http links being recognized
as such from the very begining, and thus they will use a different 'image
service' (HttpImageService) that has no notion of glance api, its required
auth etc.

Cheers,

On Fri, Aug 11, 2017 at 11:59 AM, Mark Goddard  wrote:

> Hi Pavlo,
>
> #3 is used in Bifrost, where there is no Glance service but the default
> driver is agent_ipmitool. The images are served by the local nginx service.
> For example, taken from one ironic node:
>
> 'image_source':  u'http://10.41.253.100:8080/deployment_image.qcow2'
>
> Mark
>
> On 10 August 2017 at 08:20, Pavlo Shchelokovskyy <
> pshchelokovs...@mirantis.com> wrote:
>
>> HI Dmitry,
>>
>> On Tue, Aug 8, 2017 at 7:13 PM, Dmitry Tantsur 
>> wrote:
>>
>>> Hi!
>>>
>>> Thanks for raising this.
>>>
>>> On 08/07/2017 02:47 PM, Pavlo Shchelokovskyy wrote:
>>>
 Hi all,

 currently our GlanceImageService seems to support several ways of
 defining a reference to glance image:

 1) simple image UUID [0]
 2) image UUID prefixed with 'glance://' protocol [1] (well, actually
 anything starting with 'glance://' and ending with '/')
 3) full REST path to the image (as in 
 http:///v2/images/),
 also used to extract the glance API address [2]

 Support for #3 must surely be removed, as we are moving to API endpoint
 discovery from keystone catalog.
 Besides I am pretty sure this code path can not actually be reached
 currently, as the 'is_glance_image' will ignore such image refs and return
 False for those [3], and 'get_image_service' would also not return the
 GlanceImageService instance for such image refs [4].
 Hence the question - if it is unusable anyway now, should we execute
 the standard deprecation process when removing support for it or just
 remove right away?

>>>
>>> If unsure, always use the standard process ;) Unless somebody is ready
>>> to set up DevStack, and try it out, and prove that it's completely and
>>> hopelessly broken.
>>
>>
>> Did just that [0] - hacked up our tempest configuration so that the
>> 'http' url for whole disk image used for *HttpLink standalone tests leads
>> to /v2/images/ [1].
>> As I kind of expected, both *HttpLink standalone tests failed, right on
>> validating of the deploy interface when trying to do a HEAD on that URL
>> instead of 'glance image show', receiving 401 [2].
>> On a side note, it seems our logging is insufficient in this parts, as I
>> could not find any relevant logs in ironic-conductor even at debug level,
>> all that's there are final RPC processing error logs from api.
>>
>> So I am quite confident that this code paths [3] in service_utils is a
>> dead end indeed :-/
>>
>> [0] https://review.openstack.org/#/c/492184/
>> [1] http://logs.openstack.org/84/492184/1/check/gate-ironic-
>> dsvm-standalone-ubuntu-xenial/32ea5de/logs/tempest_conf.txt.gz
>> [2] http://logs.openstack.org/84/492184/1/check/gate-ironic-
>> dsvm-standalone-ubuntu-xenial/32ea5de/logs/testr_results.html.gz
>> [3] https://github.com/openstack/ironic/blob/7e6ce7e78c4378f
>> 2f86d085df61a26507a410738/ironic/common/glance_service/
>> service_utils.py#L159-L166
>>
>>
>>>
>>>
 As for the #1 and #2 I do not see any big difference between those, and
 proposed deprecating and eventually removing support for #2 ('glance://'
 scheme) as part of [5]. Two people in review already raised a concern about
 removing such support.

>>>
>>> To be honest, I like glance:// more, as it makes it obvious where
>>> the image is coming from vs http://. I don't mind removing it too much,
>>> but I guess supporting it is not a lot of code, right?
>>>
>>
>> That's not too much burden indeed, as long as we actually do only that -
>> as I said, right now it is more like "glance:///uuid"
>>
>>
>>>
 Thus I'd like to ask a broader audience - do we need support for glance
 image references in 'glance://*' form? Is it actually used somewhere?
 What are the benefits of having it besides simple UUID?

 [0] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
 ommon/glance_service/service_utils.py#n149
 [1] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
 ommon/glance_service/service_utils.py#n155
 [2] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
 ommon/glance_service/service_utils.py#n160
 [3] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
 ommon/glance_service/service_utils.py#n208
 [4] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
 ommon/image_service.py#n267
 [5] https://review.openstack.org/#/c/467728

 Cheers,

 --
 Dr. Pavlo

Re: [openstack-dev] [ironic] support for various glance image reference formats - do we need them all?

2017-08-11 Thread Mark Goddard 
Hi Pavlo,

#3 is used in Bifrost, where there is no Glance service but the default
driver is agent_ipmitool. The images are served by the local nginx service.
For example, taken from one ironic node:

'image_source':  u'http://10.41.253.100:8080/deployment_image.qcow2'

Mark

On 10 August 2017 at 08:20, Pavlo Shchelokovskyy <
pshchelokovs...@mirantis.com> wrote:

> HI Dmitry,
>
> On Tue, Aug 8, 2017 at 7:13 PM, Dmitry Tantsur 
> wrote:
>
>> Hi!
>>
>> Thanks for raising this.
>>
>> On 08/07/2017 02:47 PM, Pavlo Shchelokovskyy wrote:
>>
>>> Hi all,
>>>
>>> currently our GlanceImageService seems to support several ways of
>>> defining a reference to glance image:
>>>
>>> 1) simple image UUID [0]
>>> 2) image UUID prefixed with 'glance://' protocol [1] (well, actually
>>> anything starting with 'glance://' and ending with '/')
>>> 3) full REST path to the image (as in 
>>> http:///v2/images/),
>>> also used to extract the glance API address [2]
>>>
>>> Support for #3 must surely be removed, as we are moving to API endpoint
>>> discovery from keystone catalog.
>>> Besides I am pretty sure this code path can not actually be reached
>>> currently, as the 'is_glance_image' will ignore such image refs and return
>>> False for those [3], and 'get_image_service' would also not return the
>>> GlanceImageService instance for such image refs [4].
>>> Hence the question - if it is unusable anyway now, should we execute the
>>> standard deprecation process when removing support for it or just remove
>>> right away?
>>>
>>
>> If unsure, always use the standard process ;) Unless somebody is ready to
>> set up DevStack, and try it out, and prove that it's completely and
>> hopelessly broken.
>
>
> Did just that [0] - hacked up our tempest configuration so that the 'http'
> url for whole disk image used for *HttpLink standalone tests leads to
> /v2/images/ [1].
> As I kind of expected, both *HttpLink standalone tests failed, right on
> validating of the deploy interface when trying to do a HEAD on that URL
> instead of 'glance image show', receiving 401 [2].
> On a side note, it seems our logging is insufficient in this parts, as I
> could not find any relevant logs in ironic-conductor even at debug level,
> all that's there are final RPC processing error logs from api.
>
> So I am quite confident that this code paths [3] in service_utils is a
> dead end indeed :-/
>
> [0] https://review.openstack.org/#/c/492184/
> [1] http://logs.openstack.org/84/492184/1/check/gate-ironic-
> dsvm-standalone-ubuntu-xenial/32ea5de/logs/tempest_conf.txt.gz
> [2] http://logs.openstack.org/84/492184/1/check/gate-ironic-
> dsvm-standalone-ubuntu-xenial/32ea5de/logs/testr_results.html.gz
> [3] https://github.com/openstack/ironic/blob/
> 7e6ce7e78c4378f2f86d085df61a26507a410738/ironic/common/
> glance_service/service_utils.py#L159-L166
>
>
>>
>>
>>> As for the #1 and #2 I do not see any big difference between those, and
>>> proposed deprecating and eventually removing support for #2 ('glance://'
>>> scheme) as part of [5]. Two people in review already raised a concern about
>>> removing such support.
>>>
>>
>> To be honest, I like glance:// more, as it makes it obvious where
>> the image is coming from vs http://. I don't mind removing it too much,
>> but I guess supporting it is not a lot of code, right?
>>
>
> That's not too much burden indeed, as long as we actually do only that -
> as I said, right now it is more like "glance:///uuid"
>
>
>>
>>> Thus I'd like to ask a broader audience - do we need support for glance
>>> image references in 'glance://*' form? Is it actually used somewhere?
>>> What are the benefits of having it besides simple UUID?
>>>
>>> [0] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>>> ommon/glance_service/service_utils.py#n149
>>> [1] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>>> ommon/glance_service/service_utils.py#n155
>>> [2] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>>> ommon/glance_service/service_utils.py#n160
>>> [3] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>>> ommon/glance_service/service_utils.py#n208
>>> [4] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>>> ommon/image_service.py#n267
>>> [5] https://review.openstack.org/#/c/467728
>>>
>>> Cheers,
>>>
>>> --
>>> Dr. Pavlo Shchelokovskyy
>>> Senior Software Engineer
>>> Mirantis Inc
>>> www.mirantis.com 
>>>
>>>
>>> 
>>> __
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe: openstack-dev-requ...@lists.op
>>> enstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>
>> 
>> __
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe:

Re: [openstack-dev] [ironic] support for various glance image reference formats - do we need them all?

2017-08-10 Thread Pavlo Shchelokovskyy 
HI Dmitry,

On Tue, Aug 8, 2017 at 7:13 PM, Dmitry Tantsur  wrote:

> Hi!
>
> Thanks for raising this.
>
> On 08/07/2017 02:47 PM, Pavlo Shchelokovskyy wrote:
>
>> Hi all,
>>
>> currently our GlanceImageService seems to support several ways of
>> defining a reference to glance image:
>>
>> 1) simple image UUID [0]
>> 2) image UUID prefixed with 'glance://' protocol [1] (well, actually
>> anything starting with 'glance://' and ending with '/')
>> 3) full REST path to the image (as in 
>> http:///v2/images/),
>> also used to extract the glance API address [2]
>>
>> Support for #3 must surely be removed, as we are moving to API endpoint
>> discovery from keystone catalog.
>> Besides I am pretty sure this code path can not actually be reached
>> currently, as the 'is_glance_image' will ignore such image refs and return
>> False for those [3], and 'get_image_service' would also not return the
>> GlanceImageService instance for such image refs [4].
>> Hence the question - if it is unusable anyway now, should we execute the
>> standard deprecation process when removing support for it or just remove
>> right away?
>>
>
> If unsure, always use the standard process ;) Unless somebody is ready to
> set up DevStack, and try it out, and prove that it's completely and
> hopelessly broken.


Did just that [0] - hacked up our tempest configuration so that the 'http'
url for whole disk image used for *HttpLink standalone tests leads to
/v2/images/ [1].
As I kind of expected, both *HttpLink standalone tests failed, right on
validating of the deploy interface when trying to do a HEAD on that URL
instead of 'glance image show', receiving 401 [2].
On a side note, it seems our logging is insufficient in this parts, as I
could not find any relevant logs in ironic-conductor even at debug level,
all that's there are final RPC processing error logs from api.

So I am quite confident that this code paths [3] in service_utils is a dead
end indeed :-/

[0] https://review.openstack.org/#/c/492184/
[1]
http://logs.openstack.org/84/492184/1/check/gate-ironic-dsvm-standalone-ubuntu-xenial/32ea5de/logs/tempest_conf.txt.gz
[2]
http://logs.openstack.org/84/492184/1/check/gate-ironic-dsvm-standalone-ubuntu-xenial/32ea5de/logs/testr_results.html.gz
[3]
https://github.com/openstack/ironic/blob/7e6ce7e78c4378f2f86d085df61a26507a410738/ironic/common/glance_service/service_utils.py#L159-L166


>
>
>> As for the #1 and #2 I do not see any big difference between those, and
>> proposed deprecating and eventually removing support for #2 ('glance://'
>> scheme) as part of [5]. Two people in review already raised a concern about
>> removing such support.
>>
>
> To be honest, I like glance:// more, as it makes it obvious where
> the image is coming from vs http://. I don't mind removing it too much,
> but I guess supporting it is not a lot of code, right?
>

That's not too much burden indeed, as long as we actually do only that - as
I said, right now it is more like "glance:///uuid"


>
>> Thus I'd like to ask a broader audience - do we need support for glance
>> image references in 'glance://*' form? Is it actually used somewhere?
>> What are the benefits of having it besides simple UUID?
>>
>> [0] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>> ommon/glance_service/service_utils.py#n149
>> [1] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>> ommon/glance_service/service_utils.py#n155
>> [2] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>> ommon/glance_service/service_utils.py#n160
>> [3] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>> ommon/glance_service/service_utils.py#n208
>> [4] http://git.openstack.org/cgit/openstack/ironic/tree/ironic/c
>> ommon/image_service.py#n267
>> [5] https://review.openstack.org/#/c/467728
>>
>> Cheers,
>>
>> --
>> Dr. Pavlo Shchelokovskyy
>> Senior Software Engineer
>> Mirantis Inc
>> www.mirantis.com 
>>
>>
>> 
>> __
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscrib
>> e
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>



-- 
Dr. Pavlo Shchelokovskyy
Senior Software Engineer
Mirantis Inc
www.mirantis.com
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [ironic] support for various glance image reference formats - do we need them all?

2017-08-08 Thread Dmitry Tantsur 

Hi!

Thanks for raising this.

On 08/07/2017 02:47 PM, Pavlo Shchelokovskyy wrote:

Hi all,

currently our GlanceImageService seems to support several ways of defining a 
reference to glance image:


1) simple image UUID [0]
2) image UUID prefixed with 'glance://' protocol [1] (well, actually anything 
starting with 'glance://' and ending with '/')
3) full REST path to the image (as in 
http:///v2/images/), also used to extract the glance API 
address [2]


Support for #3 must surely be removed, as we are moving to API endpoint 
discovery from keystone catalog.
Besides I am pretty sure this code path can not actually be reached currently, 
as the 'is_glance_image' will ignore such image refs and return False for those 
[3], and 'get_image_service' would also not return the GlanceImageService 
instance for such image refs [4].
Hence the question - if it is unusable anyway now, should we execute the 
standard deprecation process when removing support for it or just remove right away?


If unsure, always use the standard process ;) Unless somebody is ready to set up 
DevStack, and try it out, and prove that it's completely and hopelessly broken.




As for the #1 and #2 I do not see any big difference between those, and proposed 
deprecating and eventually removing support for #2 ('glance://' scheme) as part 
of [5]. Two people in review already raised a concern about removing such support.


To be honest, I like glance:// more, as it makes it obvious where the 
image is coming from vs http://. I don't mind removing it too much, but I guess 
supporting it is not a lot of code, right?




Thus I'd like to ask a broader audience - do we need support for glance image 
references in 'glance://*' form? Is it actually used somewhere? What are 
the benefits of having it besides simple UUID?


[0] 
http://git.openstack.org/cgit/openstack/ironic/tree/ironic/common/glance_service/service_utils.py#n149
[1] 
http://git.openstack.org/cgit/openstack/ironic/tree/ironic/common/glance_service/service_utils.py#n155
[2] 
http://git.openstack.org/cgit/openstack/ironic/tree/ironic/common/glance_service/service_utils.py#n160
[3] 
http://git.openstack.org/cgit/openstack/ironic/tree/ironic/common/glance_service/service_utils.py#n208
[4] 
http://git.openstack.org/cgit/openstack/ironic/tree/ironic/common/image_service.py#n267

[5] https://review.openstack.org/#/c/467728

Cheers,

--
Dr. Pavlo Shchelokovskyy
Senior Software Engineer
Mirantis Inc
www.mirantis.com 


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev