Skip to content

Commit

Permalink
Remove "missing_workers" from Status API
Browse files Browse the repository at this point in the history
Having "missing_workers" did not work well when worker names are unique
each time. It accumulated an endless number of worker names.

https://pulp.plan.io/issues/5786
closes #5786

(cherry picked from commit bf8794a)
  • Loading branch information
bmbouter committed Dec 3, 2019
1 parent a369ff7 commit c0fcdb7
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 59 deletions.
2 changes: 2 additions & 0 deletions CHANGES/5786.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Only online workers are shown in the ``/pulp/api/v3/status/`` causing environments where worker
names change to not accumulate workers endlessly.
2 changes: 2 additions & 0 deletions CHANGES/5786.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The ``/pulp/api/v3/status/`` had the ``missing_workers`` section removed. Also the
``online_workers`` key had the ``online`` and ``missing`` keys removed.
11 changes: 2 additions & 9 deletions pulpcore/app/serializers/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,9 @@ class StatusSerializer(serializers.Serializer):
many=True
)

missing_workers = WorkerSerializer(
help_text=_("List of missing workers known to the application. A missing worker is a "
"worker that was online, but has now stopped heartbeating and has potentially "
"died"),
many=True
)

online_content_apps = ContentAppStatusSerializer(
help_text=_("List of online content apps known to the application. An online worker is "
"actively heartbeating"),
help_text=_("List of online content apps known to the application. An online content app "
"is actively heartbeating and can serve data to clients"),
many=True
)

Expand Down
10 changes: 1 addition & 9 deletions pulpcore/app/serializers/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,10 @@ class WorkerSerializer(ModelSerializer):
help_text=_('Timestamp of the last time the worker talked to the service.'),
read_only=True
)
online = serializers.BooleanField(
help_text=_('True if the worker is considered online, otherwise False'),
read_only=True
)
missing = serializers.BooleanField(
help_text=_('True if the worker is considerd missing, otherwise False'),
read_only=True
)
# disable "created" because we don't care about it
created = None

class Meta:
model = models.Worker
_base_fields = tuple(set(ModelSerializer.Meta.fields) - set(['created']))
fields = _base_fields + ('name', 'last_heartbeat', 'online', 'missing')
fields = _base_fields + ('name', 'last_heartbeat')
6 changes: 0 additions & 6 deletions pulpcore/app/views/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ def get(self, request, format=None):
except Exception:
online_workers = None

try:
missing_workers = Worker.objects.missing_workers()
except Exception:
missing_workers = None

try:
online_content_apps = ContentAppStatus.objects.online()
except Exception:
Expand All @@ -71,7 +66,6 @@ def get(self, request, format=None):
data = {
'versions': versions,
'online_workers': online_workers,
'missing_workers': missing_workers,
'online_content_apps': online_content_apps,
'database_connection': db_status,
'redis_connection': redis_status,
Expand Down
1 change: 0 additions & 1 deletion pulpcore/tests/functional/api/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ def verify_get_response(self, status):
validate(status, STATUS)
self.assertTrue(status['database_connection']['connected'])
self.assertTrue(status['redis_connection']['connected'])
self.assertEqual(status['missing_workers'], [])
self.assertNotEqual(status['online_workers'], [])
self.assertNotEqual(status['versions'], [])
self.assertIsNotNone(status['storage'])
36 changes: 2 additions & 34 deletions pulpcore/tests/functional/api/test_workers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from random import choice

from pulp_smash import api, cli, config
from pulp_smash.pulp3.constants import STATUS_PATH, WORKER_PATH
from pulp_smash.pulp3.constants import WORKER_PATH
from requests.exceptions import HTTPError

from pulpcore.tests.functional.utils import set_up_module as setUpModule # noqa:F401
Expand Down Expand Up @@ -73,8 +73,6 @@ def test_03_positive_filters(self):
page = self.client.get(WORKER_PATH, params={
'last_heartbeat__gte': self.worker['last_heartbeat'],
'name': self.worker['name'],
'online': self.worker['online'],
'missing': self.worker['missing'],
})
self.assertEqual(
len(page['results']), 1,
Expand All @@ -92,8 +90,6 @@ def test_04_negative_filters(self):
page = self.client.get(WORKER_PATH, params={
'last_heartbeat__gte': str(datetime.now() + timedelta(days=1)),
'name': self.worker['name'],
'online': self.worker['online'],
'missing': self.worker['missing'],
})
self.assertEqual(len(page['results']), 0)

Expand Down Expand Up @@ -143,38 +139,10 @@ def test_01_start_new_worker(self):
self.assertIn('resource-worker-99', self.worker['name'])

@skip_if(bool, 'worker', False)
def test_02_stop_worker(self):
"""Stop the worker and assert it is offline."""
self.svc_mgr.stop(['pulpcore-worker@99'])
time.sleep(2)
worker = self.client.get(self.worker['pulp_href'])
self.assertEqual(worker['online'], False)

@skip_if(bool, 'worker', False)
def test_03_status_api_omits_offline_worker(self):
"""Status API doesn't show offline workers."""
online_workers = self.client.get(STATUS_PATH)['online_workers']
self.assertNotIn(
self.worker['pulp_href'],
[worker['pulp_href'] for worker in online_workers]
)

@skip_if(bool, 'worker', False)
def test_03_read_all_workers(self):
def test_02_read_all_workers(self):
"""Worker API shows all workers including offline."""
workers = self.client.get(WORKER_PATH)['results']
self.assertIn(
self.worker['pulp_href'],
[worker['pulp_href'] for worker in workers]
)

@skip_if(bool, 'worker', False)
def test_03_filter_offline_worker(self):
"""Worker API filter only offline workers."""
workers = self.client.get(
WORKER_PATH, params={'online': False}
)['results']
self.assertIn(
self.worker['pulp_href'],
[worker['pulp_href'] for worker in workers]
)

0 comments on commit c0fcdb7

Please sign in to comment.