Flexible scheduling #2786

jpbruinsslot · 2024-04-04T12:30:30Z

Please refer to #204 for more background on this PR

Changes

Since this is a big PR with substantial changes, I've added the following for more guidance on review and qa. It is helpful to read the above-linked issue to get more context. And whenever you need me I can give you this as well in a call.

The main changes are:

Addition of a new Schedule model to support a more deterministic approach to (re)scheduling of tasks
Removal of the PrioritizedItem model and its database table. The queue is not a separate table anymore, a 'view' of the database filtered on status queued is the current state of the queued items of a scheduler.

Specific code changes of interest:

Model changes
- Schedule model added to models/schedule.py
  - migrations added ( take specific note of the p_item change to data in the task table)
  - store added to storage/schedule_store.py
- Removed PrioritizedItem
  - migration added for removal of items table
BoefjeScheduler changes
- added a new process that adds new tasks to be picked up by checking the schedule of prior tasks: push_tasks_for_rescheduling()
NormalizerScheduler changes
- minor changes made to support the new setup
Abstract base class Scheduler changes - some methods have been moved around so below are the main points of interest:
- minor changes made to support the new setup
- The post_push() method creates a Schedule for when a task has been added to the queue.
API server changes
- Added endpoints for Schedule
- Updated Task endpoints to support the new setup
Boefje Runner changes to support the new setup
Rocky changes to support the new setup

QA notes

Since this pr is now a replacement for the rescheduling of tasks within the scheduler the functionality of KAT should remain the same. So the main issues that could arise is from starting, (automatic) rescheduling of scans. All other actions of scanning that would touch the scheduler.
- To check if schedule objects have been created for the boefje tasks, you can run through the onboarding, do some scans and check the schedules curl http://localhost:8004/schedules
Migrations, since there are substantial data migrations with this pr, care should be given on checking whether migrations from data prior to this pr will applied correctly with this pr.
- Check out main
- make reset
- Walk through the onboarding, do some scans
- Check out feature/mula/refactor-queue
- docker compose kill shutdown all the containers
- make kat (we don't want to reset we want the migration to be applied on the initial run)

Code Checklist

All the commits in this PR are properly PGP-signed and verified.
This PR only contains functionality relevant to the issue.
I have written unit tests for the changes or fixes I made.
I have checked the documentation and made changes where necessary.
I have performed a self-review of my code and refactored it to the best of my abilities.

Tickets have been created for newly discovered issues.
For any non-trivial functionality, I have added integration and/or end-to-end tests.
I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.

Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

* main: (21 commits) Add IPv6 config to docker-compose.yml (#2256) Remove IPs with zero vulnerabilities (#2319) Translations update from Hosted Weblate (#2311) Chore/update pr template with comments (#2305) Sector report summary - Best and worst scoring security checks (#2312) Remove icons from compliance issue table (#2340) More ulimits for buggy celery (#2338) Remove smartphone from bug report template (#2334) add meta / cache hash for rpki boefje to raw output (#2255) Add max fds ulimit to octopoes api worker (#2327) Fix multiple Debian issues (#2283) Fix/upgrade jinja2 (#2326) Remove preselection from multireport flow (#2318) Updated template file to respect environment prefixes in docs (#2317) fix zero division (#2298) List item behaviour (#2281) Translations update from Hosted Weblate (#2279) Fixed invalid type usage in `get_rabbit_channel` and `close_rabbit_channel` (#2280) Translations update from Hosted Weblate (#2261) Fix export buttons report (#2259) ...

* main: Fix/394 Introduce clearance level control for objects imported by CSV (#2390) Update RabbitMQ to the latest version (#2392) Show created at and data from in reports (#2370) Rename invalid rpki finding to expired (#2377) Convert `docker-compose` to `docker compose` (#2341) Remove uWSGI (#2366) Prevent double github actions (#2374)

* main: Fix/394 Introduce clearance level control for objects imported by CSV (#2390) Update RabbitMQ to the latest version (#2392) Show created at and data from in reports (#2370) Rename invalid rpki finding to expired (#2377) Convert `docker-compose` to `docker compose` (#2341) Remove uWSGI (#2366) Prevent double github actions (#2374) Fix WEASYPRINT_BASEURL default value and change ports in docker-compose.yml (#2373) Remove debian11 packages (#2358) Error handling for Generate Report (#2274) Update dependencies (#2348) Add token authentication (#2349) Adds CAA records to the model, boefje, normalizer, adds a check bit and a finding (#2315) Check for sudo in install and update script (#2360) Feat/normalizer mimetype upload deeplink (#2220) Add hrefs to Basic Security overview (#2330) Render dicts and list ooi attrs as jsonfield (#2355)

…lexible-scheduling * chore/mula/update-architecture-doc: Fix Fix diagram010.svg Update docs and restructure some code Update documentation Add diagrams Update architecture documentation

* main: Fix new boefjes issue for scheduler (#3297) Add audit logging to CRUD actions using Django signals (#3314) fix: Button height (#3316) Add user id to OOI (#3305) Translations update from Hosted Weblate (#3179) Restructure scheduler storage module (#3294) Fix CSRF error in API with token auth (#3313)

* main: Restructure scheduler server module (#3295)

* main: Restructure scheduler development scripts (#3293) Change report flow to POST requests (#3174)

Donnype

Good stuff @jpbruinsslot! Do you have a todo list or timeline on when we can merge this and make the switch?

rocky/tests/conftest.py

boefjes/boefjes/clients/bytes_client.py

mula/scheduler/connectors/services/octopoes.py

mula/scheduler/server/handlers/queues.py

Donnype · 2024-08-07T11:54:45Z

mula/scheduler/schedulers/scheduler.py

+        # We at least delay a job by the grace period
+        minimum = self.ctx.config.pq_grace_period
+        deadline = datetime.now(timezone.utc) + timedelta(seconds=minimum)
+
+        # We want to delay the job by a random amount of time, in a range of 5 hours
+        jitter_range_seconds = 5 * 60 * 60
+        jitter_offset = timedelta(
+            seconds=random.uniform(-jitter_range_seconds, jitter_range_seconds)  # noqa
+        )

-        return True
+        # Check if the adjusted time is earlier than the minimum, and
+        # ensure that the adjusted time is not earlier than the deadline
+        adjusted_time = deadline + jitter_offset
+        adjusted_time = max(adjusted_time, deadline)

-    def is_item_on_queue_by_hash(self, item_hash: str) -> bool:
-        return self.queue.is_item_on_queue_by_hash(item_hash)
+        return adjusted_time


Note that, if I read this correctly, this create a uniform distribution between "(minimal) deadline - 5 hours" and "deadline + 5 hours", and then sets everything below the deadline equal to the deadline. In practice this will converge to a situation where 50% of the time the adjusted_time equals the deadline and the other 50% is uniformly distributed over the 5 hours after the deadline. I think that's not what's intended? Perhaps the jitter offset should be:

jitter_offset = timedelta(seconds=random.uniform(0, 2*jitter_range_seconds))

To spread it out uniformly over the 10 hours after the (minimal) deadline?

@Donnype I've updated it, would you mind checking?

mula/scheduler/storage/schedule_store.py

mula/scheduler/alembic/versions/0008_add_task_schedule.py

Donnype · 2024-08-07T12:13:38Z

mula/scheduler/alembic/versions/0008_add_task_schedule.py

+depends_on = None
+
+
+def upgrade():


Looks good! I challenge you to test it

Yeah, tested it with a minimal set. But would be good to fully test it

ammar92

Looks good. Just a few remarks. We've already discussed the PR in a high level overview offline

mula/Makefile

mula/scheduler/context/context.py

mula/scheduler/schedulers/normalizer.py

mula/scheduler/server/serializers/schedule.py

mula/scheduler/server/serializers/task.py

ammar92 · 2024-08-07T12:53:28Z

mula/scheduler/utils/cron.py

+from croniter import croniter
+
+
+def next_run(expression: str, start_time: datetime = datetime.now(timezone.utc)):


You can't assign datetime.now(timezone.utc) in the signature, otherwise it will always have the same value (based on the module import time). The Pythonic way is to assign a None as default, and assign a new datetime in the function body if the passed value is indeed None.

jpbruinsslot · 2024-08-07T14:15:56Z

Good stuff @jpbruinsslot! Do you have a todo list or timeline on when we can merge this and make the switch?

Thanks! And thank you for your suggestions. I wanted to get the reviews out first. For the next steps, I'll implement the suggestions, get QA to look at it and focus on the migration for a bit to make sure that works out fine. When merged I'd like to have it deployed in order to test to uncover edge cases, and how it will fare with more throughput/load.

* main: Basic audit trails via logging (#3317) Raw upload with Scan OOIS (#3169) Fix Garbage collection and disappearing ports issue (#3214) Updated `Django` and `opentelemetry` packages (#3324)

stephanie0x00 · 2024-08-12T13:56:05Z

Checklist for QA:

I have checked out this branch, and successfully ran a fresh make reset.
I confirmed that there are no unintended functional regressions in this branch:
- I have managed to pass the onboarding flow
- Objects and Findings are created properly
- Tasks are created and completed properly
I confirmed that the PR's advertised feature or hotfix works as intended.
I checked the logs for errors and/or warnings and made issues where necessary

What works:

General scheduling works on a 'make reset', tasks can be rescheduled, links in the UI to/from the task list seem to work. Findings are created.
Migrating also seems to work. Can still reschedule tasks, perform new scanning activities. Data of tasks list is still visible. Looks good 👍

What doesn't work:

When migrating (followed the steps as described in the QA notes), the task list history disappears and an error message is shown in the UI. The task statistics table does show that multiple tasks are present.
n/a

Bug or feature?:

n/a

jpbruinsslot · 2024-08-12T14:37:28Z

Updated and amended qa instructions, for additional testing

jpbruinsslot added 30 commits January 2, 2024 18:00

Update flexible scheduling to new main

a52d173

Continue refactor

3447649

Make test work

99aeeba

Continue adding tests

0a96e21

Add job_store tests

3ead50e

Add boefje test for 'no ooi' boefjes

e762a79

Add more job storage tests, and fix big

e6f4773

Remove re-enable of job when task pushed

d111b51

Remove test-case

3a9a95d

Start with api endpoints for jobs

9535b70

Implement api endpoints, tests, and ranker

21e8f33

Basic deadline calculation

5123f33

Update

fa15756

Implement cron like functionality

45fb545

Add ValidationError

291a73d

Remove rate limit mentions in favour of feature branch

ed2fd7f

Trying pre-commit

4d020a7

Update documentation

0f193fb

Update docs with schematic

9b34f56

Update schematic

776242d

Update architecture documentation

dcecb2f

Add diagrams

cc85e65

Update documentation

01e01c5

Update docs and restructure some code

44e415f

Fix diagram010.svg

8968abe

Fix

86db29b

Merge branch 'chore/mula/update-architecture-doc' into feature/mula/f…

2a7693b

…lexible-scheduling * chore/mula/update-architecture-doc: Fix Fix diagram010.svg Update docs and restructure some code Update documentation Add diagrams Update architecture documentation

jpbruinsslot added 18 commits August 1, 2024 14:32

Fix for boefje/normalizer runner

aebb870

Pre-commit

f6be13a

Fix requirements

2516b1b

Update croniter

abf5bf1

Update from croniter types

f777ef3

Ignore type

dc7f1f4

Ignore security-sensitive check of random

6ed961f

Fix shadowing of python builtin

beeab12

Pre-commit

c849d0a

Fix linting

00c126d

Fix manual rescheduling

7926f53

Fix schedule_id when manual rescheduling

4b46b0c

Merge branch 'main' into feature/mula/refactor-queue

69e6faf

* main: Restructure scheduler server module (#3295)

Merge branch 'main' into feature/mula/refactor-queue

0e4b2fa

* main: Restructure scheduler development scripts (#3293) Change report flow to POST requests (#3174)

Fix formatting

7371c55

Formatting

e4daab6

Formatting

bfda8fb

Donnype reviewed Aug 7, 2024

View reviewed changes

ammar92 reviewed Aug 7, 2024

View reviewed changes

jpbruinsslot added 3 commits August 8, 2024 12:15

Updates according to code review

299fc22

Merge branch 'main' into feature/mula/refactor-queue

efb871a

* main: Basic audit trails via logging (#3317) Raw upload with Scan OOIS (#3169) Fix Garbage collection and disappearing ports issue (#3214) Updated `Django` and `opentelemetry` packages (#3324)

Fix poetry

50c39a2

Merge branch 'main' into feature/mula/refactor-queue

ee4c395

underdarknl approved these changes Aug 13, 2024

View reviewed changes

underdarknl merged commit df6f526 into main Aug 13, 2024
21 checks passed

underdarknl deleted the feature/mula/refactor-queue branch August 13, 2024 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flexible scheduling #2786

Flexible scheduling #2786

jpbruinsslot commented Apr 4, 2024 •

edited

Loading

Donnype left a comment

Donnype Aug 7, 2024

jpbruinsslot Aug 8, 2024

Donnype Aug 7, 2024

jpbruinsslot Aug 7, 2024

ammar92 left a comment

ammar92 Aug 7, 2024

jpbruinsslot commented Aug 7, 2024

stephanie0x00 commented Aug 12, 2024 •

edited

Loading

jpbruinsslot commented Aug 12, 2024

		from croniter import croniter


		def next_run(expression: str, start_time: datetime = datetime.now(timezone.utc)):

Flexible scheduling #2786

Flexible scheduling #2786

Conversation

jpbruinsslot commented Apr 4, 2024 • edited Loading

Changes

QA notes

Code Checklist

Checklist for code reviewers:

Checklist for QA:

Donnype left a comment

Choose a reason for hiding this comment

Donnype Aug 7, 2024

Choose a reason for hiding this comment

jpbruinsslot Aug 8, 2024

Choose a reason for hiding this comment

Donnype Aug 7, 2024

Choose a reason for hiding this comment

jpbruinsslot Aug 7, 2024

Choose a reason for hiding this comment

ammar92 left a comment

Choose a reason for hiding this comment

ammar92 Aug 7, 2024

Choose a reason for hiding this comment

jpbruinsslot commented Aug 7, 2024

stephanie0x00 commented Aug 12, 2024 • edited Loading

Checklist for QA:

What works:

What doesn't work:

Bug or feature?:

jpbruinsslot commented Aug 12, 2024

jpbruinsslot commented Apr 4, 2024 •

edited

Loading

stephanie0x00 commented Aug 12, 2024 •

edited

Loading