Skip to content

Commit 451622b

Browse files
authored
fix(app): an app can now be removed from the database even if the namespace for it was already deleted (#1101)
Fixes #1100
1 parent dffcced commit 451622b

2 files changed

Lines changed: 47 additions & 11 deletions

File tree

rootfs/api/models/app.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -242,16 +242,25 @@ def delete(self, *args, **kwargs):
242242
"""Delete this application including all containers"""
243243
self.log("deleting environment")
244244
try:
245-
self._scheduler.ns.delete(self.id)
245+
# check if namespace exists
246+
self._scheduler.ns.get(self.id)
246247

247-
# wait 30 seconds for termination
248-
for _ in range(30):
249-
try:
250-
self._scheduler.ns.get(self.id)
251-
except KubeException:
252-
break
253-
except KubeException as e:
254-
raise ServiceUnavailable('Could not delete Kubernetes Namespace {}'.format(self.id)) from e # noqa
248+
try:
249+
self._scheduler.ns.delete(self.id)
250+
251+
# wait 30 seconds for termination
252+
for _ in range(30):
253+
try:
254+
self._scheduler.ns.get(self.id)
255+
except KubeHTTPException as e:
256+
# only break out on a 404
257+
if e.response.status_code == 404:
258+
break
259+
except KubeException as e:
260+
raise ServiceUnavailable('Could not delete Kubernetes Namespace {} within 30 seconds'.format(self.id)) from e # noqa
261+
except KubeHTTPException:
262+
# it's fine if the namespace does not exist - delete app from the DB
263+
pass
255264

256265
self._clean_app_logs()
257266
return super(App, self).delete(*args, **kwargs)

rootfs/api/tests/test_app.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from rest_framework.authtoken.models import Token
1818

1919
from api.models import App, Config
20-
from scheduler import KubeException
20+
from scheduler import KubeException, KubeHTTPException
2121

2222
from api.exceptions import DeisException
2323
from api.tests import adapter, mock_port, DeisTestCase
@@ -101,7 +101,7 @@ def test_app_logs(self, mock_requests, mock_get):
101101
response = self.client.get(url)
102102
self.assertEqual(response.status_code, 204, response.content)
103103

104-
# test logs - 404 from deis-logger
104+
# test logs - from deis-logger
105105
mock_response.status_code = 404
106106
response = self.client.get(url)
107107
self.assertEqual(response.status_code, 204, response.content)
@@ -405,6 +405,33 @@ def test_app_delete_failure_kubernetes_destroy(self, mock_requests):
405405
response = self.client.delete('/v2/apps/{}'.format(app_id))
406406
self.assertEqual(response.status_code, 503, response.data)
407407

408+
def test_app_delete_missing_namespace(self, mock_requests):
409+
"""
410+
Create an app and then delete but have namespace missing
411+
Should still succeed
412+
"""
413+
# create
414+
app_id = self.create_app()
415+
416+
with mock.patch('scheduler.resources.namespace.Namespace.get') as mock_kube:
417+
# instead of full request mocking, fake it out in a simple way
418+
class Response(object):
419+
def json(self):
420+
return '{}'
421+
422+
response = Response()
423+
response.status_code = 404
424+
response.reason = "Not Found"
425+
kube_exception = KubeHTTPException(response, 'big boom')
426+
mock_kube.side_effect = kube_exception
427+
428+
response = self.client.delete('/v2/apps/{}'.format(app_id))
429+
self.assertEqual(response.status_code, 204, response.data)
430+
431+
# verify that app is gone
432+
response = self.client.get('/v2/apps/{}'.format(app_id))
433+
self.assertEqual(response.status_code, 404, response.data)
434+
408435
def test_app_verify_application_health_success(self, mock_requests):
409436
"""
410437
Create an application which in turn causes a health check to run against

0 commit comments

Comments
 (0)