adding a new driver requires changing patcher.py

Bug #1353631 reported by aeva black
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ironic
Fix Released
Medium
Lucas Alvares Gomes

Bug Description

When adding a new deploy driver to Ironic (eg, the IPA and iLO drivers), we currently have to change the nova.virt.ironic.patcher module. This breaks the abstraction -- Nova-compute should not need to behave differently for different Ironic drivers.

In nova/virt/ironic/patcher.py, there are currently two classes:
  GenericDriverFields
  PXEDriverFields(GenericDriverFields)

The PXEDriverFields class never calls any methods in the base class, so there's currently no benefit to the inheritence. Further, the PXEDriverFIelds class contains all the handling of "instance_info" -- which, by design, should be common across *all* drivers (not just PXE drivers). Since the GenericDriverFields class does not do anything, a node for which this class is used will not receive any "instance_info" and will be unprovisionable by Ironic. Thus the problem encountered by the IPA and iLO drivers.

In Icehouse, Ironic required the pxe deploy kernel & ramdisk to be stored on the Nova flavor. This is no longer required, but is still allowed for backwards-compatibility, and this code is here:
  https://github.com/openstack/ironic/blob/master/ironic/nova/virt/ironic/patcher.py#L102-L105

All the rest, which modifies "instance_info", should be moved to the GenericDriverFields class.

Tags: nova-driver
aeva black (tenbrae)
summary: - patcher.py inappropriately handling instance_info as though it's part of
- the PXE driver
+ adding a new driver requires changing patcher.py
Changed in ironic:
status: New → Triaged
importance: Undecided → High
milestone: none → juno-3
description: updated
tags: added: nova-driver
Changed in ironic:
importance: High → Medium
Changed in ironic:
assignee: nobody → Lucas Alvares Gomes (lucasagomes)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ironic (master)

Fix proposed to branch: master
Review: https://review.openstack.org/112623

Changed in ironic:
status: Triaged → In Progress
Changed in ironic:
assignee: Lucas Alvares Gomes (lucasagomes) → Devananda van der Veen (devananda)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ironic (master)

Reviewed: https://review.openstack.org/112623
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=1062b79a35763a9f9257d9743f884202525bc8a7
Submitter: Jenkins
Branch: master

commit 1062b79a35763a9f9257d9743f884202525bc8a7
Author: Lucas Alvares Gomes <email address hidden>
Date: Thu Aug 7 17:39:09 2014 +0100

    Move the 'instance_info' fields to GenericDriverFields

    The instance_info parameters are common across all drivers and not
    just for PXE drivers.

    This patch corrects an issue in nova/virt/ironic/patcher.py by moving
    the generation of the instance_info patch to the common base class.
    It also adds notes stating the intent to remove the PXE-specific
    patching in Kilo.

    Closes-Bug: #1353631
    Change-Id: I80d2208d41d21821f0da662637418ecd76b5db2b

Changed in ironic:
status: In Progress → Fix Committed
aeva black (tenbrae)
Changed in ironic:
assignee: Devananda van der Veen (devananda) → Lucas Alvares Gomes (lucasagomes)
Thierry Carrez (ttx)
Changed in ironic:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in ironic:
milestone: juno-3 → 2014.2
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.