Re: [openstack-dev] [Glance] Need to revert Don't enable all stores by default
On 11/03/14 16:25 -0700, Clint Byrum wrote: Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. I don't think this change has to be reverted. We could add an upgrade path for this. We could enable a driver if its config options were set and warn the user about this change. Also, we could make sure we import all drivers and the config options are registered but that we don't try to enable them. Also, I don't expect (yeah, I know this is not always the case) users to blindly upgrade if they really care about their cloud deployment. Since this change will be part of the change log and the release notes, I expect the user to be aware of it. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. This is exactly the problem. With the current approach, the user has not explicitly enabled the swift store. The user just put swift configs. With the current 'enable all and let it fail' approach, it is really confusing for users to see all the failures and it's not nice to enable things by default for the user. Thanks for raising this issue, I didn't think about this corner case. Flavio -- @flaper87 Flavio Percoco pgpmk_F9F9rJP.pgp Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Glance] Need to revert Don't enable all stores by default
On 03/12/2014 06:38 AM, Flavio Percoco wrote: On 11/03/14 16:25 -0700, Clint Byrum wrote: Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. I don't think this change has to be reverted. We could add an upgrade path for this. We could enable a driver if its config options were set and warn the user about this change. Also, we could make sure we import all drivers and the config options are registered but that we don't try to enable them. Also, I don't expect (yeah, I know this is not always the case) users to blindly upgrade if they really care about their cloud deployment. Since this change will be part of the change log and the release notes, I expect the user to be aware of it. OpenStack's 2 largest public clouds don't wait for releases, so that's not really a good answer. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. This is exactly the problem. With the current approach, the user has not explicitly enabled the swift store. The user just put swift configs. With the current 'enable all and let it fail' approach, it is really confusing for users to see all the failures and it's not nice to enable things by default for the user. Thanks for raising this issue, I didn't think about this corner case. Flavio In fairness, this wasn't a corner case. Grenade was blocking this change for the whole cycle until a change was made in stable/havana devstack that sneaked around it with https://review.openstack.org/#/c/75827/. :) In addition, the commit in question for glance - https://review.openstack.org/#/c/59150/ didn't have UpgradeImpact, which is the signaling mechanism for these kinds of issues. I do think this is a real issue. OpenStack really is expected to be CD upgradable, not just post release and post release notes. Commits in OpenStack need to take that into account. A compatibility behavior should be put in place here. I do agree the current behavior isn't nice with gorpy error messages all the time. However, a completely legitimate approach would be: If configuration for a storage back end existed, but the driver wasn't explicitly set, load and configure that driver and throw a big DEPRECATION WARNING in the logs that Glance will require explicit loading of drivers in an upcoming release. That would let you move forward, and provide some user signally. That is definitely more effort, but as a community we've decided to support the CD method for OpenStack, which means we need to take account of these kinds of cases. -Sean -- Sean Dague Samsung Research America s...@dague.net / sean.da...@samsung.com http://dague.net signature.asc Description: OpenPGP digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Glance] Need to revert Don't enable all stores by default
On 12/03/14 07:18 -0400, Sean Dague wrote: On 03/12/2014 06:38 AM, Flavio Percoco wrote: On 11/03/14 16:25 -0700, Clint Byrum wrote: Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. I don't think this change has to be reverted. We could add an upgrade path for this. We could enable a driver if its config options were set and warn the user about this change. Also, we could make sure we import all drivers and the config options are registered but that we don't try to enable them. Also, I don't expect (yeah, I know this is not always the case) users to blindly upgrade if they really care about their cloud deployment. Since this change will be part of the change log and the release notes, I expect the user to be aware of it. OpenStack's 2 largest public clouds don't wait for releases, so that's not really a good answer. Like I said, I know it's not always the case. :) I'll work on a better upgrade path for this. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. This is exactly the problem. With the current approach, the user has not explicitly enabled the swift store. The user just put swift configs. With the current 'enable all and let it fail' approach, it is really confusing for users to see all the failures and it's not nice to enable things by default for the user. Thanks for raising this issue, I didn't think about this corner case. Flavio In fairness, this wasn't a corner case. Grenade was blocking this change for the whole cycle until a change was made in stable/havana devstack that sneaked around it with https://review.openstack.org/#/c/75827/. :) Yeah but the failures weren't about glance failing to start because swift's options were not registered. The failures were about swift tests failing because it wasn't enabled (IIRC). Same reason, different issue. In addition, the commit in question for glance - https://review.openstack.org/#/c/59150/ didn't have UpgradeImpact, which is the signaling mechanism for these kinds of issues. Fair enough. As mentioned in my previous email, the upgrade process was discussed but we failed (or at least I failed) at considering this impact. I do think this is a real issue. OpenStack really is expected to be CD upgradable, not just post release and post release notes. Commits in OpenStack need to take that into account. Already working on a way to reduce the impact of this change. We can revert it and re-submit a patch that handles the upgrade path. I do agree the current behavior isn't nice with gorpy error messages all the time. However, a completely legitimate approach would be: If configuration for a storage back end existed, but the driver wasn't explicitly set, load and configure that driver and throw a big DEPRECATION WARNING in the logs that Glance will require explicit loading of drivers in an upcoming release. That would let you move forward, and provide some user signally. Yup, I mentioned this in my previous email. I think it's the fairest approach for now. [snip] Flavio -- @flaper87 Flavio Percoco pgpF9lSsoLapb.pgp Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Glance] Need to revert Don't enable all stores by default
On 11/03/14 16:25 -0700, Clint Byrum wrote: Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. Here's an attempt to fix this issues without reverting the patch. Feedback appreciated. https://review.openstack.org/#/c/79935/ Flavio -- @flaper87 Flavio Percoco pgp99JsUDXYp3.pgp Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Glance] Need to revert Don't enable all stores by default
On 03/12/2014 09:01 AM, Flavio Percoco wrote: On 11/03/14 16:25 -0700, Clint Byrum wrote: Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. Here's an attempt to fix this issues without reverting the patch. Feedback appreciated. https://review.openstack.org/#/c/79935/ ACK. Looks pretty good. You might want to consider using one of the oslo deprecation functions to make it consistent on the deprecation side. -Sean -- Sean Dague Samsung Research America s...@dague.net / sean.da...@samsung.com http://dague.net signature.asc Description: OpenPGP digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Glance] Need to revert Don't enable all stores by default
On 03/12/2014 01:10 PM, Jay Pipes wrote: On Wed, 2014-03-12 at 07:18 -0400, Sean Dague wrote: On 03/12/2014 06:38 AM, Flavio Percoco wrote: On 11/03/14 16:25 -0700, Clint Byrum wrote: Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. I don't think this change has to be reverted. We could add an upgrade path for this. We could enable a driver if its config options were set and warn the user about this change. Also, we could make sure we import all drivers and the config options are registered but that we don't try to enable them. Also, I don't expect (yeah, I know this is not always the case) users to blindly upgrade if they really care about their cloud deployment. Since this change will be part of the change log and the release notes, I expect the user to be aware of it. OpenStack's 2 largest public clouds don't wait for releases, so that's not really a good answer. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. This is exactly the problem. With the current approach, the user has not explicitly enabled the swift store. The user just put swift configs. With the current 'enable all and let it fail' approach, it is really confusing for users to see all the failures and it's not nice to enable things by default for the user. Thanks for raising this issue, I didn't think about this corner case. Flavio In fairness, this wasn't a corner case. Grenade was blocking this change for the whole cycle until a change was made in stable/havana devstack that sneaked around it with https://review.openstack.org/#/c/75827/. :) In addition, the commit in question for glance - https://review.openstack.org/#/c/59150/ didn't have UpgradeImpact, which is the signaling mechanism for these kinds of issues. I do think this is a real issue. OpenStack really is expected to be CD upgradable, not just post release and post release notes. Commits in OpenStack need to take that into account. A compatibility behavior should be put in place here. I do agree the current behavior isn't nice with gorpy error messages all the time. However, a completely legitimate approach would be: If configuration for a storage back end existed, but the driver wasn't explicitly set, load and configure that driver and throw a big DEPRECATION WARNING in the logs that Glance will require explicit loading of drivers in an upcoming release. That would let you move forward, and provide some user signally. That's pretty much what already happens. On startup, Glance will log a message about a particular store driver being disabled [1] because configuration settings were not set properly. [2] IIRC, on startup is the only place these messages occur (not, for instance, every time somebody uploads an image), so I'm not entirely sure what the big deal was. Long term, moving stores into entrypoints might be a cleaner solution, but you will still need to validate configuration for those endpoints on startup -- all endpoints give you is a cleaner method than set your new store in the known_stores configuration option. The issue is this was a failure going from old defaults to new defaults, which is why it was actually blocked by grenade for months. The difference is actually we should be working with the drivers that were configured, and deprecate the fact that you can get away without specifying them. Then you can roll forward gracefully, see a warning message on your working new config, go handle the situation, and later when the backwards compatible behavior is removed you are ok. -Sean -- Sean Dague Samsung Research America s...@dague.net / sean.da...@samsung.com http://dague.net signature.asc Description: OpenPGP digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Glance] Need to revert Don't enable all stores by default
On Wed, 2014-03-12 at 13:21 -0400, Sean Dague wrote: On 03/12/2014 01:10 PM, Jay Pipes wrote: On Wed, 2014-03-12 at 07:18 -0400, Sean Dague wrote: On 03/12/2014 06:38 AM, Flavio Percoco wrote: On 11/03/14 16:25 -0700, Clint Byrum wrote: Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. I don't think this change has to be reverted. We could add an upgrade path for this. We could enable a driver if its config options were set and warn the user about this change. Also, we could make sure we import all drivers and the config options are registered but that we don't try to enable them. Also, I don't expect (yeah, I know this is not always the case) users to blindly upgrade if they really care about their cloud deployment. Since this change will be part of the change log and the release notes, I expect the user to be aware of it. OpenStack's 2 largest public clouds don't wait for releases, so that's not really a good answer. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. This is exactly the problem. With the current approach, the user has not explicitly enabled the swift store. The user just put swift configs. With the current 'enable all and let it fail' approach, it is really confusing for users to see all the failures and it's not nice to enable things by default for the user. Thanks for raising this issue, I didn't think about this corner case. Flavio In fairness, this wasn't a corner case. Grenade was blocking this change for the whole cycle until a change was made in stable/havana devstack that sneaked around it with https://review.openstack.org/#/c/75827/. :) In addition, the commit in question for glance - https://review.openstack.org/#/c/59150/ didn't have UpgradeImpact, which is the signaling mechanism for these kinds of issues. I do think this is a real issue. OpenStack really is expected to be CD upgradable, not just post release and post release notes. Commits in OpenStack need to take that into account. A compatibility behavior should be put in place here. I do agree the current behavior isn't nice with gorpy error messages all the time. However, a completely legitimate approach would be: If configuration for a storage back end existed, but the driver wasn't explicitly set, load and configure that driver and throw a big DEPRECATION WARNING in the logs that Glance will require explicit loading of drivers in an upcoming release. That would let you move forward, and provide some user signally. That's pretty much what already happens. On startup, Glance will log a message about a particular store driver being disabled [1] because configuration settings were not set properly. [2] IIRC, on startup is the only place these messages occur (not, for instance, every time somebody uploads an image), so I'm not entirely sure what the big deal was. Long term, moving stores into entrypoints might be a cleaner solution, but you will still need to validate configuration for those endpoints on startup -- all endpoints give you is a cleaner method than set your new store in the known_stores configuration option. The issue is this was a failure going from old defaults to new defaults, which is why it was actually blocked by grenade for months. The difference is actually we should be working with the drivers that were configured, and deprecate the fact that you can get away without specifying them. Then you can roll forward gracefully, see a warning message on your working new config, go handle the situation, and later when the backwards compatible behavior is removed you are ok. I think you may have misunderstood me :) I was saying that I don't understand what the big deal was about log messages on startup around failure to configure drivers properly. I didn't think that was something that needed to be fixed. Best, -jay ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Glance] Need to revert Don't enable all stores by default
On Wed, Mar 12, 2014 at 6:40 AM, Sean Dague s...@dague.net wrote: On 03/12/2014 09:01 AM, Flavio Percoco wrote: On 11/03/14 16:25 -0700, Clint Byrum wrote: Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. Here's an attempt to fix this issues without reverting the patch. Feedback appreciated. https://review.openstack.org/#/c/79935/ ACK. Looks pretty good. You might want to consider using one of the oslo deprecation functions to make it consistent on the deprecation side. -Sean -- Sean Dague Samsung Research America s...@dague.net / sean.da...@samsung.com http://dague.net ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev Sorry, I suppose I should have interrogated the backwards-incompatibility assumptions people were making about this change a bit more. It looks like the latest patch is a great deprecation mechanism. Thanks for working out a solution, Flavio et al. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [Glance] Need to revert Don't enable all stores by default
Hi. I asked in #openstack-glance a few times today but got no response, so sorry for the list spam. https://review.openstack.org/#/c/79710/ This change introduces a backward incompatible change to defaults with Havana. If a user has chosen to configure swift, but did not add swift to the known_stores, then when that user upgrades Glance, Glance will fail to start because their swift configuration will be invalid. This broke TripleO btw, which tries hard to use default configurations. Also I am not really sure why this approach was taken. If a user has explicitly put swift configuration options in their config file, why not just load swift store? Oslo.config will help here in that you can just add all of the config options but not actually expect them to be set. It seems entirely backwards to just fail in this case. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
