[RFE] Moving network interface calls out of the deployment interface

Bug #1679834 reported by Vladyslav Drok
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ironic
Fix Released
Wishlist
Vladyslav Drok

Bug Description

In ironic, we have multiple places where we are calling network interface from inside the deployment interface, for both our main deployment machanisms - agent (e.g. https://github.com/openstack/ironic/blob/stable/ocata/ironic/drivers/modules/agent.py#L403) and iscsi (e.g. https://github.com/openstack/ironic/blob/stable/ocata/ironic/drivers/modules/iscsi_deploy.py#L471). From my POV, this brings several problems:

1. This makes the third party deploy driver maintainers to make changes to their drivers to support things like multitenancy, or the storage interface actions being currently proposed;
2. We can not ensure that third-party drivers are doing the right thing with networking setup (at least on the abstract level, this of course does not prevent anyone to write a flawed network interface), which may lead to security issues;
3. This basically means that our abstraction for interfaces is not good, as each interface should be doing a specific thing that is defined by the interfaces' API, and should not directly call other interfaces; instead, the coordination should be done in conductor.

Basically, to move the network interface calls to conductor, the changes are pretty minimal, as most of the things we do with networking are idempotent, so even if third party deploy interfaces will keep calling the network interface, they will not break. We'll also try to communicate that the conductor now deals with networking stuff, so there is no need to put it in the deploy interface.

The only change that might be non-trivial is finishing the deployment, as this is completely in control of the deployment interface, so in conductor, we don't know when do we have to switch to the tenant network. This can be mitigated by introducing some kind of callback (in task manager?) that will be called on 'done' event for nodes being deployed.

Tags: needs-spec rfe
Vladyslav Drok (vdrok)
Changed in ironic:
importance: Undecided → Wishlist
assignee: nobody → Vladyslav Drok (vdrok)
tags: added: rfe
Changed in ironic:
status: New → In Progress
Revision history for this message
Sam Betts (sambetts) wrote :

I'm not sure I think this a good idea, or the correct understanding of Ironic's architecture.

My understanding of the Ironic interface architecture is that the deploy interface is the main interface that understands the whole process for deploying that node and will use the other interfaces to achieve its goal. If that is the case then the conductor should only need to call the deploy interface for deploying a node and the deploy interface should be charge of calling power, network, boot etc at the right time in its particular deployment process which might differ depending on deployment process or hardware etc.

Revision history for this message
Vladyslav Drok (vdrok) wrote :

Right, that's one opinion. My thought on this is, deploy interface should only implement the things that are defined in base deploy interface. There is no easy way to define that e.g. in reboot_and_finish_deploy method you need to do remove_provisioning_network, add_tenant_network, attach_storage etc. If it was all done by the conductor, the deploy interface could concentrate solely on the thing it was created for - writing the image on the node's disk, either via iscsi, or agent commands, or like in case of ansible deploy driver running ansible playbooks there. This would also mean that implementing cinder storage interface enables it for all deploy interfaces, not just the two that we currently have in tree.

Revision history for this message
Vladyslav Drok (vdrok) wrote :

dtantsur wanted a spec, added a tag

tags: added: needs-spec
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to ironic-specs (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/468000

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to ironic (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/476601

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on ironic-specs (master)

Change abandoned by Vladyslav Drok (<email address hidden>) on branch: master
Review: https://review.openstack.org/468000

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on ironic (master)

Change abandoned by Vladyslav Drok (<email address hidden>) on branch: master
Review: https://review.openstack.org/453139

Changed in ironic:
status: In Progress → Fix Released
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.