Rewrite SubscriptionReconciler to use new planner and Pipeline CRs#206
Open
ryannedolan wants to merge 1 commit intomainfrom
Open
Rewrite SubscriptionReconciler to use new planner and Pipeline CRs#206ryannedolan wants to merge 1 commit intomainfrom
ryannedolan wants to merge 1 commit intomainfrom
Conversation
Replace the legacy SubscriptionReconciler (which used HoptimatorPlanner, Operator, and Resource templates) with a new implementation built on the modern planner infrastructure (DeploymentService, PipelineRel, Deployers). The new reconciler creates Pipeline K8s CRs owned by Subscriptions, delegating status monitoring to the existing PipelineReconciler. This provides a clear upgrade path for the internal SubscriptionOperator fork while preserving the Subscription CRD contract. Also fixes a hanging K8sJobDeployerTest caused by stale mock stubs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jogrogan
reviewed
Apr 15, 2026
Collaborator
jogrogan
left a comment
jogrogan
left a comment
There was a problem hiding this comment.
Change overall makes sense. Discussed already, but think we need some additional testing with our internal extensions before we can merge
Comment on lines
+70
to
+71
| SubscriptionReconciler.Planner planner = SubscriptionReconciler.jdbcPlanner(jdbcConnection); | ||
| new PipelineOperatorApp(context).run(planner); |
Collaborator
There was a problem hiding this comment.
It seems like there is a lot of logic just to allow SubscriptionReconciler to support drop in planners, though in practice it'll always use the jdbcPlanner. Do you think we need this?
| @@ -6,9 +6,6 @@ plugins { | |||
|
|
|||
Collaborator
There was a problem hiding this comment.
I suspect there are more files that are now unused in the hoptimator-operator package, namely "Operator.java" but maybe more that we can remove.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SubscriptionReconcilerto useDeploymentService.plan()/PipelineRelinstead of the legacyHoptimatorPlanner,Operator, andResourcetemplate systemPipelineK8s CRs owned by Subscriptions, delegating readiness monitoring to the existingPipelineReconcilerSUBSCRIPTIONSendpoint inK8sApiEndpointsand wired the controller intoPipelineOperatorAppSubscriptionEnvironment(deprecated) and droppedhoptimator-planner/hoptimator-catalog/hoptimator-avrodependencies from the operator modulePlannerinterface for testability, withjdbcPlanner()factory for production useK8sJobDeployerTestcaused by stale mock stubsMigration note: The
hoptimator-plannermodule is intentionally untouched — the internalSubscriptionOperatorfork still depends on the published jar. This PR provides a clear upgrade path: deploy the new operator and existingSubscriptionCRs will be handled by the new planner.Testing Done
SubscriptionReconcilerusingFakeK8sApipatternK8sJobDeployerTest./gradlew testpasses with 0 failures