Browse Source

Remove unneeded call to nova API, defer other API calls

We get an instance ID directly from nova, which calls our API,
consequently we don't need to call back to nova to double check
if the instance ID realy exists.

Additionally, defer calling keystone and glance APIs to the moment
that the retrieved objects are realy needed.

Change-Id: I64a20c88229490690798aaf75ca0d96d98032467
Grzegorz Grasza 3 months ago
parent
commit
db868ea7c1
3 changed files with 31 additions and 114 deletions
  1. 27
    47
      novajoin/join.py
  2. 4
    63
      novajoin/tests/unit/api/v1/test_api.py
  3. 0
    4
      novajoin/tests/unit/fake_constants.py

+ 27
- 47
novajoin/join.py View File

@@ -24,7 +24,6 @@ from novajoin import exception
24 24
 from novajoin.glance import get_default_image_service
25 25
 from novajoin.ipa import IPAClient
26 26
 from novajoin import keystone_client
27
-from novajoin.nova import get_instance
28 27
 from novajoin import policy
29 28
 from novajoin import util
30 29
 
@@ -119,48 +118,36 @@ class JoinController(Controller):
119 118
         except exception.PolicyNotAuthorized:
120 119
             raise base.Fault(webob.exc.HTTPForbidden())
121 120
 
122
-        instance_id = body.get('instance-id')
123
-        image_id = body.get('image-id')
124
-        project_id = body.get('project-id')
125 121
         hostname_short = body.get('hostname')
126
-        metadata = body.get('metadata', {})
127
-
128
-        if not instance_id:
129
-            LOG.error('No instance-id in request')
130
-            raise base.Fault(webob.exc.HTTPBadRequest())
131
-
132 122
         if not hostname_short:
133 123
             LOG.error('No hostname in request')
134 124
             raise base.Fault(webob.exc.HTTPBadRequest())
135 125
 
136
-        if not image_id:
137
-            LOG.error('No image-id in request')
138
-            raise base.Fault(webob.exc.HTTPBadRequest())
139
-
140
-        if not project_id:
141
-            LOG.error('No project-id in request')
142
-            raise base.Fault(webob.exc.HTTPBadRequest())
143
-
144
-        enroll = metadata.get('ipa_enroll', '')
145
-
146
-        if enroll.lower() != 'true':
147
-            LOG.debug('IPA enrollment not requested in instance creation')
126
+        metadata = body.get('metadata', {})
127
+        enroll = metadata.get('ipa_enroll', '').lower() == 'true'
148 128
 
149
-        image_service = get_default_image_service()
150 129
         image_metadata = {}
151
-        try:
152
-            image = image_service.show(context, image_id)
153
-        except (exception.ImageNotFound, exception.ImageNotAuthorized) as e:
154
-            msg = 'Failed to get image: %s' % e
155
-            LOG.error(msg)
156
-            raise base.Fault(webob.exc.HTTPBadRequest(explanation=msg))
157
-        else:
158
-            image_metadata = image.get('properties', {})
130
+        if not enroll:
131
+            LOG.debug('IPA enrollment not requested in instance creation')
132
+            # Check the image metadata to see if enrollment was requested
133
+
134
+            image_id = body.get('image-id')
135
+            if not image_id:
136
+                LOG.error('No image-id in request')
137
+                raise base.Fault(webob.exc.HTTPBadRequest())
138
+            image_service = get_default_image_service()
139
+            try:
140
+                image = image_service.show(context, image_id)
141
+            except (exception.ImageNotFound,
142
+                    exception.ImageNotAuthorized) as e:
143
+                msg = 'Failed to get image: %s' % e
144
+                LOG.error(msg)
145
+                raise base.Fault(webob.exc.HTTPBadRequest(explanation=msg))
146
+            else:
147
+                image_metadata = image.get('properties', {})
159 148
 
160
-        # Check the image metadata to see if enrollment was requested
161
-        if enroll.lower() != 'true':
162
-            enroll = image_metadata.get('ipa_enroll', '')
163
-            if enroll.lower() != 'true':
149
+            enroll = image_metadata.get('ipa_enroll', '').lower() == 'true'
150
+            if not enroll:
164 151
                 LOG.debug('IPA enrollment not requested in image')
165 152
                 return {}
166 153
             else:
@@ -168,22 +155,15 @@ class JoinController(Controller):
168 155
         else:
169 156
             LOG.debug('IPA enrollment requested as property')
170 157
 
171
-        # Ensure this instance exists in nova and retrieve the
172
-        # name of the user that requested it.
173
-        instance = get_instance(instance_id)
174
-        if instance is None:
175
-            msg = 'No such instance-id, %s' % instance_id
176
-            LOG.error(msg)
177
-            raise base.Fault(webob.exc.HTTPBadRequest(explanation=msg))
178
-
179
-        # TODO(rcritten): Eventually may check the user for permission
180
-        # as well using:
181
-        # user = keystone_client.get_user_name(instance.user_id)
182
-
183 158
         hostclass = metadata.get('ipa_hostclass')
184 159
         if hostclass:
185 160
             # Only look up project_name when hostclass is requested to
186 161
             # save a round-trip with Keystone.
162
+            project_id = body.get('project-id')
163
+            if not project_id:
164
+                LOG.error('No project-id in request')
165
+                raise base.Fault(webob.exc.HTTPBadRequest())
166
+
187 167
             project_name = keystone_client.get_project_name(project_id)
188 168
             if project_name is None:
189 169
                 msg = 'No such project-id, %s' % project_id

+ 4
- 63
novajoin/tests/unit/api/v1/test_api.py View File

@@ -70,27 +70,8 @@ class JoinTest(test.TestCase):
70 70
         else:
71 71
             assert(False)
72 72
 
73
-    def test_no_instanceid(self):
74
-        body = {"metadata": {"ipa_enroll": "True"},
75
-                "image-id": fake.IMAGE_ID,
76
-                "project-id": fake.PROJECT_ID,
77
-                "hostname": "test"}
78
-        req = fakes.HTTPRequest.blank('/v1/')
79
-        req.method = 'POST'
80
-        req.content_type = "application/json"
81
-
82
-        # Not using assertRaises because the exception is wrapped as
83
-        # a Fault
84
-        try:
85
-            self.join_controller.create(req, body)
86
-        except Fault as fault:
87
-            assert fault.status_int == 400
88
-        else:
89
-            assert(False)
90
-
91 73
     def test_no_imageid(self):
92
-        body = {"metadata": {"ipa_enroll": "True"},
93
-                "instance-id": fake.INSTANCE_ID,
74
+        body = {"metadata": {"ipa_enroll": "False"},
94 75
                 "project-id": fake.PROJECT_ID,
95 76
                 "hostname": "test"}
96 77
         req = fakes.HTTPRequest.blank('/v1/')
@@ -108,7 +89,6 @@ class JoinTest(test.TestCase):
108 89
 
109 90
     def test_no_hostname(self):
110 91
         body = {"metadata": {"ipa_enroll": "True"},
111
-                "instance-id": fake.INSTANCE_ID,
112 92
                 "project-id": fake.PROJECT_ID,
113 93
                 "image-id": fake.IMAGE_ID}
114 94
         req = fakes.HTTPRequest.blank('/v1/')
@@ -125,8 +105,7 @@ class JoinTest(test.TestCase):
125 105
             assert(False)
126 106
 
127 107
     def test_no_project_id(self):
128
-        body = {"metadata": {"ipa_enroll": "True"},
129
-                "instance-id": fake.INSTANCE_ID,
108
+        body = {"metadata": {"ipa_enroll": "True", "ipa_hostclass": "foo"},
130 109
                 "image-id": fake.IMAGE_ID,
131 110
                 "hostname": "test"}
132 111
         req = fakes.HTTPRequest.blank('/v1/')
@@ -146,7 +125,6 @@ class JoinTest(test.TestCase):
146 125
     def test_request_no_enrollment(self, mock_get_image):
147 126
         mock_get_image.return_value = FakeImageService()
148 127
         body = {"metadata": {"ipa_enroll": "False"},
149
-                "instance-id": fake.INSTANCE_ID,
150 128
                 "project-id": fake.PROJECT_ID,
151 129
                 "image-id": fake.IMAGE_ID,
152 130
                 "hostname": "test"}
@@ -162,7 +140,6 @@ class JoinTest(test.TestCase):
162 140
     def test_request_invalid_image(self, mock_get_image):
163 141
         mock_get_image.side_effect = Fault(webob.exc.HTTPBadRequest())
164 142
         body = {"metadata": {"ipa_enroll": "False"},
165
-                "instance-id": fake.INSTANCE_ID,
166 143
                 "project-id": fake.PROJECT_ID,
167 144
                 "image-id": "invalid",
168 145
                 "hostname": "test"}
@@ -181,13 +158,11 @@ class JoinTest(test.TestCase):
181 158
             assert(False)
182 159
 
183 160
     @mock.patch('novajoin.ipa.SafeConfigParser')
184
-    @mock.patch('novajoin.join.get_instance')
185 161
     @mock.patch('novajoin.join.get_default_image_service')
186 162
     @mock.patch('novajoin.util.get_domain')
187 163
     def test_valid_request(self, mock_get_domain, mock_get_image,
188
-                           mock_get_instance, mock_conf_parser):
164
+                           mock_conf_parser):
189 165
         mock_get_image.return_value = FakeImageService()
190
-        mock_get_instance.return_value = fake.fake_instance
191 166
         mock_get_domain.return_value = "test"
192 167
 
193 168
         mock_conf_parser_instance = mock.MagicMock()
@@ -196,7 +171,6 @@ class JoinTest(test.TestCase):
196 171
         mock_conf_parser.return_value = mock_conf_parser_instance
197 172
 
198 173
         body = {"metadata": {"ipa_enroll": "True"},
199
-                "instance-id": fake.INSTANCE_ID,
200 174
                 "project-id": fake.PROJECT_ID,
201 175
                 "image-id": fake.IMAGE_ID,
202 176
                 "hostname": "test"}
@@ -221,15 +195,12 @@ class JoinTest(test.TestCase):
221 195
 
222 196
     @mock.patch('novajoin.ipa.SafeConfigParser')
223 197
     @mock.patch('novajoin.keystone_client.get_project_name')
224
-    @mock.patch('novajoin.join.get_instance')
225 198
     @mock.patch('novajoin.join.get_default_image_service')
226 199
     @mock.patch('novajoin.util.get_domain')
227 200
     def test_valid_hostclass_request(self, mock_get_domain, mock_get_image,
228
-                                     mock_get_instance,
229 201
                                      mock_get_project_name,
230 202
                                      mock_conf_parser):
231 203
         mock_get_image.return_value = FakeImageService()
232
-        mock_get_instance.return_value = fake.fake_instance
233 204
         mock_get_domain.return_value = "test"
234 205
         mock_get_project_name.return_value = "test"
235 206
 
@@ -239,7 +210,6 @@ class JoinTest(test.TestCase):
239 210
         mock_conf_parser.return_value = mock_conf_parser_instance
240 211
 
241 212
         body = {"metadata": {"ipa_enroll": "True"},
242
-                "instance-id": fake.INSTANCE_ID,
243 213
                 "project-id": fake.PROJECT_ID,
244 214
                 "image-id": fake.IMAGE_ID,
245 215
                 "hostname": "test"}
@@ -262,45 +232,16 @@ class JoinTest(test.TestCase):
262 232
         # because in all likelihood the keytab cannot be read (and
263 233
         # probably doesn't exist. This can be ignored.
264 234
 
265
-    @mock.patch('novajoin.join.get_instance')
266
-    @mock.patch('novajoin.join.get_default_image_service')
267
-    def test_invalid_instance_id(self, mock_get_image, mock_get_instance):
268
-        """Mock the instance to not exist so there is nothing to enroll."""
269
-        mock_get_image.return_value = FakeImageService()
270
-        mock_get_instance.return_value = None
271
-
272
-        body = {"metadata": {"ipa_enroll": "True"},
273
-                "instance-id": "invalid",
274
-                "project-id": fake.PROJECT_ID,
275
-                "image-id": fake.IMAGE_ID,
276
-                "hostname": "test"}
277
-        req = fakes.HTTPRequest.blank('/v1')
278
-        req.method = 'POST'
279
-        req.content_type = "application/json"
280
-        req.body = jsonutils.dump_as_bytes(body)
281
-
282
-        # Not using assertRaises because the exception is wrapped as
283
-        # a Fault
284
-        try:
285
-            self.join_controller.create(req, body)
286
-        except Fault as fault:
287
-            assert fault.status_int == 400
288
-        else:
289
-            assert(False)
290
-
291
-    @mock.patch('novajoin.join.get_instance')
292 235
     @mock.patch('novajoin.join.get_default_image_service')
293 236
     @mock.patch('novajoin.keystone_client.get_project_name')
294 237
     @mock.patch('novajoin.util.get_domain')
295 238
     def test_invalid_project_id(self, mock_get_domain, mock_get_project_name,
296
-                                mock_get_image, mock_get_instance):
239
+                                mock_get_image):
297 240
         mock_get_image.return_value = FakeImageService()
298
-        mock_get_instance.return_value = None
299 241
         mock_get_project_name.return_value = None
300 242
         mock_get_domain.return_value = "test"
301 243
 
302 244
         body = {"metadata": {"ipa_enroll": "True", "ipa_hostclass": "foo"},
303
-                "instance-id": fake.INSTANCE_ID,
304 245
                 "project-id": "invalid",
305 246
                 "image-id": fake.IMAGE_ID,
306 247
                 "hostname": "test"}

+ 0
- 4
novajoin/tests/unit/fake_constants.py View File

@@ -14,8 +14,4 @@
14 14
 
15 15
 PROJECT_ID = '89afd400-b646-4bbc-b12b-c0a4d63e5bd3'
16 16
 USER_ID = 'c853ca26-e8ea-4797-8a52-ee124a013d0e'
17
-INSTANCE_ID = 'e4274dc8-325a-409b-92fd-cfdfdd65ae8b'
18 17
 IMAGE_ID = 'b8c88e01-c820-40f6-b026-00926706e374'
19
-
20
-# In reality this should be a Server object.
21
-fake_instance = {'instance_name': 'test', 'id': INSTANCE_ID}

Loading…
Cancel
Save