Re: [openstack-dev] [ironic] [inspector] RFC: deprecating "set IPMI credentials" feature in ironic-inspector

2017-01-05 Thread Dmitry Tantsur

On 12/13/2016 01:40 PM, Dmitry Tantsur wrote:

Hi folks!

Since nearly its beginning, ironic-inspector has had a controversial feature: we
allow a user to request changing IPMI credentials of the node after
introspection. The new credentials are passed back from inspector to the
ramdisk, and the ramdisk calls "ipmitool" to set them.

Now I realize that the feature has quite a few substantial drawbacks:
1. It's a special case in ironic-inspector. It's the only thing that runs after
introspection, and it requires special state machine states and actions.
2. There is no way to signal errors back from the ramdisk. We can only poll the
nodes to see if the new credentials match.
3. This is the only place where ironic-inspector modifies physical nodes (as
opposed to modifying the ironic database). This feels like a violation of our 
goal.
4. It depends on ipmitool actually being able to update credentials from within
the node without knowing the current ones. I'm not sure how wildly it's
supported. I'm pretty sure some hardware does not support it.
5. It's not and never will be tested by any CI. It's not possible to test on VMs
at all.
6. Due to its dangerous nature, this feature is hidden behind a configuration
option, and is disabled by default.

The upside I see is that it may play nicely with node autodiscovery. I'm not
sure they work together today, though. We didn't end up using this feature in
our products, and I don't recall being approached by people using it.

I suggest deprecating this feature and removing it in Pike. The rough plan is as
follows:

I. Ocata:
 * Deprecate the configuration option enabling this feature.
 * Create an API version that returns HTTP 400 when this feature is requested.


A review is posted for this part: https://review.openstack.org/#/c/417041/


 * Deprecate the associated arguments in CLI.
 * Issue a deprecating warning in IPA when this feature is used.

II. Pike:
 * Remove the feature from IPA and ironic-inspector.
 * Remove the feature from CLI.

Please respond with your comments and/or objects to this thread. I'll soon
prepare a patch on which you'll also be able to comment.

Dmitry.

__
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


Re: [openstack-dev] [ironic] [inspector] RFC: deprecating "set IPMI credentials" feature in ironic-inspector

2016-12-13 Thread Jay Faulkner

> On Dec 13, 2016, at 4:40 AM, Dmitry Tantsur  wrote:
> 
> Hi folks!
> 
> Since nearly its beginning, ironic-inspector has had a controversial feature: 
> we allow a user to request changing IPMI credentials of the node after 
> introspection. The new credentials are passed back from inspector to the 
> ramdisk, and the ramdisk calls "ipmitool" to set them.
> 
> Now I realize that the feature has quite a few substantial drawbacks:
> 1. It's a special case in ironic-inspector. It's the only thing that runs 
> after introspection, and it requires special state machine states and actions.
> 2. There is no way to signal errors back from the ramdisk. We can only poll 
> the nodes to see if the new credentials match.
> 3. This is the only place where ironic-inspector modifies physical nodes (as 
> opposed to modifying the ironic database). This feels like a violation of our 
> goal.
> 4. It depends on ipmitool actually being able to update credentials from 
> within the node without knowing the current ones. I'm not sure how wildly 
> it's supported. I'm pretty sure some hardware does not support it.
> 5. It's not and never will be tested by any CI. It's not possible to test on 
> VMs at all.
> 6. Due to its dangerous nature, this feature is hidden behind a configuration 
> option, and is disabled by default.
> 
> The upside I see is that it may play nicely with node autodiscovery. I'm not 
> sure they work together today, though. We didn't end up using this feature in 
> our products, and I don't recall being approached by people using it.
> 
> I suggest deprecating this feature and removing it in Pike. The rough plan is 
> as follows:
> 
> I. Ocata:
> * Deprecate the configuration option enabling this feature.
> * Create an API version that returns HTTP 400 when this feature is requested.
> * Deprecate the associated arguments in CLI.
> * Issue a deprecating warning in IPA when this feature is used.
> 
> II. Pike:
> * Remove the feature from IPA and ironic-inspector.
> * Remove the feature from CLI.
> 
> Please respond with your comments and/or objects to this thread. I'll soon 
> prepare a patch on which you'll also be able to comment.
> 

I agree with deprecating this version of this feature. I do see the potential 
for credential rotation as a thing Ironic could handle in the future, but it 
would need to be handled in a periodic fashion vs being done once at startup.

I’m +2 to what’s proposed.

-Jay

> Dmitry.
> 
> __
> 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


Re: [openstack-dev] [ironic] [inspector] RFC: deprecating "set IPMI credentials" feature in ironic-inspector

2016-12-13 Thread Mario Villaplana
Given that this seems to be a special case outside of what
ironic-inspector normally handles, I'd be +1 to removing it.

Perhaps this feature, if needed, is best implemented as a vendor
passthru in ironic itself.

Mario

On Tue, Dec 13, 2016 at 7:40 AM, Dmitry Tantsur  wrote:
> Hi folks!
>
> Since nearly its beginning, ironic-inspector has had a controversial
> feature: we allow a user to request changing IPMI credentials of the node
> after introspection. The new credentials are passed back from inspector to
> the ramdisk, and the ramdisk calls "ipmitool" to set them.
>
> Now I realize that the feature has quite a few substantial drawbacks:
> 1. It's a special case in ironic-inspector. It's the only thing that runs
> after introspection, and it requires special state machine states and
> actions.
> 2. There is no way to signal errors back from the ramdisk. We can only poll
> the nodes to see if the new credentials match.
> 3. This is the only place where ironic-inspector modifies physical nodes (as
> opposed to modifying the ironic database). This feels like a violation of
> our goal.
> 4. It depends on ipmitool actually being able to update credentials from
> within the node without knowing the current ones. I'm not sure how wildly
> it's supported. I'm pretty sure some hardware does not support it.
> 5. It's not and never will be tested by any CI. It's not possible to test on
> VMs at all.
> 6. Due to its dangerous nature, this feature is hidden behind a
> configuration option, and is disabled by default.
>
> The upside I see is that it may play nicely with node autodiscovery. I'm not
> sure they work together today, though. We didn't end up using this feature
> in our products, and I don't recall being approached by people using it.
>
> I suggest deprecating this feature and removing it in Pike. The rough plan
> is as follows:
>
> I. Ocata:
>  * Deprecate the configuration option enabling this feature.
>  * Create an API version that returns HTTP 400 when this feature is
> requested.
>  * Deprecate the associated arguments in CLI.
>  * Issue a deprecating warning in IPA when this feature is used.
>
> II. Pike:
>  * Remove the feature from IPA and ironic-inspector.
>  * Remove the feature from CLI.
>
> Please respond with your comments and/or objects to this thread. I'll soon
> prepare a patch on which you'll also be able to comment.
>
> Dmitry.
>
> __
> 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


[openstack-dev] [ironic] [inspector] RFC: deprecating "set IPMI credentials" feature in ironic-inspector

2016-12-13 Thread Dmitry Tantsur

Hi folks!

Since nearly its beginning, ironic-inspector has had a controversial feature: we 
allow a user to request changing IPMI credentials of the node after 
introspection. The new credentials are passed back from inspector to the 
ramdisk, and the ramdisk calls "ipmitool" to set them.


Now I realize that the feature has quite a few substantial drawbacks:
1. It's a special case in ironic-inspector. It's the only thing that runs after 
introspection, and it requires special state machine states and actions.
2. There is no way to signal errors back from the ramdisk. We can only poll the 
nodes to see if the new credentials match.
3. This is the only place where ironic-inspector modifies physical nodes (as 
opposed to modifying the ironic database). This feels like a violation of our goal.
4. It depends on ipmitool actually being able to update credentials from within 
the node without knowing the current ones. I'm not sure how wildly it's 
supported. I'm pretty sure some hardware does not support it.
5. It's not and never will be tested by any CI. It's not possible to test on VMs 
at all.
6. Due to its dangerous nature, this feature is hidden behind a configuration 
option, and is disabled by default.


The upside I see is that it may play nicely with node autodiscovery. I'm not 
sure they work together today, though. We didn't end up using this feature in 
our products, and I don't recall being approached by people using it.


I suggest deprecating this feature and removing it in Pike. The rough plan is as 
follows:


I. Ocata:
 * Deprecate the configuration option enabling this feature.
 * Create an API version that returns HTTP 400 when this feature is requested.
 * Deprecate the associated arguments in CLI.
 * Issue a deprecating warning in IPA when this feature is used.

II. Pike:
 * Remove the feature from IPA and ironic-inspector.
 * Remove the feature from CLI.

Please respond with your comments and/or objects to this thread. I'll soon 
prepare a patch on which you'll also be able to comment.


Dmitry.

__
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