feat: [AAP-64171] Backport project sync feature branch to main#1482
feat: [AAP-64171] Backport project sync feature branch to main#1482hsong-rh wants to merge 4 commits intoansible:mainfrom
Conversation
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1482 +/- ##
==========================================
+ Coverage 91.51% 91.81% +0.29%
==========================================
Files 235 237 +2
Lines 10135 10444 +309
==========================================
+ Hits 9275 9589 +314
+ Misses 860 855 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/run-e2e |
|
The e2e test is passed |
…ible#1471) https://issues.redhat.com/browse/AAP-64075 https://issues.redhat.com/browse/AAP-64076 - Add project auto-sync configuration fields (`update_revision_on_launch`, `scm_update_cache_timeout`, `last_synced_at`) with `needs_update_on_launch` property and grace period protection against infinite restart loops - Enhance project sync task management to add comprehensive exception handling with `_get_project_safely()`, `_handle_project_error_recovery()`, and enhanced monitoring for stuck projects - Replace broad exception catching with specific `ObjectDoesNotExist` and `DatabaseError` handling
…e#1473) https://issues.redhat.com/browse/AAP-64077 Replace the proposed ActivationSyncDependency table with a simple `awaiting_project_sync` boolean flag on the Activation model. Add SHA256 hash fields for efficient rulebook content change detection. - **Boolean flag**: awaiting_project_sync on Activation replaces the dependency table - **SHA256 hashing**: rulesets_sha256 on Rulebook, rulebook_rulesets_sha256 on Activation - **Sync-before-launch**: Enable/restart checks needs_project_update_on_launch, sets flag, triggers sync, returns 202 - **Auto-restart**: After sync, activations with restart_on_project_update=True and changed content are restarted - **Error handling**: sync_project() failure resets flag; sync failure sets waiting activations to ERROR - **Safety nets**: disable()/destroy() clear the flag; monitor recovers orphaned flags - **No double-restart**: Auto-restart query excludes awaiting_project_sync=True activations - **Migration 0069**: Adds all new fields + RunPython to backfill SHA256 hashes - [x] Enable returns 202 when project needs sync - [x] Enable skips sync_project when project already syncing - [x] Enable proceeds normally (204) when no sync needed - [x] Restart returns 202 when project needs sync - [x] sync_project() failure resets flag and returns error - [x] Disable clears awaiting_project_sync flag - [x] Destroy clears awaiting_project_sync flag - [x] Auto-restart triggers on content change (SHA256 mismatch) - [x] Auto-restart skips unchanged content - [x] Auto-restart skips awaiting-sync activations (no double restart) - [x] Hash-only change updates metadata without restart - [x] Resume enables disabled waiting activations - [x] Resume restarts enabled waiting activations - [x] Resume handles multiple activations per project - [x] Resume failure sets activation to ERROR - [x] Sync failure sets ERROR on waiting activations - [x] Sync failure ignores non-waiting activations - [x] Deleted project cleans up waiting activations - [x] Monitor recovers orphaned awaiting activations (completed project) - [x] Monitor recovers orphaned awaiting activations (failed project) - [x] Monitor skips actively syncing projects - [x] Migration populates SHA256 for existing rulebooks - [x] Migration populates SHA256 for existing activations - [x] Migration handles empty rulesets
…gs (ansible#1480) https://issues.redhat.com/browse/AAP-65811 Skip auto-restart for activations with source_mappings when rulebook content changes during project sync, freezing their cached content so the stale state is preserved for warning detection. Add a computed `warnings` list field to the activation detail API endpoint that surfaces when the activation's rulebook SHA256 differs from the current upstream rulebook, or when the linked rulebook has been deleted. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace check_project_queue_health with check_default_worker_health (renamed on main) - Add check_dispatcherd_workers_health mocks to sync dependency tests (new health check added on main) - Remove stale check_project_queue_health import from test_projects.py Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
| "skip_audit_events": activation.skip_audit_events, | ||
| "rulebook_name": activation.rulebook.name, | ||
| "rulebook_rulesets": activation.rulebook_rulesets, | ||
| "rulebook_rulesets_sha256": activation.rulebook_rulesets_sha256, |
There was a problem hiding this comment.
Is this the revision hash? If so, is this what controller calls it? It doesn't seem familiar to me as a field that controller sends.
There was a problem hiding this comment.
No, this is not the Git revision hash (that's git_hash). rulebook_rulesets_sha256 is a SHA256 hash of the rulebook's YAML rulesets content, computed locally by EDA using get_rulebook_hash(). It's not sent by controller — it's generated at activation create/update time and stored so we can later detect if the upstream rulebook content changed after a project sync. The naming follows the existing rulebook_rulesets field, with _sha256 appended to indicate it's a content hash of that field.
| "rulebook_rulesets" | ||
| ] = activation.rulebook.rulesets | ||
| "rulebook_rulesets_sha256" | ||
| ] = get_rulebook_hash(rulesets) |
There was a problem hiding this comment.
The argument in the definition of get_rulebook_hash is called 'rulebook'. Here we call it rulesets, which implies plural rule set where as the function definition argument implies a singular rulebook. I understand that this function returns a hash, but I don't really understand how that hash applies to project sync.
There was a problem hiding this comment.
Good observation on the naming. The get_rulebook_hash function parameter is called rulebook because it takes the string content of a rulebook. Here rulesets refers to activation.rulebook.rulesets, which is the model field that stores the YAML content string — so it's the same data, the variable name just follows the field name. We could rename the variable for clarity but it matches the existing pattern from the previous code.
Regarding project sync: when a project sync occurs, it re-imports the git repo and updates each Rulebook.rulesets_sha256. The activation stores its own copy in rulebook_rulesets_sha256 from when it was created/updated. During project sync, we compare the two hashes — if they differ, the rulebook content has changed and the activation either auto-restarts (if restart_on_project_update=True) or surfaces a warning in the API response indicating source mappings may be stale.
| """Check if activation requires project update on launch.""" | ||
| if not self.project: | ||
| return False | ||
| return self.project.needs_update_on_launch |
There was a problem hiding this comment.
Shouldn't we check if the cache timeout has expired here?
There was a problem hiding this comment.
The cache timeout check is already handled inside self.project.needs_update_on_launch (see src/aap_eda/core/models/project.py). That property checks: (1) whether update_revision_on_launch is enabled, (2) a 30-second grace period to prevent infinite sync-restart loops, (3) whether scm_update_cache_timeout has expired based on last_synced_at. The activation property intentionally just delegates to the project to keep the logic in one place and avoid duplication.
3 participants
https://issues.redhat.com/browse/AAP-64171
Summary
Backport the project-sync feature branch into main. This PR includes three features and a rebase conflict resolution commit:
update_revision_on_launchandscm_update_cache_timeoutfields to the Project model, enabling automatic project synchronization before activation launch (migration 0068)awaiting_project_syncflag. When a project sync changes rulebook content, enabled activations withrestart_on_project_update=Trueare automatically restarted (migration 0069)source_mappingsand surfaces a computedwarningsfield on the activation detail API when rulebook content has driftedcheck_project_queue_health→check_default_worker_health) and addscheck_dispatcherd_workers_healthmocks to sync dependency testsTest plan
test_projects.py,test_activation.py,test_project.py🤖 Generated with Claude Code