mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-09 03:36:48 +00:00
feat(operations): a job's error and warning show in the reader's language
A job stored its error and its result's warning as English text, much of it built with the values in it (volume paths, a disk, libvirt's error), so no catalog could translate it, and the job pages and panel showed English whatever the language. A job now stores them as messages: our English text, marked for translation, and its values; libvirt's own text is a message without values and is shown as it is. The jobs API and pages put them in the reader's language. OperationError keeps its text and values apart for the same reason, so a refusal raised while a job is submitted is shown translated too. Messages one after another still read as sentences. Errors stored before are kept as one message each, shown as they were.
This commit is contained in:
parent
0eab09e19d
commit
1e66367bb4
29 changed files with 927 additions and 333 deletions
|
|
@ -6,12 +6,14 @@ from rest_framework import serializers
|
|||
|
||||
from operations.jobs import KINDS, label_of
|
||||
from operations.models import Operation
|
||||
from operations.utils import shown_label
|
||||
from operations.utils import shown_label, words
|
||||
|
||||
|
||||
class OperationSerializer(serializers.ModelSerializer):
|
||||
label = serializers.SerializerMethodField()
|
||||
phase = serializers.SerializerMethodField()
|
||||
error = serializers.SerializerMethodField()
|
||||
result = serializers.SerializerMethodField()
|
||||
resource_label = serializers.SerializerMethodField()
|
||||
cancellable = serializers.SerializerMethodField()
|
||||
in_request = serializers.SerializerMethodField()
|
||||
|
|
@ -35,6 +37,15 @@ class OperationSerializer(serializers.ModelSerializer):
|
|||
return (_("%(phase)s (over its time limit)") % {"phase": phase}).strip()
|
||||
return phase
|
||||
|
||||
def get_error(self, op) -> str:
|
||||
return words(op.error)
|
||||
|
||||
def get_result(self, op) -> dict:
|
||||
"""What the job made; its warning in the viewer's language."""
|
||||
if op.result.get("warning"):
|
||||
return {**op.result, "warning": words(op.result["warning"])}
|
||||
return op.result
|
||||
|
||||
def get_resource_label(self, op) -> str:
|
||||
return shown_label(op, self.context["request"].user)
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ from django.db import transaction
|
|||
from django.db.models import F, Value
|
||||
from django.db.models.functions import Coalesce
|
||||
from django.utils import timezone
|
||||
from django.utils.translation import gettext_noop
|
||||
|
||||
from logs.audit import write
|
||||
from vrtManager.util import OperationError
|
||||
|
|
@ -44,7 +45,7 @@ class Outcome:
|
|||
|
||||
state: str
|
||||
result: dict = field(default_factory=dict)
|
||||
error: str = ""
|
||||
error: list = field(default_factory=list) # messages (utils.say)
|
||||
phase: str = ""
|
||||
stalled: bool = False
|
||||
progress: tuple | None = None
|
||||
|
|
@ -153,7 +154,7 @@ def _existing(user, key, kind_name, params):
|
|||
|
||||
def submit(kind_name, user, raw_params, submission_key):
|
||||
if not KEY_RE.fullmatch(submission_key or ""):
|
||||
raise OperationError("The form has no valid submission key; reload the page")
|
||||
raise OperationError(gettext_noop("The form has no valid submission key; reload the page"))
|
||||
kind = get_kind(kind_name)
|
||||
params = kind.normalize(raw_params)
|
||||
utils.check_params(params)
|
||||
|
|
@ -199,10 +200,10 @@ def cancel(op_id, user):
|
|||
_log(op.username, *entry, None)
|
||||
elif op.state == Operation.RUNNING:
|
||||
if not get_kind(op.kind).cancellable:
|
||||
raise OperationError("This job cannot be cancelled once it has started")
|
||||
raise OperationError(gettext_noop("This job cannot be cancelled once it has started"))
|
||||
Operation.objects.filter(pk=op.pk).update(state=Operation.CANCELLING, cancel_requested=True)
|
||||
elif op.state != Operation.CANCELLING:
|
||||
raise OperationError("The job has already ended")
|
||||
raise OperationError(gettext_noop("The job has already ended"))
|
||||
op.refresh_from_db()
|
||||
if request_job: # no worker signals it: the request stops it
|
||||
get_kind(op.kind).cancel(op)
|
||||
|
|
@ -226,14 +227,13 @@ def _log(user, host, instance, uuid, message, success):
|
|||
def finish(op, outcome, *, worker=None):
|
||||
"""Terminal state, claims and history together, once. With a worker,
|
||||
only while it holds the lease and owns the job. Uncertain keeps the claims."""
|
||||
error = utils.safe_text(outcome.error)
|
||||
with fenced(worker.identity) if worker is not None else transaction.atomic():
|
||||
q = Operation.objects.filter(pk=op.pk, state__in=Operation.ACTIVE)
|
||||
if worker is not None:
|
||||
q = q.filter(worker=worker.identity)
|
||||
# success clears the phase and completes the progress; another end keeps the last ones
|
||||
done = outcome.state == Operation.SUCCEEDED
|
||||
if not q.update(state=outcome.state, result=outcome.result, error=error,
|
||||
if not q.update(state=outcome.state, result=outcome.result, error=outcome.error,
|
||||
phase=Value(outcome.phase[:255]) if outcome.phase else ("" if done else F("phase")),
|
||||
progress_done=Coalesce(F("progress_total"), F("progress_done")) if done else F("progress_done"),
|
||||
finished_at=timezone.now()):
|
||||
|
|
@ -251,14 +251,14 @@ def resolve(op_id, user, outcome_state, note):
|
|||
if not user.is_superuser:
|
||||
raise PermissionDenied
|
||||
if outcome_state not in (Operation.SUCCEEDED, Operation.FAILED):
|
||||
raise OperationError("Choose succeeded or failed")
|
||||
raise OperationError(gettext_noop("Choose succeeded or failed"))
|
||||
note = note.strip()
|
||||
if not note:
|
||||
raise OperationError("Write a note on what was checked")
|
||||
raise OperationError(gettext_noop("Write a note on what was checked"))
|
||||
with transaction.atomic():
|
||||
op = Operation.objects.select_for_update().get(pk=op_id)
|
||||
if op.state != Operation.UNCERTAIN:
|
||||
raise OperationError("Only an uncertain job can be resolved")
|
||||
raise OperationError(gettext_noop("Only an uncertain job can be resolved"))
|
||||
Operation.objects.filter(pk=op.pk).update(state=outcome_state, resolution=outcome_state,
|
||||
resolution_note=note, resolved_by=user, resolved_at=timezone.now())
|
||||
Claim.objects.filter(operation=op).delete()
|
||||
|
|
|
|||
27
operations/migrations/0002_error_messages.py
Normal file
27
operations/migrations/0002_error_messages.py
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
"""A job's error and its result's warning are lists of messages
|
||||
(operations.utils.say), so that they are shown in the language of whoever
|
||||
reads them. Those stored before are one message each, without values: shown
|
||||
as they were."""
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
def to_messages(apps, schema_editor):
|
||||
Operation = apps.get_model("operations", "Operation")
|
||||
for op in Operation.objects.exclude(error_text="").only("pk", "error_text"):
|
||||
Operation.objects.filter(pk=op.pk).update(error=[{"text": op.error_text, "values": {}}])
|
||||
for op in Operation.objects.only("pk", "result"):
|
||||
if isinstance(op.result.get("warning"), str):
|
||||
Operation.objects.filter(pk=op.pk).update(
|
||||
result={**op.result, "warning": [{"text": op.result["warning"], "values": {}}]})
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
dependencies = [("operations", "0001_initial")]
|
||||
|
||||
operations = [
|
||||
migrations.RenameField("operation", "error", "error_text"),
|
||||
migrations.AddField("operation", "error", models.JSONField(default=list)),
|
||||
migrations.RunPython(to_messages, migrations.RunPython.noop),
|
||||
migrations.RemoveField("operation", "error_text"),
|
||||
]
|
||||
|
|
@ -45,7 +45,7 @@ class Operation(models.Model):
|
|||
progress_done = models.BigIntegerField(null=True)
|
||||
progress_total = models.BigIntegerField(null=True)
|
||||
result = models.JSONField(default=dict)
|
||||
error = models.TextField(blank=True)
|
||||
error = models.JSONField(default=list) # messages (operations.utils.say), shown in the reader's language
|
||||
created_at = models.DateTimeField(auto_now_add=True, db_index=True)
|
||||
started_at = models.DateTimeField(null=True)
|
||||
finished_at = models.DateTimeField(null=True)
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@
|
|||
</div>
|
||||
<div class="small text-body-secondary mt-1">{% trans "User" %}: {{ job.username }}</div>
|
||||
</div>
|
||||
{% if job.phase or op.progress_total or job.cancellable or op.error or job.result.links or job.result.warning or job.result.left_behind or job.resolution %}
|
||||
{% if job.phase or op.progress_total or job.cancellable or job.error or job.result.links or job.result.warning or job.result.left_behind or job.resolution %}
|
||||
<div class="card-body pb-0">
|
||||
{% if job.phase or job.cancellable %}
|
||||
<div class="d-flex flex-wrap align-items-center gap-2 mb-3">
|
||||
|
|
@ -45,9 +45,9 @@
|
|||
<div class="progress-bar" style="width: {{ percent }}%"></div>
|
||||
</div>
|
||||
{% endif %}
|
||||
{% if op.error %}
|
||||
{% if job.error %}
|
||||
<div class="alert alert-{{ op.state|state_color }}" role="alert">
|
||||
{% bs_icon 'exclamation-triangle' %} {{ op.error }}
|
||||
{% bs_icon 'exclamation-triangle' %} {{ job.error }}
|
||||
</div>
|
||||
{% endif %}
|
||||
{% if job.result.links %}
|
||||
|
|
|
|||
|
|
@ -84,7 +84,7 @@
|
|||
</div>
|
||||
{% endif %}
|
||||
{% elif op.state == "failed" or op.state == "uncertain" %}
|
||||
<div class="small text-body-secondary" title="{{ op.error }}">{{ op.error|truncatechars:80 }}</div>
|
||||
<div class="small text-body-secondary" title="{{ row.error }}">{{ row.error|truncatechars:80 }}</div>
|
||||
{% endif %}
|
||||
<div class="small text-body-secondary">{{ op.created_at|date:"Y-m-d H:i:s T" }}{% if request.user.is_superuser %}<span class="d-md-none"> · {{ row.username }}</span>{% endif %}</div>
|
||||
{# below md the hidden columns show here #}
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ from instances.models import Instance
|
|||
from operations import claims, jobs
|
||||
from operations.models import Claim, Operation
|
||||
from operations.tests import fakes
|
||||
from operations.utils import say
|
||||
|
||||
|
||||
class OperationsApiTestCase(TransactionTestCase):
|
||||
|
|
@ -35,6 +36,17 @@ class OperationsApiTestCase(TransactionTestCase):
|
|||
Operation.objects.filter(pk=self.op.pk).update(time_limit=60, started_at=timezone.now() - timedelta(seconds=61))
|
||||
self.assertEqual(self.client.get(url).json()["phase"], "Merging the snapshot (over its time limit)")
|
||||
|
||||
def test_the_error_and_warning_come_in_the_users_language_with_their_values(self):
|
||||
# ours with its values; libvirt's text as it is
|
||||
Operation.objects.filter(pk=self.op.pk).update(
|
||||
state=Operation.FAILED,
|
||||
error=say("The pool has no volume named %(name)s", name="disk.qcow2") + say("unable to open: busy"),
|
||||
result={"warning": say("The copy was not made")})
|
||||
self.client.force_login(self.owner)
|
||||
job = self.client.get(f"/api/v1/operations/{self.op.pk}/", HTTP_ACCEPT_LANGUAGE="tr").json()
|
||||
self.assertEqual(job["error"], "Havuzda disk.qcow2 adında birim yok. unable to open: busy")
|
||||
self.assertEqual(job["result"]["warning"], "Kopya oluşturulmadı")
|
||||
|
||||
def test_users_list_only_their_own_jobs(self):
|
||||
self.client.force_login(self.other)
|
||||
self.assertEqual(self.client.get("/api/v1/operations/").json()["count"], 0)
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ from unittest.mock import patch
|
|||
from django.contrib.auth import get_user_model
|
||||
from django.test import TransactionTestCase
|
||||
|
||||
from operations.utils import say, words
|
||||
from operations import jobs
|
||||
from operations.jobs import Outcome
|
||||
from operations.models import Claim, Operation, WorkerLease, acquire
|
||||
|
|
@ -55,20 +56,20 @@ class RecoveryTestCase(TransactionTestCase):
|
|||
|
||||
def test_a_proven_success_does_not_carry_the_raised_error(self):
|
||||
op = self.orphan({}, worker=self.worker.identity)
|
||||
self.scheduler.inspect_now(op.pk, error="failed after effects")
|
||||
self.scheduler.inspect_now(op.pk, error=say("failed after effects"))
|
||||
with patch.object(fakes.ScriptedKind, "reconcile", return_value=Outcome(Operation.SUCCEEDED)):
|
||||
self.run_due()
|
||||
op = self.reload(op)
|
||||
self.assertEqual((op.state, op.error), (Operation.SUCCEEDED, ""))
|
||||
self.assertEqual((op.state, words(op.error)), (Operation.SUCCEEDED, ""))
|
||||
|
||||
def test_an_uncertain_outcome_keeps_the_error_the_job_raised(self):
|
||||
op = self.orphan({}, worker=self.worker.identity)
|
||||
self.scheduler.inspect_now(op.pk, error="resource busy: X")
|
||||
self.scheduler.inspect_now(op.pk, error=say("resource busy: X"))
|
||||
with patch.object(fakes.ScriptedKind, "reconcile",
|
||||
return_value=Outcome(Operation.UNCERTAIN, error="check the VM")):
|
||||
return_value=Outcome(Operation.UNCERTAIN, error=say("check the VM"))):
|
||||
self.run_due()
|
||||
op = self.reload(op)
|
||||
self.assertEqual((op.state, op.error), (Operation.UNCERTAIN, "resource busy: X. check the VM"))
|
||||
self.assertEqual((op.state, words(op.error)), (Operation.UNCERTAIN, "resource busy: X. check the VM"))
|
||||
|
||||
def test_uncertain_keeps_the_claims(self):
|
||||
op = self.orphan({"outcome": "uncertain"})
|
||||
|
|
@ -116,31 +117,31 @@ class RecoveryTestCase(TransactionTestCase):
|
|||
self.run_at(1000, Outcome(Operation.RUNNING, stalled=True))
|
||||
self.run_at(1000 + STALL_LIMIT, Outcome(Operation.RUNNING, stalled=True))
|
||||
op = self.reload(op)
|
||||
self.assertEqual((op.state, op.error), (Operation.UNCERTAIN, STALLED))
|
||||
self.assertEqual((op.state, words(op.error)), (Operation.UNCERTAIN, STALLED))
|
||||
self.assertTrue(Claim.objects.filter(operation=op).exists())
|
||||
self.assertNotIn(op.pk, self.scheduler.stalled_since)
|
||||
|
||||
def test_a_stalled_merge_keeps_the_error_the_job_raised(self):
|
||||
op = self.orphan({}, worker=self.worker.identity)
|
||||
self.scheduler.inspect_now(op.pk, error="resource busy: X")
|
||||
self.scheduler.inspect_now(op.pk, error=say("resource busy: X"))
|
||||
self.run_at(1000, Outcome(Operation.RUNNING, stalled=True))
|
||||
self.scheduler.next_try[op.pk] = 0
|
||||
self.run_at(1000 + STALL_LIMIT, Outcome(Operation.RUNNING, stalled=True))
|
||||
self.assertEqual(self.reload(op).error, f"resource busy: X. {STALLED}")
|
||||
self.assertEqual(words(self.reload(op).error), f"resource busy: X. {STALLED}")
|
||||
|
||||
def test_a_raised_error_ending_with_a_dot_is_not_doubled(self):
|
||||
op = self.orphan({}, worker=self.worker.identity)
|
||||
self.scheduler.inspect_now(op.pk, error="refused.")
|
||||
with patch.object(fakes.ScriptedKind, "reconcile", return_value=Outcome(Operation.UNCERTAIN, error="check")):
|
||||
self.scheduler.inspect_now(op.pk, error=say("refused."))
|
||||
with patch.object(fakes.ScriptedKind, "reconcile", return_value=Outcome(Operation.UNCERTAIN, error=say("check"))):
|
||||
self.run_due()
|
||||
self.assertEqual(self.reload(op).error, "refused. check")
|
||||
self.assertEqual(words(self.reload(op).error), "refused. check")
|
||||
|
||||
def test_an_outcome_without_an_error_stores_the_raised_one(self):
|
||||
op = self.orphan({}, worker=self.worker.identity)
|
||||
self.scheduler.inspect_now(op.pk, error="resource busy: X")
|
||||
self.scheduler.inspect_now(op.pk, error=say("resource busy: X"))
|
||||
with patch.object(fakes.ScriptedKind, "reconcile", return_value=Outcome(Operation.FAILED)):
|
||||
self.run_due()
|
||||
self.assertEqual(self.reload(op).error, "resource busy: X")
|
||||
self.assertEqual(words(self.reload(op).error), "resource busy: X")
|
||||
|
||||
def test_progress_resets_the_stall_timer(self):
|
||||
op = self.orphan({})
|
||||
|
|
@ -171,8 +172,8 @@ class RecoveryTestCase(TransactionTestCase):
|
|||
reconcile.assert_not_called()
|
||||
op = self.reload(op)
|
||||
# the stored error is shown to the job's owner: the host goes to the worker log only
|
||||
self.assertEqual((op.state, op.error), (Operation.UNCERTAIN, ABANDONED))
|
||||
self.assertNotIn("other-host", op.error)
|
||||
self.assertEqual((op.state, words(op.error)), (Operation.UNCERTAIN, ABANDONED))
|
||||
self.assertNotIn("other-host", words(op.error))
|
||||
self.assertIn("other-host", "\n".join(logs.output))
|
||||
self.assertTrue(Claim.objects.filter(operation=op).exists())
|
||||
|
||||
|
|
@ -196,7 +197,7 @@ class RecoveryTestCase(TransactionTestCase):
|
|||
|
||||
def test_an_own_job_that_raised_before_its_effects_fails_and_releases(self):
|
||||
op = self.orphan({"outcome": "uncertain"}, worker=self.worker.identity, effects=False)
|
||||
self.scheduler.inspect_now(op.pk, error="refused before effects")
|
||||
self.scheduler.inspect_now(op.pk, error=say("refused before effects"))
|
||||
self.run_due()
|
||||
op = self.reload(op)
|
||||
self.assertEqual(op.state, Operation.FAILED)
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ from django.utils import timezone
|
|||
from vrtManager.util import OperationError
|
||||
|
||||
from logs.models import Logs
|
||||
from operations.utils import say, words
|
||||
from operations import claims, jobs
|
||||
from operations.jobs import Outcome
|
||||
from operations.models import Claim, LeaseLost, Operation, WorkerLease, acquire
|
||||
|
|
@ -153,7 +154,7 @@ class CancelFinishResolveTestCase(TransactionTestCase):
|
|||
def test_a_failure_keeps_the_phase_stored_after_the_job_was_loaded(self):
|
||||
op = jobs.submit("test.noop", self.user, {}, KEY)
|
||||
Operation.objects.filter(pk=op.pk).update(phase="merging") # op in memory still has ""
|
||||
jobs.finish(op, Outcome(Operation.FAILED, error="x"))
|
||||
jobs.finish(op, Outcome(Operation.FAILED, error=say("x")))
|
||||
op.refresh_from_db()
|
||||
self.assertEqual(op.phase, "merging")
|
||||
|
||||
|
|
@ -161,14 +162,14 @@ class CancelFinishResolveTestCase(TransactionTestCase):
|
|||
op = jobs.submit("test.noop", self.user, {}, KEY)
|
||||
Operation.objects.filter(pk=op.pk).update(phase="merging", progress_done=5, progress_total=10)
|
||||
op.refresh_from_db()
|
||||
jobs.finish(op, Outcome(Operation.FAILED, error="x"))
|
||||
jobs.finish(op, Outcome(Operation.FAILED, error=say("x")))
|
||||
op.refresh_from_db()
|
||||
self.assertEqual((op.phase, op.progress_done), ("merging", 5))
|
||||
|
||||
def test_finish_happens_once(self):
|
||||
op = jobs.submit("test.noop", self.user, {}, KEY)
|
||||
self.assertTrue(jobs.finish(op, Outcome(Operation.SUCCEEDED, result={"x": 1})))
|
||||
self.assertFalse(jobs.finish(op, Outcome(Operation.FAILED, error="late")))
|
||||
self.assertFalse(jobs.finish(op, Outcome(Operation.FAILED, error=say("late"))))
|
||||
op.refresh_from_db()
|
||||
self.assertEqual((op.state, op.result), (Operation.SUCCEEDED, {"x": 1}))
|
||||
self.assertEqual(Logs.objects.count(), 1)
|
||||
|
|
@ -187,14 +188,14 @@ class CancelFinishResolveTestCase(TransactionTestCase):
|
|||
op = jobs.submit("test.noop", self.user, {}, KEY)
|
||||
Operation.objects.filter(pk=op.pk).update(username="u" * 150)
|
||||
op.refresh_from_db()
|
||||
jobs.finish(op, Outcome(Operation.FAILED, error="qemu+tcp://admin:s3cret@192.0.2.5/system failed"))
|
||||
jobs.finish(op, Outcome(Operation.FAILED, error=say("qemu+tcp://admin:s3cret@192.0.2.5/system failed")))
|
||||
op.refresh_from_db()
|
||||
self.assertNotIn("s3cret", op.error)
|
||||
self.assertNotIn("s3cret", words(op.error))
|
||||
self.assertLessEqual(len(Logs.objects.get().user), 150)
|
||||
|
||||
def test_an_uncertain_job_keeps_its_claims_until_resolved(self):
|
||||
op = jobs.submit("test.noop", self.user, {}, KEY)
|
||||
jobs.finish(op, Outcome(Operation.UNCERTAIN, error="check the disks"))
|
||||
jobs.finish(op, Outcome(Operation.UNCERTAIN, error=say("check the disks")))
|
||||
self.assertTrue(Claim.objects.filter(operation=op).exists())
|
||||
with self.assertRaises(PermissionDenied):
|
||||
jobs.resolve(op.pk, self.user, Operation.FAILED, "checked")
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from operations.jobs import Outcome
|
|||
from operations.models import Claim, Operation, WorkerLease
|
||||
from operations.templatetags.operations import STATE_COLORS, STATE_LABELS, state_color, state_label
|
||||
from operations.tests import fakes
|
||||
from operations.utils import say
|
||||
|
||||
|
||||
class BusyMessageTestCase(SimpleTestCase):
|
||||
|
|
@ -74,7 +75,7 @@ class JobsPageTestCase(TransactionTestCase):
|
|||
self.assertNotContains(self.client.get(reverse("operations:jobs"), {"username": "other"}), link)
|
||||
|
||||
def test_resolve_needs_a_superuser_an_outcome_and_a_note(self):
|
||||
jobs.finish(self.op, Outcome(Operation.UNCERTAIN, error="check"))
|
||||
jobs.finish(self.op, Outcome(Operation.UNCERTAIN, error=say("check")))
|
||||
self.client.force_login(self.user)
|
||||
self.assertEqual(self.client.post(reverse("operations:resolve", args=[self.op.pk]),
|
||||
{"outcome": "failed", "note": "x"}).status_code, 403)
|
||||
|
|
@ -85,7 +86,7 @@ class JobsPageTestCase(TransactionTestCase):
|
|||
self.assertFalse(Claim.objects.filter(operation=self.op).exists())
|
||||
|
||||
def test_the_detail_shows_what_recovery_saw_escaped(self):
|
||||
Operation.objects.filter(pk=self.op.pk).update(error="<script>x</script>", phase="Waiting for recovery",
|
||||
Operation.objects.filter(pk=self.op.pk).update(error=say("<script>x</script>"), phase="Waiting for recovery",
|
||||
state=Operation.UNCERTAIN)
|
||||
self.client.force_login(self.user)
|
||||
response = self.client.get(reverse("operations:job", args=[self.op.pk]))
|
||||
|
|
@ -93,7 +94,7 @@ class JobsPageTestCase(TransactionTestCase):
|
|||
self.assertContains(response, "Waiting for recovery")
|
||||
|
||||
def test_the_detail_shows_the_results_warning_escaped(self):
|
||||
Operation.objects.filter(pk=self.op.pk).update(result={"warning": "not <b>thawed</b>"},
|
||||
Operation.objects.filter(pk=self.op.pk).update(result={"warning": say("not <b>thawed</b>")},
|
||||
state=Operation.SUCCEEDED)
|
||||
self.client.force_login(self.user)
|
||||
response = self.client.get(reverse("operations:job", args=[self.op.pk]))
|
||||
|
|
@ -169,7 +170,7 @@ class JobsPageTestCase(TransactionTestCase):
|
|||
self.assertNotContains(self.client.get(url), 'id="follow-active-jobs"')
|
||||
|
||||
def test_list_and_detail_show_a_state_badge(self):
|
||||
Operation.objects.filter(pk=self.op.pk).update(state=Operation.FAILED, error="disk busy")
|
||||
Operation.objects.filter(pk=self.op.pk).update(state=Operation.FAILED, error=say("disk busy"))
|
||||
self.client.force_login(self.admin)
|
||||
for response in (self.client.get(reverse("operations:jobs")),
|
||||
self.client.get(reverse("operations:job", args=[self.op.pk]))):
|
||||
|
|
@ -178,12 +179,12 @@ class JobsPageTestCase(TransactionTestCase):
|
|||
self.assertContains(detail, 'class="alert alert-danger"')
|
||||
|
||||
def test_the_list_shows_the_phase_while_active_and_the_error_after_a_failure(self):
|
||||
Operation.objects.filter(pk=self.op.pk).update(state=Operation.RUNNING, phase="Copying", error="")
|
||||
Operation.objects.filter(pk=self.op.pk).update(state=Operation.RUNNING, phase="Copying", error=say(""))
|
||||
self.client.force_login(self.admin)
|
||||
self.assertContains(self.client.get(reverse("operations:jobs")), "Copying")
|
||||
Operation.objects.filter(pk=self.op.pk).update(state=Operation.SUCCEEDED)
|
||||
self.assertNotContains(self.client.get(reverse("operations:jobs")), "Copying")
|
||||
Operation.objects.filter(pk=self.op.pk).update(state=Operation.FAILED, error="x" * 200)
|
||||
Operation.objects.filter(pk=self.op.pk).update(state=Operation.FAILED, error=say("x" * 200))
|
||||
response = self.client.get(reverse("operations:jobs"))
|
||||
self.assertNotContains(response, "Copying")
|
||||
self.assertContains(response, "x" * 79 + "\u2026")
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ from django.utils import timezone
|
|||
|
||||
from appsettings.models import AppSettings
|
||||
from appsettings.settings import app_settings
|
||||
from operations.utils import words
|
||||
from operations import jobs
|
||||
from operations.models import Claim, Operation, WorkerLease
|
||||
from operations.tests import fakes
|
||||
|
|
@ -77,7 +78,7 @@ class WorkerTestCase(TransactionTestCase):
|
|||
self.start()
|
||||
self.assertTrue(wait_for(lambda: state(op) in Operation.TERMINAL))
|
||||
op.refresh_from_db()
|
||||
self.assertEqual((op.state, op.result), (Operation.SUCCEEDED, {"value": "42"}), op.error)
|
||||
self.assertEqual((op.state, op.result), (Operation.SUCCEEDED, {"value": "42"}), words(op.error))
|
||||
|
||||
def test_an_error_before_effects_fails_the_job(self):
|
||||
op = self.submit("test.scripted", {"fail_before": True}, "key-00000001")
|
||||
|
|
@ -90,7 +91,7 @@ class WorkerTestCase(TransactionTestCase):
|
|||
with patch.object(fakes.NoopKind, "precheck", side_effect=failure):
|
||||
self.start()
|
||||
self.assertTrue(wait_for(lambda: state(op) == Operation.FAILED))
|
||||
error = Operation.objects.get(pk=op.pk).error
|
||||
error = words(Operation.objects.get(pk=op.pk).error)
|
||||
self.assertEqual(error, "The host of this virtual machine cannot be reached right now.")
|
||||
self.assertNotIn("example.com", error)
|
||||
|
||||
|
|
@ -221,7 +222,7 @@ class WorkerTestCase(TransactionTestCase):
|
|||
with patch("operations.jobs.Context", side_effect=RuntimeError("no context")):
|
||||
self.start()
|
||||
self.assertTrue(wait_for(lambda: state(op) == Operation.FAILED))
|
||||
self.assertIn("no context", Operation.objects.get(pk=op.pk).error)
|
||||
self.assertIn("no context", words(Operation.objects.get(pk=op.pk).error))
|
||||
self.assertFalse(Claim.objects.filter(operation=op).exists())
|
||||
|
||||
def test_a_failing_manager_loop_ends_the_process(self):
|
||||
|
|
|
|||
|
|
@ -13,13 +13,13 @@ from django.utils import timezone
|
|||
from django.utils.timesince import timesince
|
||||
from django.utils.translation import gettext as _
|
||||
from django.utils.translation import pgettext
|
||||
from django.utils.translation import gettext_lazy
|
||||
from django.utils.translation import gettext_noop
|
||||
from libvirt import libvirtError
|
||||
|
||||
from accounts.models import UserInstance
|
||||
from appsettings.models import AppSettings
|
||||
from instances.models import Instance
|
||||
from vrtManager.util import ConnectionFailed
|
||||
from vrtManager.util import ConnectionFailed, OperationError
|
||||
|
||||
from . import jobs
|
||||
|
||||
|
|
@ -49,7 +49,7 @@ TRANSPORT_ERROR_CODES = {
|
|||
libvirt.VIR_ERR_SSH,
|
||||
libvirt.VIR_ERR_LIBSSH,
|
||||
}
|
||||
UNREACHABLE = gettext_lazy("The host of this virtual machine cannot be reached right now.")
|
||||
UNREACHABLE = gettext_noop("The host of this virtual machine cannot be reached right now.")
|
||||
|
||||
BOUNDS = {"WORKER_CONCURRENCY": (1, 16, 4), "JOB_RETENTION_DAYS": (1, 3650, 30)}
|
||||
|
||||
|
|
@ -61,7 +61,29 @@ def safe_text(text):
|
|||
return text
|
||||
|
||||
|
||||
def public_error_text(exception):
|
||||
# A text a job keeps for later (its error, its result's warning) is a list of
|
||||
# messages, each our English text, marked with gettext_noop, and its values:
|
||||
# words() puts them in the language of whoever reads them. A text that is not
|
||||
# ours (libvirt's) is a message without values and stays as it is.
|
||||
def say(text, **values):
|
||||
return [{"text": safe_text(text), "values": {key: safe_text(value) for key, value in values.items()}}]
|
||||
|
||||
|
||||
def said(exception):
|
||||
"""The messages of an exception: an OperationError's text and values, else its text."""
|
||||
if isinstance(exception, OperationError):
|
||||
return say(exception.text, **exception.values)
|
||||
return say(exception)
|
||||
|
||||
|
||||
def words(messages):
|
||||
"""messages in the active language, each a sentence of its own."""
|
||||
parts = [_(m["text"]) % m["values"] if m["values"] else _(m["text"]) for m in messages]
|
||||
return " ".join(part if last or part.endswith((".", "。", "!", "?")) else part + "."
|
||||
for last, part in ((i == len(parts) - 1, part) for i, part in enumerate(parts)))
|
||||
|
||||
|
||||
def public_error(exception):
|
||||
"""The error for users who do not see the hosts: a connection error names
|
||||
the host and how it is reached, so they learn only that it cannot be reached."""
|
||||
connection_error = isinstance(exception, ConnectionFailed) or (
|
||||
|
|
@ -69,7 +91,11 @@ def public_error_text(exception):
|
|||
and exception.get_error_domain() in TRANSPORT_ERROR_DOMAINS
|
||||
and exception.get_error_code() in TRANSPORT_ERROR_CODES
|
||||
)
|
||||
return str(UNREACHABLE) if connection_error else str(exception)
|
||||
return say(UNREACHABLE) if connection_error else said(exception)
|
||||
|
||||
|
||||
def public_error_text(exception):
|
||||
return words(public_error(exception))
|
||||
|
||||
|
||||
def check_params(params):
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from vrtManager.util import OperationError
|
|||
from . import jobs as jobs_module
|
||||
from .api.serializers import OperationSerializer
|
||||
from .models import Operation, last_seen
|
||||
from .utils import said, words
|
||||
|
||||
|
||||
def _visible(user):
|
||||
|
|
@ -71,7 +72,7 @@ def cancel(request, pk):
|
|||
try:
|
||||
jobs_module.cancel(op.pk, request.user)
|
||||
except OperationError as err:
|
||||
messages.error(request, str(err))
|
||||
messages.error(request, words(said(err)))
|
||||
return redirect("operations:job", pk=op.pk)
|
||||
|
||||
|
||||
|
|
@ -83,5 +84,5 @@ def resolve(request, pk):
|
|||
try:
|
||||
jobs_module.resolve(op.pk, request.user, request.POST.get("outcome", ""), request.POST.get("note", ""))
|
||||
except OperationError as err:
|
||||
messages.error(request, str(err))
|
||||
messages.error(request, words(said(err)))
|
||||
return redirect("operations:job", pk=op.pk)
|
||||
|
|
|
|||
|
|
@ -35,10 +35,10 @@ HOUSEKEEPING_EVERY = 600.0
|
|||
BACKOFF = (30, 60, 120, 300, 600)
|
||||
WATCH_EVERY = 15
|
||||
STALL_LIMIT = 600 # a block job left at cur == end after a libvirtd restart never ends
|
||||
STALLED = ("libvirt's merge finished copying but never completed (for example after a libvirtd restart); "
|
||||
"check the VM before resolving")
|
||||
ABANDONED = ("The worker that ran this job stopped responding; an administrator must make sure it is no longer "
|
||||
"running, then resolve this job")
|
||||
STALLED = gettext_noop("libvirt's merge finished copying but never completed (for example after a "
|
||||
"libvirtd restart); check the VM before resolving")
|
||||
ABANDONED = gettext_noop("The worker that ran this job stopped responding; an administrator must make sure it "
|
||||
"is no longer running, then resolve this job")
|
||||
WAITING = gettext_noop("Waiting for recovery")
|
||||
ACTIVE = (Operation.RUNNING, Operation.CANCELLING)
|
||||
|
||||
|
|
@ -197,7 +197,7 @@ class Worker:
|
|||
if (err := future.exception()) is not None:
|
||||
# execute raised before it could finish the job itself
|
||||
logger.error("job %s ended unfinished: %s", op_id, utils.safe_text(err))
|
||||
self.control.submit(_guarded, self.recovery.inspect_now, op_id, utils.public_error_text(err))
|
||||
self.control.submit(_guarded, self.recovery.inspect_now, op_id, utils.public_error(err))
|
||||
|
||||
def _slots(self):
|
||||
return (self.fixed_concurrency or utils.concurrency()) - len(self.jobs)
|
||||
|
|
@ -288,9 +288,9 @@ def execute(worker, op_id):
|
|||
try:
|
||||
# jobs have no request: the error users see never names a host
|
||||
if ctx.effects_started:
|
||||
worker.recovery.inspect_now(op.pk, error=utils.public_error_text(err))
|
||||
worker.recovery.inspect_now(op.pk, error=utils.public_error(err))
|
||||
else:
|
||||
jobs.finish(op, jobs.Outcome(Operation.FAILED, error=utils.public_error_text(err)), worker=worker)
|
||||
jobs.finish(op, jobs.Outcome(Operation.FAILED, error=utils.public_error(err)), worker=worker)
|
||||
except LeaseLost:
|
||||
pass
|
||||
finally:
|
||||
|
|
@ -330,7 +330,7 @@ class Scheduler:
|
|||
.values_list("pk", flat=True)):
|
||||
self.next_try.setdefault(op_id, 0)
|
||||
|
||||
def inspect_now(self, op_id, error=""):
|
||||
def inspect_now(self, op_id, error=()):
|
||||
self.errors[op_id] = error
|
||||
self.next_try[op_id] = 0
|
||||
|
||||
|
|
@ -358,7 +358,7 @@ class Scheduler:
|
|||
op.refresh_from_db()
|
||||
if previous_host != self.worker.host:
|
||||
# the error is shown to the job's owner: the host goes to the worker log only
|
||||
self._finish(op, jobs.Outcome(Operation.UNCERTAIN, error=ABANDONED))
|
||||
self._finish(op, jobs.Outcome(Operation.UNCERTAIN, error=utils.say(ABANDONED)))
|
||||
logger.warning("job %s ended uncertain: its worker %s stopped responding", op.pk, previous)
|
||||
return False
|
||||
return True
|
||||
|
|
@ -369,8 +369,7 @@ class Scheduler:
|
|||
|
||||
def _error(self, op_id, finding):
|
||||
"""What an unproven end stores: the error the job raised, then what libvirt shows."""
|
||||
raised = self.errors.get(op_id, "")
|
||||
return f"{raised.rstrip('.')}. {finding}" if raised and finding else finding or raised
|
||||
return [*self.errors.get(op_id, ()), *finding]
|
||||
|
||||
def _phase(self, op_id, text, progress=None):
|
||||
fields = {"phase": utils.safe_text(text)[:255]} if text else {}
|
||||
|
|
@ -390,8 +389,8 @@ class Scheduler:
|
|||
if op.worker != self.worker.identity and not self._take_over(op):
|
||||
return
|
||||
if not op.effects_started:
|
||||
self._finish(op, jobs.Outcome(Operation.FAILED, error=self.errors.get(op_id)
|
||||
or "The worker stopped before the job changed anything"))
|
||||
self._finish(op, jobs.Outcome(Operation.FAILED, error=list(self.errors.get(op_id, ()))
|
||||
or utils.say(gettext_noop("The worker stopped before the job changed anything"))))
|
||||
return
|
||||
try:
|
||||
outcome = jobs.get_kind(op.kind).reconcile(op)
|
||||
|
|
@ -407,7 +406,7 @@ class Scheduler:
|
|||
if not outcome.stalled:
|
||||
self.stalled_since.pop(op_id, None)
|
||||
elif now - self.stalled_since.setdefault(op_id, now) >= STALL_LIMIT:
|
||||
self._finish(op, jobs.Outcome(Operation.UNCERTAIN, error=self._error(op_id, STALLED)))
|
||||
self._finish(op, jobs.Outcome(Operation.UNCERTAIN, error=self._error(op_id, utils.say(STALLED))))
|
||||
return
|
||||
self.next_try[op_id] = now + WATCH_EVERY
|
||||
self._phase(op_id, outcome.phase, outcome.progress)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue