|
1 Upstream patch to address CVE-2015-3280. This fix will be included in |
|
2 the future 2014.2.3 (juno) release. |
|
3 |
|
4 From fa72fb8b51d59e04913c871539cee98a3da79058 Mon Sep 17 00:00:00 2001 |
|
5 From: Rajesh Tailor <[email protected]> |
|
6 Date: Wed, 4 Mar 2015 05:05:19 -0800 |
|
7 Subject: Delete orphaned instance files from compute nodes |
|
8 |
|
9 While resizing/revert-resizing instance, if instance gets deleted |
|
10 in between, then instance files remains either on the source or |
|
11 destination compute node. |
|
12 |
|
13 To address this issue, added a new periodic task |
|
14 '_cleanup_incomplete_migrations' which takes care of deleting |
|
15 instance files from source/destination compute nodes and then |
|
16 mark migration record as failed so that it doesn't appear again |
|
17 in the next periodic task run. |
|
18 |
|
19 Conflicts: |
|
20 nova/compute/manager.py |
|
21 nova/tests/unit/compute/test_compute_mgr.py |
|
22 |
|
23 SecurityImpact |
|
24 Closes-Bug: 1392527 |
|
25 Change-Id: I9866d8e32e99b9f907921f4b226edf7b62bd83a7 |
|
26 (cherry picked from commit 18d6b5cc79973fc553daf7a92f22cce4dc0ca013) |
|
27 |
|
28 --- nova-2014.2.2/nova/compute/manager.py.~1~ 2015-09-02 14:46:43.532548379 -0700 |
|
29 +++ nova-2014.2.2/nova/compute/manager.py 2015-09-02 14:47:57.813280934 -0700 |
|
30 @@ -257,12 +257,18 @@ def errors_out_migration(function): |
|
31 def decorated_function(self, context, *args, **kwargs): |
|
32 try: |
|
33 return function(self, context, *args, **kwargs) |
|
34 - except Exception: |
|
35 + except Exception as ex: |
|
36 with excutils.save_and_reraise_exception(): |
|
37 migration = kwargs['migration'] |
|
38 - status = migration.status |
|
39 - if status not in ['migrating', 'post-migrating']: |
|
40 - return |
|
41 + |
|
42 + # NOTE(rajesht): If InstanceNotFound error is thrown from |
|
43 + # decorated function, migration status should be set to |
|
44 + # 'error', without checking current migration status. |
|
45 + if not isinstance(ex, exception.InstanceNotFound): |
|
46 + status = migration.status |
|
47 + if status not in ['migrating', 'post-migrating']: |
|
48 + return |
|
49 + |
|
50 migration.status = 'error' |
|
51 try: |
|
52 migration.save(context.elevated()) |
|
53 @@ -3469,6 +3475,7 @@ class ComputeManager(manager.Manager): |
|
54 @wrap_exception() |
|
55 @reverts_task_state |
|
56 @wrap_instance_event |
|
57 + @errors_out_migration |
|
58 @wrap_instance_fault |
|
59 def revert_resize(self, context, instance, migration, reservations): |
|
60 """Destroys the new instance on the destination machine. |
|
61 @@ -3523,6 +3530,7 @@ class ComputeManager(manager.Manager): |
|
62 @wrap_exception() |
|
63 @reverts_task_state |
|
64 @wrap_instance_event |
|
65 + @errors_out_migration |
|
66 @wrap_instance_fault |
|
67 def finish_revert_resize(self, context, instance, reservations, migration): |
|
68 """Finishes the second half of reverting a resize. |
|
69 @@ -6246,3 +6254,48 @@ class ComputeManager(manager.Manager): |
|
70 instance.cleaned = True |
|
71 with utils.temporary_mutation(context, read_deleted='yes'): |
|
72 instance.save(context) |
|
73 + |
|
74 + @periodic_task.periodic_task(spacing=CONF.instance_delete_interval) |
|
75 + def _cleanup_incomplete_migrations(self, context): |
|
76 + """Delete instance files on failed resize/revert-resize operation |
|
77 + |
|
78 + During resize/revert-resize operation, if that instance gets deleted |
|
79 + in-between then instance files might remain either on source or |
|
80 + destination compute node because of race condition. |
|
81 + """ |
|
82 + LOG.debug('Cleaning up deleted instances with incomplete migration ') |
|
83 + migration_filters = {'host':CONF.host, |
|
84 + 'status': 'error'} |
|
85 + migrations = objects.MigrationList.get_by_filters(context, |
|
86 + migration_filters) |
|
87 + |
|
88 + if not migrations: |
|
89 + return |
|
90 + |
|
91 + inst_uuid_from_migrations = set([migration.instance_uuid for migration |
|
92 + in migrations]) |
|
93 + |
|
94 + inst_filters = {'deleted': True, 'soft_deleted': False, |
|
95 + 'uuid': inst_uuid_from_migrations} |
|
96 + attrs = ['info_cache', 'security_groups', 'system_metadata'] |
|
97 + with utils.temporary_mutation(context, read_deleted='yes'): |
|
98 + instances = objects.InstanceList.get_by_filters( |
|
99 + context, inst_filters, expected_attrs=attrs, use_slave=True) |
|
100 + |
|
101 + for instance in instances: |
|
102 + if instance.host != CONF.host: |
|
103 + for migration in migrations: |
|
104 + if instance.uuid == migration.instance_uuid: |
|
105 + # Delete instance files if not cleanup properly either |
|
106 + # from the source or destination compute nodes when |
|
107 + # the instance is deleted during resizing. |
|
108 + self.driver.delete_instance_files(instance) |
|
109 + try: |
|
110 + migration.status = 'failed' |
|
111 + migration.save(context.elevated()) |
|
112 + except exception.MigrationNotFound: |
|
113 + LOG.warning(_LW("Migration %s is not found."), |
|
114 + migration.id, context=context, |
|
115 + instance=instance) |
|
116 + break |
|
117 + |
|
118 |
|
119 --- ./nova/tests/compute/test_compute_mgr.py.~1~ 2015-09-29 09:45:07.760433246 -0700 |
|
120 +++ ./nova/tests/compute/test_compute_mgr.py 2015-09-29 09:48:00.008811912 -0700 |
|
121 @@ -1047,6 +1047,79 @@ class ComputeManagerUnitTestCase(test.No |
|
122 self.assertFalse(c.cleaned) |
|
123 self.assertEqual('1', c.system_metadata['clean_attempts']) |
|
124 |
|
125 + @mock.patch.object(objects.Migration, 'save') |
|
126 + @mock.patch.object(objects.MigrationList, 'get_by_filters') |
|
127 + @mock.patch.object(objects.InstanceList, 'get_by_filters') |
|
128 + def _test_cleanup_incomplete_migrations(self, inst_host, |
|
129 + mock_inst_get_by_filters, |
|
130 + mock_migration_get_by_filters, |
|
131 + mock_save): |
|
132 + def fake_inst(context, uuid, host): |
|
133 + inst = objects.Instance(context) |
|
134 + inst.uuid = uuid |
|
135 + inst.host = host |
|
136 + return inst |
|
137 + |
|
138 + def fake_migration(uuid, status, inst_uuid, src_host, dest_host): |
|
139 + migration = objects.Migration() |
|
140 + migration.uuid = uuid |
|
141 + migration.status = status |
|
142 + migration.instance_uuid = inst_uuid |
|
143 + migration.source_compute = src_host |
|
144 + migration.dest_compute = dest_host |
|
145 + return migration |
|
146 + |
|
147 + fake_instances = [fake_inst(self.context, '111', inst_host), |
|
148 + fake_inst(self.context, '222', inst_host)] |
|
149 + |
|
150 + fake_migrations = [fake_migration('123', 'error', '111', |
|
151 + 'fake-host', 'fake-mini'), |
|
152 + fake_migration('456', 'error', '222', |
|
153 + 'fake-host', 'fake-mini')] |
|
154 + |
|
155 + mock_migration_get_by_filters.return_value = fake_migrations |
|
156 + mock_inst_get_by_filters.return_value = fake_instances |
|
157 + |
|
158 + with mock.patch.object(self.compute.driver, 'delete_instance_files'): |
|
159 + self.compute._cleanup_incomplete_migrations(self.context) |
|
160 + |
|
161 + # Ensure that migration status is set to 'failed' after instance |
|
162 + # files deletion for those instances whose instance.host is not |
|
163 + # same as compute host where periodic task is running. |
|
164 + for inst in fake_instances: |
|
165 + if inst.host != CONF.host: |
|
166 + for mig in fake_migrations: |
|
167 + if inst.uuid == mig.instance_uuid: |
|
168 + self.assertEqual('failed', mig.status) |
|
169 + |
|
170 + def test_cleanup_incomplete_migrations_dest_node(self): |
|
171 + """Test to ensure instance files are deleted from destination node. |
|
172 + |
|
173 + If an instance gets deleted during resizing/revert-resizing |
|
174 + operation, in that case instance files gets deleted from |
|
175 + instance.host (source host here), but there is possibility that |
|
176 + instance files could be present on destination node. |
|
177 + |
|
178 + This test ensures that `_cleanup_incomplete_migration` periodic |
|
179 + task deletes orphaned instance files from destination compute node. |
|
180 + """ |
|
181 + self.flags(host='fake-mini') |
|
182 + self._test_cleanup_incomplete_migrations('fake-host') |
|
183 + |
|
184 + def test_cleanup_incomplete_migrations_source_node(self): |
|
185 + """Test to ensure instance files are deleted from source node. |
|
186 + |
|
187 + If instance gets deleted during resizing/revert-resizing operation, |
|
188 + in that case instance files gets deleted from instance.host (dest |
|
189 + host here), but there is possibility that instance files could be |
|
190 + present on source node. |
|
191 + |
|
192 + This test ensures that `_cleanup_incomplete_migration` periodic |
|
193 + task deletes orphaned instance files from source compute node. |
|
194 + """ |
|
195 + self.flags(host='fake-host') |
|
196 + self._test_cleanup_incomplete_migrations('fake-mini') |
|
197 + |
|
198 def test_attach_interface_failure(self): |
|
199 # Test that the fault methods are invoked when an attach fails |
|
200 db_instance = fake_instance.fake_db_instance() |