chore: dependencies trigger downstream target reconciles #731
Conversation
WalkthroughAdds a DeploymentDependencyAction and TriggerJobSuccess, registers the action in the orchestrator to reconcile dependent targets on job success, removes an orchestrator early-return when policies are empty so actions run with an empty policy slice, deletes one unit test, and tweaks an E2E job-update sequence to use a job copy. Changes
Sequence DiagramsequenceDiagram
participant Job as Job Status Change
participant Orchestrator as Action Orchestrator
participant DDA as DeploymentDependencyAction
participant Store as Store/ReleaseTarget Repo
participant Reconcile as ReconcileTarget
Job->>Orchestrator: OnJobSuccess trigger
Orchestrator->>DDA: Execute(action trigger, context)
DDA->>Store: GetAllForDeployment(affectedResource)
Store-->>DDA: [targets...]
DDA->>DDA: filter out current target
loop for each remaining target
DDA->>Store: GetPolicies(target)
Store-->>DDA: [policies...]
alt target has DeploymentDependency rule
DDA->>Reconcile: ReconcileFn(target, TriggerJobSuccess)
Reconcile-->>DDA: result / error
end
end
DDA-->>Orchestrator: complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)apps/workspace-engine/**/*.go📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-08-12T20:49:05.086ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentdependency/deployment_dependency_policy_action.go (1)
45-48: Consider wrapping underlying errors for better error traces.The error handling loses the original error context. Wrapping errors with
%wwould preserve the error chain for debugging.Apply this diff:
policies, err := d.store.ReleaseTargets.GetPolicies(ctx, target) if err != nil { - return fmt.Errorf("failed to get policies for release target: %s", target.Key()) + return fmt.Errorf("failed to get policies for release target %s: %w", target.Key(), err) } for _, target := range targetsToReconcile { if err := d.reconcileFn(ctx, target); err != nil { - return fmt.Errorf("failed to reconcile target: %s", target.Key()) + return fmt.Errorf("failed to reconcile target %s: %w", target.Key(), err) } }Also applies to: 65-67
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/workspace-engine/pkg/workspace/releasemanager/action/orchestrator_test.go(0 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentdependency/deployment_dependency_policy_action.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go(1 hunks)apps/workspace-engine/pkg/workspace/workspace.go(2 hunks)apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go(1 hunks)
💤 Files with no reviewable changes (1)
- apps/workspace-engine/pkg/workspace/releasemanager/action/orchestrator_test.go
🧰 Additional context used
📓 Path-based instructions (2)
apps/workspace-engine/**/*.go
📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
apps/workspace-engine/**/*.go: Do not add extraneous inline comments that state the obvious
Do not add comments that simply restate what the code does
Do not add comments for standard Go patterns (e.g., noting WaitGroup or semaphore usage)
Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods
Files:
apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentdependency/deployment_dependency_policy_action.goapps/workspace-engine/pkg/workspace/releasemanager/trace/types.goapps/workspace-engine/pkg/workspace/workspace.go
apps/workspace-engine/**/*_test.go
📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
Follow the existing test structure used in *_test.go files
Files:
apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go
🧠 Learnings (4)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
📚 Learning: 2025-10-24T00:02:29.723Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 696
File: apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor.go:63-67
Timestamp: 2025-10-24T00:02:29.723Z
Learning: In workspace-engine replay mode (apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor.go), jobs are created with Pending status even during replay because job updates are still accepted and processed. If a job remains Pending after replay completes, it genuinely is still pending because no updates occurred during replay.
Applied to files:
apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go
📚 Learning: 2025-08-12T20:49:05.086Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 655
File: apps/workspace-engine/pkg/engine/workspace/fluent.go:166-171
Timestamp: 2025-08-12T20:49:05.086Z
Learning: The UpdateDeploymentVersions() method with OperationCreate case in apps/workspace-engine/pkg/engine/workspace/fluent.go is specifically designed only for creating new deployment versions, not for handling potential duplicates or existing versions.
Applied to files:
apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.goapps/workspace-engine/pkg/workspace/workspace.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*.go : Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods
Applied to files:
apps/workspace-engine/pkg/workspace/workspace.go
🧬 Code graph analysis (3)
apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go (2)
apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (1)
Status(68-68)apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
JobStatusSuccessful(135-135)JobUpdateEvent(691-698)Job(657-671)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentdependency/deployment_dependency_policy_action.go (3)
apps/workspace-engine/pkg/workspace/releasemanager/action/action.go (2)
ActionTrigger(9-9)ActionContext(19-23)apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (1)
TriggerJobSuccess(94-94)apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
Release(878-885)
apps/workspace-engine/pkg/workspace/workspace.go (4)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentdependency/deployment_dependency_policy_action.go (1)
NewDeploymentDependencyAction(18-23)apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
ReleaseTarget(888-892)apps/workspace-engine/pkg/workspace/releasemanager/trace/recorder.go (1)
ReconcileTarget(18-41)apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (2)
WithTrigger(167-171)TriggerJobSuccess(94-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (linux/amd64)
- GitHub Check: tests
- GitHub Check: workspace-engine-tests
- GitHub Check: Lint
- GitHub Check: Typecheck
🔇 Additional comments (7)
apps/workspace-engine/pkg/workspace/workspace.go (2)
5-5: LGTM!The new imports are properly utilized in the action registration below.
Also applies to: 10-11
40-47: LGTM!The action registration correctly follows the existing pattern and integrates the deployment dependency reconciliation flow with the appropriate trigger.
apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (1)
94-94: LGTM!The new trigger constant is well-defined and follows the established pattern.
apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go (1)
102-114: LGTM!Creating a copy of the job before mutation is the correct approach to avoid side effects on the original test data. The copy pattern is safe since the modified fields are value types or pointer reassignments.
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentdependency/deployment_dependency_policy_action.go (3)
11-23: LGTM!The type definitions and constructor follow Go best practices and are straightforward.
25-27: LGTM!The Name method provides a clear identifier for the action.
30-30: No action required. Theaction.TriggerJobSuccessconstant is properly defined inaction.goand used correctly at line 30. The reference totrace.TriggerJobSuccessin the review comment is incorrect—no such constant exists in the codebase.Likely an incorrect or invalid review comment.
.../releasemanager/policy/evaluator/deploymentdependency/deployment_dependency_policy_action.go
Show resolved
Hide resolved
adityachoudhari26
deleted the
dependencies-trigger-downstream-reconcile
branch
Summary by CodeRabbit
New Features
Behavior Change
Tests
✏️ Tip: You can customize this high-level summary in your review settings.