The failpoint harness found 29 defects on devel by breaking a two-controller
docker-compose cluster. 22 of them are now reproduced here with the four
seams, in seconds and without a cluster:
test_scenarios_liveness.py a controller judged lost while alive
test_scenarios_adoption.py finalize errors, the quiet-job deadline
test_scenarios_job_control.py cancels of orphaned jobs, reaped jobs whose
playbook keeps running
test_scenarios_data.py a reaped job's stats, host ids on replay,
activity stream, files left on disk
Each defect test asserts the behavior the job should have and is marked
xfail(strict=True) with its Jira bug. When a fix lands, the test passes,
strict xfail fails the run, and the marker comes off.
The fast-clock test in test_adoption_clock.py now asserts the expected
behavior the same way. The decision unit tests that describe the inputs
behind three of these bugs point at them.
Regression tests cover what devel already gets right: repeated restarts,
a 20,000 event replay, a lost controller's waiting jobs, and a controller
gone from the mesh having its job reaped.
The other 7 defects need real process restarts, Redis, the scheduler's
clock or timing under load, and stay with the harness.
Co-Authored-By: Claude <noreply@anthropic.com>
SonarCloud flagged update_model's cognitive complexity and uncovered
takeover paths. Move the transactional write and the retry wait out of
update_model, move BaseTask.run's finalize-and-release into its own
method, and cover the takeover, lost-claim and retry paths.
Co-Authored-By: Claude <noreply@anthropic.com>
Adopting an orphaned job now claims it first: the heartbeat replaces the
job's celery_task_id with a new task id, only if it still holds the
value the heartbeat read, and then dispatches adopt_job_async under that
id. Previously the task was dispatched first and its id recorded after,
and two heartbeats could both record one.
Final status writes are fenced by that id. update_model takes
owner_task_id and raises NotOwner, writing nothing, when the job's
celery_task_id no longer matches. BaseTask.run passes the id it was
dispatched under, and adoption passes its claimed id. A task that loses
ownership neither writes its final status nor releases the work unit,
which now belongs to the new owner. An adoption whose claim was replaced
before it starts does nothing. Cancels follow celery_task_id, so they go
to the current owner.
This changes behavior: a controller that only looked dead can no longer
overwrite an adopted job's result when it comes back.
Co-Authored-By: Claude <noreply@anthropic.com>
SonarCloud flagged _heartbeat_instance_management's cognitive complexity.
Pull the receptor status read, this instance's check-in and the missing
record registration into small functions, and cover each.
Co-Authored-By: Claude <noreply@anthropic.com>
The heartbeat's judgment of lost peers and orphaned jobs, the adoption
deadline, and an adopted job's finish time now read the time from a
clock callable that defaults to django.utils.timezone.now.
cluster_node_heartbeat delegates to run_heartbeat(binder, clock), and
adopt_job_async to adopt_job(job_id, open_work, clock). Behavior is
unchanged.
awx.main.tests.fake_clock provides FakeClock. New functional tests check
the lost-peer grace period to the second, clock skew in both
directions (a fast judge finds a healthy peer lost; a dead peer with a
fast clock is found late), and the adoption deadline end to end against
the fake receptor.
Co-Authored-By: Claude <noreply@anthropic.com>
Streaming results, reading status, canceling and releasing a work unit
now go through ReceptorWork instead of raw receptorctl commands. The
process phase, error handling, release, same-controller adoption and the
work-unit reaper take a ReceptorWork. Behavior is unchanged.
awx.main.tests.fake_receptor provides FakeReceptorWork, which serves the
ansible-runner worker line protocol from canned events and can break a
stream partway, fail status calls or lose a unit. New functional tests
drive the real reattach, dedup and finalize code against it, including
one that records today's behavior when a results stream breaks: a unit
that succeeded is finalized failed.
Co-Authored-By: Claude <noreply@anthropic.com>
gate_lost_instances now takes the mesh check as a callable and skips it
when nothing is lost, so the heartbeat has no branch of its own there.
New tests cover a lost instance's job the decision doesn't reap, and a
heartbeat job the decision skips.
Co-Authored-By: Claude <noreply@anthropic.com>
Move the decisions the heartbeat makes about lost instances and orphaned
jobs into awx.main.tasks.adoption_decisions: which peers are lost, which
lost instances the mesh gate lets through, what happens to a lost
instance and its jobs, whether a running job is adopted, reaped or left
alone, and whether an orphan is past the adoption deadline. The
functions read plain attributes and touch no database, receptor or
dispatcher, so they can be tested with simple objects.
awx.main.tasks.system keeps the queries and side effects and calls the
decisions. Behavior is unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
* Introduced in PR https://github.com/ansible/awx/pull/16058/changes
then a later large merge from AAP back into devel removed the changes
* This PR re-introduces the github app lookup migration rename tests
with the migration names updated and the kind to namespace correction
* Multiple credentialtype's have the same kind and kind values look
like: cloud, network, machine, etc.
* namespace is the field that we want to rename
* Previously, we would error out because we assumed that when we got a
metrics payload from redis, that there was data in it and it was for
the current host.
* Now, we do not assume that since we got a metrics payload, that is
well formed and for the current hostname because the hostname could
have changed and we could have not yet collected metrics for the new
host.
* We had race conditions with the system_administrator role being
created just-in-time. Instead of fixing the race condition(s), dodge
them by ensuring the role always exists
* Django url validators support ipv6, our custom URLField
allow_plain_hostname feature was messing with the hostname before it
was passed to Django validators.
* This change dodges the allow_plan_hostname transformations if the
value looks like it's an ipv6 one.
* Django url validators support ipv6, our custom URLField
allow_plain_hostname feature was messing with the hostname before it
was passed to Django validators.
* This change dodges the allow_plan_hostname transformations if the
value looks like it's an ipv6 one.
* Time has passed. Channels (4.2.0) no longer raises a deprecation
warning for this case. It used to (4.1.0).
* We do NOT serve http requests over daphne, this is the default
behavior of ProtocolTypeRouter() when the http param is NOT included
* Time has passed. Channels (4.2.0) no longer raises a deprecation
warning for this case. It used to (4.1.0).
* All is good. No code changes needed for this. We do NOT service http
requests over daphne, just websockets. We, correctly, do NOT supply
the http key so daphne does NOT service http requests.
* Unit tests do not create CredentialType records for Credential
plugins. Instead, they explicitly instantiate CredentialType(s) for
Credential plugins. They rely on CredentialType.defaults[key] to do
so. This change makes sure custom_injectors get bolted onto the
created CredentialType.
* AAP 2.5 Controller 4.6 Org, User, and Team endpoints are restricted.
When the user performs a restricted operation via the Controller
collection, kindly notify them that they should be using the platform
collection instead.
* There isn't a great reason to allow the UI to edit these meta-data
fields that denote the last time an analytics job ran.
* The only reason I hesitate to mark them uneditable in the API is that
they are useful to change in order to influence when the jobs run.
Mostly for debug purposes or 1-off.
* There isn't a great reason to allow the UI to edit these meta-data
fields that denote the last time an analytics job ran.
* The only reason I hesitate to mark them uneditable in the API is that
they are useful to change in order to influence when the jobs run.
Mostly for debug purposes or 1-off.
* Register all discovered CredentialType(s) after Django finishes
loading
* Protect parallel registrations using shared postgres advisory lock
* The down-side of this is that this will run when it does not need to,
adding overhead to the init process.
* Only register discovered credential types in the database IF
migrations have ran and are up-to-date.
* #egg _could_ be awx-plugins.some.other.provided.package
* Also point at ansible devel instead of a forked branch since the
entrypoints PR has now merged to devel
```
/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/python.py:163:
PytestReturnNotNoneWarning: Expected None, but
awx/main/tests/unit/test_tasks.py::TestJobCredentials::test_custom_environment_injectors_with_boolean_extra_vars
returned ['successful', 0], which will be an error in a future version
of pytest. Did you mean to use `assert` instead of `return`?
```
* Dug into the git blame for this one
060585434a is the commit for any
historians. It was wrongfully carried over from a mock pexpect
implementation. Our new tests are nice. They don't go as far as trying
to run the task so they do not need to mock pexpect. That is why it is
safe to remove this code without finding it a new home.
* Check upstream django-ansible-base releases. If the version upstream
does not match the version we are pinned to then submit a PR with the
upstream version.
* Include a bit of context into the name of the delete function. The
HTTP_ added prepended string may be unexpected if Django's header
transformation isn't top of mind.
Chris MeyersandClaude