crowbar-openstack
crowbar-openstack copied to clipboard
Enable Swift support for temporary URLs by default.
One example of the use of temporary URLs is the Ironic agent_... driver workflow in which the agent on the deployed node downloads the image from Glance using a Swift temporary URL.
Don't we want the support for providing the secret key, as @skazi0 was mentioning?
@jsuchome we don't :) Since yesterday I learned a couple of interesting things about Temp URLs. One of them is that the Key needs to be set for glance account to work for ironic (so it doesn't make sense to add it to swift barclamp). Second one is that we can actually extract the existing key so I will not add a setting for this at all. I'll read the existing key and put it into ironic config and if it is not set I'll set a random one.
See: https://docs.openstack.org/cli-reference/swift.html#swift-stat Example output:
Account: AUTH_da98101601104009954b440adb0d5fb0
Containers: 1
Objects: 13
Bytes: 475471315
Containers in policy "policy-0": 1
Objects in policy "policy-0": 13
Bytes in policy "policy-0": 475471315
Meta Temp-Url-Key: tempurlkey123
X-Account-Project-Domain-Id: default
X-Timestamp: 1493291281.59816
X-Trans-Id: tx8c04fb5cc4914d2a8ec68-005902edba
Content-Type: text/plain; charset=utf-8
Accept-Ranges: bytes
Cool!
But than again - do we need to enabled TempURL by default? We could just let ironic barclamp to check it is enabled.
(I'm not saying we shouldn't, maybe it's good to have it enabled by default.)
I was thinking about this too. There are a couple of options:
- no changes in swift barclamp, ironic barclamp will fail validation if tempurl is not enabled
- no changes in swift barclamp, ironic silently enables tempurl even if it's disabled in proposal (we already do it for e.g. glance v1 api and force_config_drive)
- default tempurl enabled in swift barclamp + validation from #1 to be sure
I'm not sure if there is e.g. some performance impact if tempurl is enabled but not used (i.e. no ironic installed).
@skazi0 I would not go with option 2. 1 was my original idea, but 3 might be good as well. Maybe @dirkmueller could decide.
Anyway, that schema migration update is probably needed
Right, if we go with option #3 then schema migration would be needed to keep things consistent.
Please prefix the commit msg with swift:
. See git log
for examples
I already have updates to the Glance and Ironic barclamps that support creation and usage of the temporary url key. I wanted to start the patch chain with this bootstrap change to enable tempurl processing.
From: Jacek Tomasiak <[email protected]>
To: crowbar/crowbar-openstack [email protected] Cc: Peter Piela [email protected]; Author [email protected] Sent: Friday, April 28, 2017 3:23 AM Subject: Re: [crowbar/crowbar-openstack] Enable Swift support for temporary URLs by default. (#936)
@jsuchome we don't :) Since yesterday I learned a couple of interesting things about Temp URLs. One of them is that the Key needs to be set for glance account to work for ironic (so it doesn't make sense to add it to swift barclamp). Second one is that we can actually extract the existing key so I will not add a setting for this at all. I'll read the existing key and put it into ironic config and if it is not set I'll set a random one.See: https://docs.openstack.org/cli-reference/swift.html#swift-stat Example output: Account: AUTH_da98101601104009954b440adb0d5fb0 Containers: 1 Objects: 13 Bytes: 475471315 Containers in policy "policy-0": 1 Objects in policy "policy-0": 13 Bytes in policy "policy-0": 475471315 Meta Temp-Url-Key: tempurlkey123 X-Account-Project-Domain-Id: default X-Timestamp: 1493291281.59816 X-Trans-Id: tx8c04fb5cc4914d2a8ec68-005902edba Content-Type: text/plain; charset=utf-8 Accept-Ranges: bytes — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
It is not a requirement that tempurl handling is enabled by default, but it seems more straightforward. I also think that the tempurl decision should be made by Glance. My changes to the Glance barclamp do the following: (1) Search for Swift proxy servers(2) If a proxy server is found (a) set the Glance default store to Swift (b) create the Swift temporary-url secret for the Glance account (c) get from Swift (and store) the account associated with the service.glance user (a), (b), (c) could be made conditional on Ironic including support for agent drivers, but this introduces a tight coupling that I am not sure is warranted. I am looking for guidance. Peter
From: Jiří Suchomel <[email protected]>
To: crowbar/crowbar-openstack [email protected] Cc: Peter Piela [email protected]; Author [email protected] Sent: Friday, April 28, 2017 3:35 AM Subject: Re: [crowbar/crowbar-openstack] Enable Swift support for temporary URLs by default. (#936)
Cool!But than again - do we need to enabled TempURL by default? We could just let ironic barclamp to check it is enabled.(I'm not saying we shouldn't, maybe it's good to have it enabled by default.)— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
It is not a requirement that tempurl handling is enabled by default, but it seems more straightforward.
Be it default or not, other barclamps must check if tempurl is enabled anyway.
Yes, but there are questions about how that checking is exposed. If Glance is not using Swift to store images then Ironic does not need to know about temporary URLs. Also this is only an issue for Ironic if a node has been enrolled using an agent-driver.
Peter
From: Jiří Suchomel <[email protected]>
To: crowbar/crowbar-openstack [email protected] Cc: Peter Piela [email protected]; Author [email protected] Sent: Friday, April 28, 2017 9:05 AM Subject: Re: [crowbar/crowbar-openstack] Enable Swift support for temporary URLs by default. (#936)
It is not a requirement that tempurl handling is enabled by default, but it seems more straightforward. Be it default or not, other barclamps must check if tempurl is enabled anyway.— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
@pcpiela take a look at my version here: https://github.com/crowbar/crowbar-openstack/pull/937/commits (last commit) I added validation for glance and swift settings to ironic barclamp. The TempURL handling is done in ironic barclamp only if enabled_drivers attribute include some agent_*. Later we plan to add some UI for agent selection so this attribute should be pretty future proof.
Jacek, The code looks reasonable to me, similar to what I am using. My thinking says that temp-url-key creation for the glance user should happen only in the Glance barclamp. From an encapsulation point of view I think of the Glance barclamp as the owner of the glance user account and capabilities provided by the Glance service. Peter
From: Jacek Tomasiak <[email protected]>
To: crowbar/crowbar-openstack [email protected] Cc: Peter Piela [email protected]; Mention [email protected] Sent: Friday, April 28, 2017 9:23 AM Subject: Re: [crowbar/crowbar-openstack] Enable Swift support for temporary URLs by default. (#936)
@pcpiela take a look at my version here: https://github.com/crowbar/crowbar-openstack/pull/937/commits (last commit) I added validation for glance and swift settings to ironic barclamp. The TempURL handling is done in ironic barclamp only if enabled_drivers attribute include some agent_*. Later we plan to add some UI for agent selection so this attribute should be pretty future proof.— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.