summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authoresberglu <esberglu@us.ibm.com>2018-02-12 12:18:57 -0600
committeresberglu <esberglu@us.ibm.com>2018-02-13 11:26:47 -0600
commitdb686fe1856456e5e1486f0935aa4426c805520f (patch)
tree9c99ae875644a916ea540b32c781ba03d4e889b3
parentc966524672cedba5c1bc40b76daf0b3745dc0c1d (diff)
Use correct arguments in task inits
The CreateAndConnectCfgDrive and DeleteVOpt tasks were passing the instance into the task.Task init as the first argument. The first arg for the Task init is actually the task name [1]. This means that those task names were string representations of the instance which will cause failures if there are any non-ascii characters. This corrects the Task init calls by removing instance as the first argument. The name kwarg is now being used throughout the PowerVM tasks to avoid similar issues in the future. [1] https://github.com/openstack/taskflow/blob/3.1.0/taskflow/task.py#L62 Change-Id: I991a5ea33daa9a21f774fe24882ed40f990b8e0e Closes-Bug: #1748950
Notes
Notes (review): Code-Review+1: Eric Fried <efried@us.ibm.com> Verified+1: IBM PowerKVM CI <kvmpower@linux.vnet.ibm.com> Code-Review+1: melissaml <ma.lei@99cloud.net> Code-Review+2: Matt Riedemann <mriedem.os@gmail.com> Code-Review+2: Dan Smith <dms@danplanet.com> Workflow+1: Dan Smith <dms@danplanet.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Tue, 13 Feb 2018 23:17:21 +0000 Reviewed-on: https://review.openstack.org/543571 Project: openstack/nova Branch: refs/heads/master
-rw-r--r--nova/tests/unit/virt/powervm/tasks/test_network.py17
-rw-r--r--nova/tests/unit/virt/powervm/tasks/test_storage.py37
-rw-r--r--nova/tests/unit/virt/powervm/tasks/test_vm.py20
-rw-r--r--nova/virt/powervm/tasks/network.py6
-rw-r--r--nova/virt/powervm/tasks/storage.py12
-rw-r--r--nova/virt/powervm/tasks/vm.py8
6 files changed, 87 insertions, 13 deletions
diff --git a/nova/tests/unit/virt/powervm/tasks/test_network.py b/nova/tests/unit/virt/powervm/tasks/test_network.py
index 08bceff..9d6951e 100644
--- a/nova/tests/unit/virt/powervm/tasks/test_network.py
+++ b/nova/tests/unit/virt/powervm/tasks/test_network.py
@@ -78,6 +78,11 @@ class TestNetwork(test.NoDBTestCase):
78 # code was called 78 # code was called
79 self.assertEqual(3, mock_unplug.call_count) 79 self.assertEqual(3, mock_unplug.call_count)
80 80
81 # Validate args on taskflow.task.Task instantiation
82 with mock.patch('taskflow.task.Task.__init__') as tf:
83 tf_net.UnplugVifs(self.apt, inst, net_info)
84 tf.assert_called_once_with(name='unplug_vifs')
85
81 @mock.patch('nova.virt.powervm.vm.get_instance_wrapper') 86 @mock.patch('nova.virt.powervm.vm.get_instance_wrapper')
82 def test_unplug_vifs_invalid_state(self, mock_get_wrap): 87 def test_unplug_vifs_invalid_state(self, mock_get_wrap):
83 """Tests that the delete raises an exception if bad VM state.""" 88 """Tests that the delete raises an exception if bad VM state."""
@@ -129,6 +134,12 @@ class TestNetwork(test.NoDBTestCase):
129 # created. 134 # created.
130 self.assertEqual(pre_cnas + [mock_new_cna], all_cnas) 135 self.assertEqual(pre_cnas + [mock_new_cna], all_cnas)
131 136
137 # Validate args on taskflow.task.Task instantiation
138 with mock.patch('taskflow.task.Task.__init__') as tf:
139 tf_net.PlugVifs(mock.MagicMock(), self.apt, inst, net_info)
140 tf.assert_called_once_with(
141 name='plug_vifs', provides='vm_cnas', requires=['lpar_wrap'])
142
132 @mock.patch('nova.virt.powervm.vif.plug') 143 @mock.patch('nova.virt.powervm.vif.plug')
133 @mock.patch('nova.virt.powervm.vm.get_cnas') 144 @mock.patch('nova.virt.powervm.vm.get_cnas')
134 def test_plug_vifs_rmc_no_create(self, mock_vm_get, mock_plug): 145 def test_plug_vifs_rmc_no_create(self, mock_vm_get, mock_plug):
@@ -288,6 +299,12 @@ class TestNetwork(test.NoDBTestCase):
288 mock_vm_get.assert_not_called() 299 mock_vm_get.assert_not_called()
289 mock_crt_cna.assert_not_called() 300 mock_crt_cna.assert_not_called()
290 301
302 # Validate args on taskflow.task.Task instantiation
303 with mock.patch('taskflow.task.Task.__init__') as tf:
304 tf_net.PlugMgmtVif(self.apt, inst)
305 tf.assert_called_once_with(
306 name='plug_mgmt_vif', provides='mgmt_cna', requires=['vm_cnas'])
307
291 def test_get_vif_events(self): 308 def test_get_vif_events(self):
292 # Set up common mocks. 309 # Set up common mocks.
293 inst = powervm.TEST_INSTANCE 310 inst = powervm.TEST_INSTANCE
diff --git a/nova/tests/unit/virt/powervm/tasks/test_storage.py b/nova/tests/unit/virt/powervm/tasks/test_storage.py
index 1ff78aa..7df5170 100644
--- a/nova/tests/unit/virt/powervm/tasks/test_storage.py
+++ b/nova/tests/unit/virt/powervm/tasks/test_storage.py
@@ -63,6 +63,13 @@ class TestStorage(test.NoDBTestCase):
63 task.revert('mgmt_cna', 'result', 'flow_failures') 63 task.revert('mgmt_cna', 'result', 'flow_failures')
64 self.mock_mb.assert_not_called() 64 self.mock_mb.assert_not_called()
65 65
66 # Validate args on taskflow.task.Task instantiation
67 with mock.patch('taskflow.task.Task.__init__') as tf:
68 tf_stg.CreateAndConnectCfgDrive(
69 self.adapter, self.instance, 'injected_files',
70 'network_info', 'stg_ftsk', admin_pass='admin_pass')
71 tf.assert_called_once_with(name='cfg_drive', requires=['mgmt_cna'])
72
66 def test_delete_vopt(self): 73 def test_delete_vopt(self):
67 # Test with no FeedTask 74 # Test with no FeedTask
68 task = tf_stg.DeleteVOpt(self.adapter, self.instance) 75 task = tf_stg.DeleteVOpt(self.adapter, self.instance)
@@ -81,6 +88,11 @@ class TestStorage(test.NoDBTestCase):
81 self.mock_mb.dlt_vopt.assert_called_once_with( 88 self.mock_mb.dlt_vopt.assert_called_once_with(
82 self.instance, stg_ftsk='ftsk') 89 self.instance, stg_ftsk='ftsk')
83 90
91 # Validate args on taskflow.task.Task instantiation
92 with mock.patch('taskflow.task.Task.__init__') as tf:
93 tf_stg.DeleteVOpt(self.adapter, self.instance)
94 tf.assert_called_once_with(name='vopt_delete')
95
84 def test_delete_disk(self): 96 def test_delete_disk(self):
85 stor_adpt_mappings = mock.Mock() 97 stor_adpt_mappings = mock.Mock()
86 98
@@ -88,11 +100,23 @@ class TestStorage(test.NoDBTestCase):
88 task.execute(stor_adpt_mappings) 100 task.execute(stor_adpt_mappings)
89 self.disk_dvr.delete_disks.assert_called_once_with(stor_adpt_mappings) 101 self.disk_dvr.delete_disks.assert_called_once_with(stor_adpt_mappings)
90 102
103 # Validate args on taskflow.task.Task instantiation
104 with mock.patch('taskflow.task.Task.__init__') as tf:
105 tf_stg.DeleteDisk(self.disk_dvr)
106 tf.assert_called_once_with(
107 name='delete_disk', requires=['stor_adpt_mappings'])
108
91 def test_detach_disk(self): 109 def test_detach_disk(self):
92 task = tf_stg.DetachDisk(self.disk_dvr, self.instance) 110 task = tf_stg.DetachDisk(self.disk_dvr, self.instance)
93 task.execute() 111 task.execute()
94 self.disk_dvr.detach_disk.assert_called_once_with(self.instance) 112 self.disk_dvr.detach_disk.assert_called_once_with(self.instance)
95 113
114 # Validate args on taskflow.task.Task instantiation
115 with mock.patch('taskflow.task.Task.__init__') as tf:
116 tf_stg.DetachDisk(self.disk_dvr, self.instance)
117 tf.assert_called_once_with(
118 name='detach_disk', provides='stor_adpt_mappings')
119
96 def test_attach_disk(self): 120 def test_attach_disk(self):
97 stg_ftsk = mock.Mock() 121 stg_ftsk = mock.Mock()
98 disk_dev_info = mock.Mock() 122 disk_dev_info = mock.Mock()
@@ -113,6 +137,12 @@ class TestStorage(test.NoDBTestCase):
113 task.revert(disk_dev_info, 'result', 'flow failures') 137 task.revert(disk_dev_info, 'result', 'flow failures')
114 self.disk_dvr.detach_disk.assert_called_once_with(self.instance) 138 self.disk_dvr.detach_disk.assert_called_once_with(self.instance)
115 139
140 # Validate args on taskflow.task.Task instantiation
141 with mock.patch('taskflow.task.Task.__init__') as tf:
142 tf_stg.AttachDisk(self.disk_dvr, self.instance, stg_ftsk)
143 tf.assert_called_once_with(
144 name='attach_disk', requires=['disk_dev_info'])
145
116 def test_create_disk_for_img(self): 146 def test_create_disk_for_img(self):
117 image_meta = mock.Mock() 147 image_meta = mock.Mock()
118 148
@@ -136,3 +166,10 @@ class TestStorage(test.NoDBTestCase):
136 "timed out") 166 "timed out")
137 task.revert('result', 'flow failures') 167 task.revert('result', 'flow failures')
138 self.disk_dvr.delete_disks.assert_called_once_with(['result']) 168 self.disk_dvr.delete_disks.assert_called_once_with(['result'])
169
170 # Validate args on taskflow.task.Task instantiation
171 with mock.patch('taskflow.task.Task.__init__') as tf:
172 tf_stg.CreateDiskForImg(
173 self.disk_dvr, self.context, self.instance, image_meta)
174 tf.assert_called_once_with(
175 name='create_disk_from_img', provides='disk_dev_info')
diff --git a/nova/tests/unit/virt/powervm/tasks/test_vm.py b/nova/tests/unit/virt/powervm/tasks/test_vm.py
index 08a841c..2a5988b 100644
--- a/nova/tests/unit/virt/powervm/tasks/test_vm.py
+++ b/nova/tests/unit/virt/powervm/tasks/test_vm.py
@@ -47,12 +47,22 @@ class TestVMTasks(test.NoDBTestCase):
47 mock_stg.assert_called_once_with([mock_vm_crt.return_value.id], 'ftsk', 47 mock_stg.assert_called_once_with([mock_vm_crt.return_value.id], 'ftsk',
48 lpars_exist=True) 48 lpars_exist=True)
49 49
50 # Validate args on taskflow.task.Task instantiation
51 with mock.patch('taskflow.task.Task.__init__') as tf:
52 tf_vm.Create(self.apt, 'host_wrapper', self.instance, 'ftsk')
53 tf.assert_called_once_with(name='crt_vm', provides='lpar_wrap')
54
50 @mock.patch('nova.virt.powervm.vm.power_on') 55 @mock.patch('nova.virt.powervm.vm.power_on')
51 def test_power_on(self, mock_pwron): 56 def test_power_on(self, mock_pwron):
52 pwron = tf_vm.PowerOn(self.apt, self.instance) 57 pwron = tf_vm.PowerOn(self.apt, self.instance)
53 pwron.execute() 58 pwron.execute()
54 mock_pwron.assert_called_once_with(self.apt, self.instance) 59 mock_pwron.assert_called_once_with(self.apt, self.instance)
55 60
61 # Validate args on taskflow.task.Task instantiation
62 with mock.patch('taskflow.task.Task.__init__') as tf:
63 tf_vm.PowerOn(self.apt, self.instance)
64 tf.assert_called_once_with(name='pwr_vm')
65
56 @mock.patch('nova.virt.powervm.vm.power_on') 66 @mock.patch('nova.virt.powervm.vm.power_on')
57 @mock.patch('nova.virt.powervm.vm.power_off') 67 @mock.patch('nova.virt.powervm.vm.power_off')
58 def test_power_on_revert(self, mock_pwroff, mock_pwron): 68 def test_power_on_revert(self, mock_pwroff, mock_pwron):
@@ -96,8 +106,18 @@ class TestVMTasks(test.NoDBTestCase):
96 mock_pwroff.assert_called_once_with(self.apt, self.instance, 106 mock_pwroff.assert_called_once_with(self.apt, self.instance,
97 force_immediate=True) 107 force_immediate=True)
98 108
109 # Validate args on taskflow.task.Task instantiation
110 with mock.patch('taskflow.task.Task.__init__') as tf:
111 tf_vm.PowerOff(self.apt, self.instance)
112 tf.assert_called_once_with(name='pwr_off_vm')
113
99 @mock.patch('nova.virt.powervm.vm.delete_lpar') 114 @mock.patch('nova.virt.powervm.vm.delete_lpar')
100 def test_delete(self, mock_dlt): 115 def test_delete(self, mock_dlt):
101 delete = tf_vm.Delete(self.apt, self.instance) 116 delete = tf_vm.Delete(self.apt, self.instance)
102 delete.execute() 117 delete.execute()
103 mock_dlt.assert_called_once_with(self.apt, self.instance) 118 mock_dlt.assert_called_once_with(self.apt, self.instance)
119
120 # Validate args on taskflow.task.Task instantiation
121 with mock.patch('taskflow.task.Task.__init__') as tf:
122 tf_vm.Delete(self.apt, self.instance)
123 tf.assert_called_once_with(name='dlt_vm')
diff --git a/nova/virt/powervm/tasks/network.py b/nova/virt/powervm/tasks/network.py
index 12f3030..d96ff25 100644
--- a/nova/virt/powervm/tasks/network.py
+++ b/nova/virt/powervm/tasks/network.py
@@ -59,7 +59,7 @@ class PlugVifs(task.Task):
59 self.cnas = None 59 self.cnas = None
60 60
61 super(PlugVifs, self).__init__( 61 super(PlugVifs, self).__init__(
62 'plug_vifs', provides='vm_cnas', requires=['lpar_wrap']) 62 name='plug_vifs', provides='vm_cnas', requires=['lpar_wrap'])
63 63
64 def _vif_exists(self, network_info): 64 def _vif_exists(self, network_info):
65 """Does the instance have a CNA for a given net? 65 """Does the instance have a CNA for a given net?
@@ -180,7 +180,7 @@ class UnplugVifs(task.Task):
180 self.instance = instance 180 self.instance = instance
181 self.network_infos = network_infos or [] 181 self.network_infos = network_infos or []
182 182
183 super(UnplugVifs, self).__init__('unplug_vifs') 183 super(UnplugVifs, self).__init__(name='unplug_vifs')
184 184
185 def execute(self): 185 def execute(self):
186 # If the LPAR is not in an OK state for deleting, then throw an 186 # If the LPAR is not in an OK state for deleting, then throw an
@@ -222,7 +222,7 @@ class PlugMgmtVif(task.Task):
222 self.instance = instance 222 self.instance = instance
223 223
224 super(PlugMgmtVif, self).__init__( 224 super(PlugMgmtVif, self).__init__(
225 'plug_mgmt_vif', provides='mgmt_cna', requires=['vm_cnas']) 225 name='plug_mgmt_vif', provides='mgmt_cna', requires=['vm_cnas'])
226 226
227 def execute(self, vm_cnas): 227 def execute(self, vm_cnas):
228 LOG.info('Plugging the Management Network Interface to instance.', 228 LOG.info('Plugging the Management Network Interface to instance.',
diff --git a/nova/virt/powervm/tasks/storage.py b/nova/virt/powervm/tasks/storage.py
index 50d14af..5b72ac0 100644
--- a/nova/virt/powervm/tasks/storage.py
+++ b/nova/virt/powervm/tasks/storage.py
@@ -39,7 +39,7 @@ class CreateDiskForImg(task.Task):
39 The metadata of the image of the instance. 39 The metadata of the image of the instance.
40 """ 40 """
41 super(CreateDiskForImg, self).__init__( 41 super(CreateDiskForImg, self).__init__(
42 'create_disk_from_img', provides='disk_dev_info') 42 name='create_disk_from_img', provides='disk_dev_info')
43 self.disk_dvr = disk_dvr 43 self.disk_dvr = disk_dvr
44 self.instance = instance 44 self.instance = instance
45 self.context = context 45 self.context = context
@@ -80,7 +80,7 @@ class AttachDisk(task.Task):
80 :param stg_ftsk: FeedTask to defer storage connectivity operations. 80 :param stg_ftsk: FeedTask to defer storage connectivity operations.
81 """ 81 """
82 super(AttachDisk, self).__init__( 82 super(AttachDisk, self).__init__(
83 'attach_disk', requires=['disk_dev_info']) 83 name='attach_disk', requires=['disk_dev_info'])
84 self.disk_dvr = disk_dvr 84 self.disk_dvr = disk_dvr
85 self.instance = instance 85 self.instance = instance
86 self.stg_ftsk = stg_ftsk 86 self.stg_ftsk = stg_ftsk
@@ -111,7 +111,7 @@ class DetachDisk(task.Task):
111 :param instance: The nova instance. 111 :param instance: The nova instance.
112 """ 112 """
113 super(DetachDisk, self).__init__( 113 super(DetachDisk, self).__init__(
114 'detach_disk', provides='stor_adpt_mappings') 114 name='detach_disk', provides='stor_adpt_mappings')
115 self.instance = instance 115 self.instance = instance
116 self.disk_dvr = disk_dvr 116 self.disk_dvr = disk_dvr
117 117
@@ -131,7 +131,7 @@ class DeleteDisk(task.Task):
131 :param disk_dvr: The DiskAdapter for the VM. 131 :param disk_dvr: The DiskAdapter for the VM.
132 """ 132 """
133 super(DeleteDisk, self).__init__( 133 super(DeleteDisk, self).__init__(
134 'delete_disk', requires=['stor_adpt_mappings']) 134 name='delete_disk', requires=['stor_adpt_mappings'])
135 self.disk_dvr = disk_dvr 135 self.disk_dvr = disk_dvr
136 136
137 def execute(self, stor_adpt_mappings): 137 def execute(self, stor_adpt_mappings):
@@ -158,7 +158,7 @@ class CreateAndConnectCfgDrive(task.Task):
158 VM. 158 VM.
159 """ 159 """
160 super(CreateAndConnectCfgDrive, self).__init__( 160 super(CreateAndConnectCfgDrive, self).__init__(
161 instance, 'cfg_drive', requires=['mgmt_cna']) 161 name='cfg_drive', requires=['mgmt_cna'])
162 self.adapter = adapter 162 self.adapter = adapter
163 self.instance = instance 163 self.instance = instance
164 self.injected_files = injected_files 164 self.injected_files = injected_files
@@ -197,7 +197,7 @@ class DeleteVOpt(task.Task):
197 :param instance: The nova instance. 197 :param instance: The nova instance.
198 :param stg_ftsk: FeedTask to defer storage connectivity operations. 198 :param stg_ftsk: FeedTask to defer storage connectivity operations.
199 """ 199 """
200 super(DeleteVOpt, self).__init__(instance, 'vopt_delete') 200 super(DeleteVOpt, self).__init__(name='vopt_delete')
201 self.adapter = adapter 201 self.adapter = adapter
202 self.instance = instance 202 self.instance = instance
203 self.stg_ftsk = stg_ftsk 203 self.stg_ftsk = stg_ftsk
diff --git a/nova/virt/powervm/tasks/vm.py b/nova/virt/powervm/tasks/vm.py
index 773b199..58f3301 100644
--- a/nova/virt/powervm/tasks/vm.py
+++ b/nova/virt/powervm/tasks/vm.py
@@ -48,7 +48,7 @@ class Create(task.Task):
48 :param instance: The nova instance. 48 :param instance: The nova instance.
49 :param stg_ftsk: FeedTask to defer storage connectivity operations. 49 :param stg_ftsk: FeedTask to defer storage connectivity operations.
50 """ 50 """
51 super(Create, self).__init__('crt_vm', provides='lpar_wrap') 51 super(Create, self).__init__(name='crt_vm', provides='lpar_wrap')
52 self.instance = instance 52 self.instance = instance
53 self.adapter = adapter 53 self.adapter = adapter
54 self.host_wrapper = host_wrapper 54 self.host_wrapper = host_wrapper
@@ -74,7 +74,7 @@ class PowerOn(task.Task):
74 :param adapter: The pypowervm adapter. 74 :param adapter: The pypowervm adapter.
75 :param instance: The nova instance. 75 :param instance: The nova instance.
76 """ 76 """
77 super(PowerOn, self).__init__('pwr_vm') 77 super(PowerOn, self).__init__(name='pwr_vm')
78 self.adapter = adapter 78 self.adapter = adapter
79 self.instance = instance 79 self.instance = instance
80 80
@@ -107,7 +107,7 @@ class PowerOff(task.Task):
107 :param instance: The nova instance. 107 :param instance: The nova instance.
108 :param force_immediate: Boolean. Perform a VSP hard power off. 108 :param force_immediate: Boolean. Perform a VSP hard power off.
109 """ 109 """
110 super(PowerOff, self).__init__('pwr_off_vm') 110 super(PowerOff, self).__init__(name='pwr_off_vm')
111 self.instance = instance 111 self.instance = instance
112 self.adapter = adapter 112 self.adapter = adapter
113 self.force_immediate = force_immediate 113 self.force_immediate = force_immediate
@@ -126,7 +126,7 @@ class Delete(task.Task):
126 :param adapter: The adapter for the pypowervm API. 126 :param adapter: The adapter for the pypowervm API.
127 :param instance: The nova instance. 127 :param instance: The nova instance.
128 """ 128 """
129 super(Delete, self).__init__('dlt_vm') 129 super(Delete, self).__init__(name='dlt_vm')
130 self.adapter = adapter 130 self.adapter = adapter
131 self.instance = instance 131 self.instance = instance
132 132