feat(operator): merge brokertopicconsumers of a same service into brokerconsumers #1368
Conversation
WalkthroughThe changes revolve around updating the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![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
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (72)
components/operator/config/crd/bases/auth.components.formance.com_clients.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/auth.components.formance.com_scopes.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_auths.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_controls.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_counterparties.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_ledgers.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_orchestrations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_payments.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_searches.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_searchingesters.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_wallets.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_webhooks.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_authclients.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_auths.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_benthos.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_benthosstreams.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_brokerconsumers.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_brokertopicconsumers.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_brokertopics.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_databases.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_gatewayhttpapis.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_gateways.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_ledgers.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_orchestrations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_payments.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_reconciliations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_resourcereferences.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_searches.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_settings.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_stacks.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_stargates.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_versions.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_wallets.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_webhooks.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_configurations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_licenses.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_migrations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_stacks.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_versions.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/kustomization.yamlis excluded by!**/*.yamlcomponents/operator/config/rbac/formance.com_brokerconsumer_editor_role.yamlis excluded by!**/*.yamlcomponents/operator/config/rbac/formance.com_brokerconsumer_viewer_role.yamlis excluded by!**/*.yamlcomponents/operator/config/rbac/role.yamlis excluded by!**/*.yamlcomponents/operator/config/samples/formance.com_v1beta1_brokerconsumer.yamlis excluded by!**/*.yamlcomponents/operator/config/samples/kustomization.yamlis excluded by!**/*.yamlcomponents/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_authclients.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_auths.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_benthos.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_benthosstreams.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_brokerconsumers.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_brokertopicconsumers.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_brokertopics.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_configurations.stack.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_databases.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_gatewayhttpapis.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_gateways.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_ledgers.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_migrations.stack.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_orchestrations.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_payments.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_reconciliations.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_resourcereferences.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_searches.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_settings.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_stacks.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_stacks.stack.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_stargates.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_versions.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_versions.stack.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_wallets.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_webhooks.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/rbac.authorization.k8s.io_v1_clusterrole_manager-role.yamlis excluded by!**/*.yaml,!**/gen/**
Files selected for processing (15)
- components/operator/Earthfile (1 hunks)
- components/operator/Makefile (1 hunks)
- components/operator/PROJECT (1 hunks)
- components/operator/api/formance.com/v1beta1/brokerconsumer_types.go (1 hunks)
- components/operator/api/formance.com/v1beta1/zz_generated.deepcopy.go (1 hunks)
- components/operator/docs/crd.md (4 hunks)
- components/operator/internal/resources/brokerconsumers/controller.go (1 hunks)
- components/operator/internal/resources/brokerconsumers/create.go (1 hunks)
- components/operator/internal/resources/brokertopicconsumers/controller.go (1 hunks)
- components/operator/internal/resources/orchestrations/controller.go (4 hunks)
- components/operator/internal/resources/orchestrations/deployments.go (2 hunks)
- components/operator/internal/resources/webhooks/controller.go (4 hunks)
- components/operator/internal/resources/webhooks/deployment.go (6 hunks)
- components/operator/internal/tests/brokerconsumer_controller_test.go (3 hunks)
- components/operator/internal/tests/orchestration_controller_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- components/operator/internal/tests/brokerconsumer_controller_test.go
Additional Context Used
Additional comments not posted (21)
components/operator/internal/resources/brokerconsumers/create.go (2)
16-43: TheCreatefunction efficiently creates or updates aBrokerConsumerobject. However, consider adding error handling for the case whereservicesis empty. This could prevent creating aBrokerConsumerwith no services, which might not be a valid state depending on your domain logic.
46-63: InCreateOrUpdateOnAllServices, the filtering logic to exclude services of the same kind as the consumer is clear and concise. However, ensure that theListEventPublishersfunction, which is not visible in this context, correctly handles errors and edge cases, such as when no publishers are found for the given stack.Verification successful
The
ListEventPublishersfunction inpublisher.gocorrectly handles errors through its implementation, specifically by returning an error if theListmethod fails. For the case where no publishers are found for a given stack, it returns an empty slice, which is a standard and acceptable practice in Go for handling such scenarios. The caller of this function is expected to handle the returned slice accordingly, whether it contains items or not. This addresses the concerns raised in the review comment regarding error handling and edge cases.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the error handling and edge cases in ListEventPublishers function grep -r "ListEventPublishers" .Length of output: 311
Script:
#!/bin/bash # Examine the implementation of ListEventPublishers function for error handling and edge cases cat ./components/operator/internal/core/publisher.goLength of output: 930
components/operator/api/formance.com/v1beta1/brokerconsumer_types.go (1)
23-63: TheBrokerConsumertype is well-defined, with clear specifications for desired and observed states. It's good practice to include methods likeSetReady,IsReady,SetError, andGetStackdirectly within the type definition, as it encapsulates functionality related to the object's state management. Ensure that these methods are used consistently across your controllers to manage the state ofBrokerConsumerobjects.components/operator/internal/resources/brokertopicconsumers/controller.go (1)
33-68: The refactoredReconcilefunction simplifies the management of broker topics and consumers. However, ensure that the sorting of services (sort.Strings(t.Spec.Services)) and setting of owner references (t.SetOwnerReferences(topicQuery.OwnerReferences)) are necessary and correctly implemented. These operations can have implications on the Kubernetes objects' lifecycle and should be carefully considered.Verification successful
The verification process confirms that both sorting services and setting owner references are standard practices within the project, used across various components for managing lists and Kubernetes object lifecycles, respectively. This underscores the importance of ensuring these operations are correctly implemented in the
Reconcilefunction, as they have significant implications on system behavior and resource management. The original comment remains valid and requires careful consideration of these operations.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the necessity and correctness of sorting services and setting owner references grep -r "sort.Strings" . grep -r "SetOwnerReferences" .Length of output: 1180
components/operator/internal/resources/brokerconsumers/controller.go (1)
36-79: TheReconcilefunction inbrokerconsumers/controller.godemonstrates a comprehensive approach to managingBrokerTopicobjects based onBrokerConsumerspecifications. It's important to ensure that the error handling for creating and patchingBrokerTopicobjects is robust, especially in a Kubernetes environment where transient errors can occur. Consider implementing retries or leveraging Kubernetes' built-in retry mechanism for operations that might fail due to temporary issues.components/operator/internal/resources/webhooks/controller.go (1)
77-89: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [22-99]
The updates in
webhooks/controller.goto rename the package and variables frombrokertopicconsumerstobrokerconsumersare consistent and correctly implemented. The addition ofv1beta1.BrokerConsumerin theinitfunction is a good practice for ensuring that the controller is aware of the resources it needs to manage. Ensure that all references to the old package and variable names have been updated across the entire codebase to maintain consistency.components/operator/internal/resources/orchestrations/controller.go (1)
85-92: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [22-101]
The changes in
orchestrations/controller.goto rename the package and update variable names, as well as replacing references fromBrokerTopicConsumertoBrokerConsumer, are correctly implemented. This aligns with the PR's objective to consolidate the handling of broker consumers. Ensure that the rest of the codebase reflects these changes consistently to avoid any discrepancies.components/operator/internal/tests/orchestration_controller_test.go (1)
118-121: The update to the test case description and resource loading inorchestration_controller_test.goto create aBrokerConsumerobject is accurate and reflects the changes made in the main code. This ensures that the tests remain relevant and provide coverage for the new functionality. Good practice to keep the tests aligned with the code changes.components/operator/PROJECT (1)
175-180: The addition of theBrokerConsumerAPI definition in thePROJECTfile is correctly implemented and aligns with the PR's objectives to introduce a new model for managing broker consumers. This change is essential for the Kubernetes CRD scaffolding and ensures that the newBrokerConsumertype is recognized by the Kubernetes API. Good practice to keep the project configuration up to date with the new API definitions.components/operator/internal/resources/webhooks/deployment.go (3)
67-67: The update to thecreateAPIDeploymentfunction, including the change to use a singleBrokerConsumerand the logic for constructing theKAFKA_TOPICSenvironment variable, aligns with the PR's objectives. Ensure that all calls to this function have been updated to match the new signature and that theKAFKA_TOPICSenvironment variable is constructed as intended across different scenarios.Also applies to: 87-88
113-113: The changes in thecreateWorkerDeploymentfunction, including the parameter update to use a singleBrokerConsumerand the updated logic for theKAFKA_TOPICSenvironment variable, are consistent with the PR's goals. Verify that all references to this function have been correctly updated and that theKAFKA_TOPICSenvironment variable is accurately constructed.Also applies to: 126-127
Verification successful
To complete the verification, we need to examine how the
KAFKA_TOPICSenvironment variable is constructed within thecreateWorkerDeploymentfunction. This will ensure that the changes are consistent with the PR's goals.
The verification process confirms that the
createWorkerDeploymentfunction has been updated as described, including the parameter update and the logic for constructing theKAFKA_TOPICSenvironment variable. The function's usage in the codebase aligns with the changes, and theKAFKA_TOPICSenvironment variable is accurately constructed based on theBrokerConsumerspecification.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Additional verification for createWorkerDeployment function usage and KAFKA_TOPICS construction. ast-grep --lang go --pattern $'createWorkerDeployment($_, $_, $_, $_, $_, $_)'Length of output: 246
Script:
#!/bin/bash # Examine the construction of KAFKA_TOPICS within createWorkerDeployment rg "KAFKA_TOPICS" ./components/operator/internal/resources/webhooks/deployment.goLength of output: 306
148-156: The updates tocreateSingleDeploymentandcreateDualDeploymentfunctions, adapting them to the newBrokerConsumermodel, are in line with the PR's objectives. Ensure that these functions are correctly utilized throughout the codebase with the updated signatures.Verification successful
The verification confirms that
createSingleDeploymentandcreateDualDeploymentfunctions are utilized in the codebase, specifically withincontroller.go. This aligns with the initial review, approving the updates and suggesting ensuring their correct application. Further analysis of their usage context (e.g., parameter correctness, logic flow) wasn't within the scope of this verification but is recommended as a best practice.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verification for createSingleDeployment and createDualDeployment function usage. ast-grep --lang go --pattern $'createSingleDeployment($_, $_, $_, $_, $_, $_)' ast-grep --lang go --pattern $'createDualDeployment($_, $_, $_, $_, $_, $_)'Length of output: 493
components/operator/Earthfile (1)
37-37: The update ofcontroller-gentov0.14.0aligns with the PR's objectives to modernize the project's dependencies. Ensure that the new version does not introduce any breaking changes or unexpected behavior in the generation of CRDs, RBAC, and Webhooks.components/operator/Makefile (1)
161-161: The update ofCONTROLLER_TOOLS_VERSIONtov0.14.0in the Makefile is consistent with the project's objectives to modernize dependencies. Ensure that the new version does not introduce any breaking changes or unexpected behavior in code generation tasks.components/operator/docs/crd.md (3)
16-16: The addition ofBrokerConsumerto the list of Resource Types is noted. Ensure that the corresponding section below provides comprehensive details about this new entity, including fields, descriptions, and any default values or validation rules.
243-277: The documentation forBrokerConsumerandBrokerConsumerSpechas been added correctly. It follows the established format and provides clear information about the fields, their types, and descriptions. This addition aligns with the PR's objective to introduce theBrokerConsumermodel.
1084-1084: The inclusion ofBrokerConsumerSpecin theStackDependencysection indicates thatBrokerConsumerentities can specify dependencies on stacks. This is a crucial detail for understanding howBrokerConsumerentities interact with other components in the system.components/operator/api/formance.com/v1beta1/zz_generated.deepcopy.go (4)
476-501: The deepcopy functions forBrokerConsumerare correctly implemented and follow the expected pattern for autogenerated deepcopy functions. These are crucial for the proper handling of Kubernetes objects.
503-532: The deepcopy functions forBrokerConsumerListare correctly implemented, ensuring that eachBrokerConsumerin the list is deep copied. This is essential for managing lists of Kubernetes objects.
535-553: The deepcopy functions forBrokerConsumerSpecare correctly implemented, ensuring that specifications ofBrokerConsumerobjects, including slices and simple fields, are correctly copied.
556-569: The deepcopy functions forBrokerConsumerStatusare correctly implemented, ensuring that the status ofBrokerConsumerobjects, including theCommonStatusfield, is properly copied.
| @@ -47,7 +47,7 @@ func createAuthClient(ctx Context, stack *v1beta1.Stack, orchestration *v1beta1. | |||
|
|
|||
| func createDeployment(ctx Context, stack *v1beta1.Stack, orchestration *v1beta1.Orchestration, | |||
| database *v1beta1.Database, client *v1beta1.AuthClient, | |||
| consumers []*v1beta1.BrokerTopicConsumer, image string) error { | |||
| consumer *v1beta1.BrokerConsumer, image string) error { | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process revealed that the createDeployment function's signature change has been correctly applied in several parts of the codebase, specifically within reconciliations/controller.go, auths/controller.go, and gateways/controller.go. However, there is an oversight in orchestrations/controller.go, where the function is called with the old signature. This inconsistency needs to be addressed to ensure the application's stability and to fully align with the PR's objectives of streamlining broker consumer management.
Analysis chain
The changes in the createDeployment function, including the parameter change from multiple BrokerTopicConsumers to a single BrokerConsumer and the updated logic for constructing the TOPICS environment variable, align well with the PR's objectives to streamline broker consumer management. Ensure that all calls to this function have been updated to match the new signature and that the TOPICS environment variable is constructed as intended across different scenarios.
Also applies to: 98-99
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search to find files that import the orchestrations package. Expecting list of dependent files.
rg -C3 --type go $'import .*orchestrations'
# Search old signature to find omissions. Expecting no results.
ast-grep --lang go --pattern $'createDeployment($_, $_, $_, $_, $_, $_, $_)'
# Search new signature to validate usage. Expecting list of locations where the new signature is used.
ast-grep --lang go --pattern $'createDeployment($_, $_, $_, $_, $_, $_)'
Length of output: 836
gfyrag
force-pushed
the
feat/operator-broker-consumer
branch
from
d523350
to
0cb5b62
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (72)
components/operator/config/crd/bases/auth.components.formance.com_clients.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/auth.components.formance.com_scopes.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_auths.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_controls.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_counterparties.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_ledgers.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_orchestrations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_payments.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_searches.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_searchingesters.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_wallets.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/components.formance.com_webhooks.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_authclients.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_auths.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_benthos.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_benthosstreams.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_brokerconsumers.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_brokertopicconsumers.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_brokertopics.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_databases.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_gatewayhttpapis.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_gateways.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_ledgers.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_orchestrations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_payments.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_reconciliations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_resourcereferences.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_searches.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_settings.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_stacks.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_stargates.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_versions.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_wallets.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/formance.com_webhooks.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_configurations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_licenses.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_migrations.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_stacks.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/bases/stack.formance.com_versions.yamlis excluded by!**/*.yamlcomponents/operator/config/crd/kustomization.yamlis excluded by!**/*.yamlcomponents/operator/config/rbac/formance.com_brokerconsumer_editor_role.yamlis excluded by!**/*.yamlcomponents/operator/config/rbac/formance.com_brokerconsumer_viewer_role.yamlis excluded by!**/*.yamlcomponents/operator/config/rbac/role.yamlis excluded by!**/*.yamlcomponents/operator/config/samples/formance.com_v1beta1_brokerconsumer.yamlis excluded by!**/*.yamlcomponents/operator/config/samples/kustomization.yamlis excluded by!**/*.yamlcomponents/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_authclients.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_auths.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_benthos.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_benthosstreams.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_brokerconsumers.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_brokertopicconsumers.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_brokertopics.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_configurations.stack.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_databases.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_gatewayhttpapis.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_gateways.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_ledgers.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_migrations.stack.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_orchestrations.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_payments.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_reconciliations.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_resourcereferences.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_searches.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_settings.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_stacks.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_stacks.stack.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_stargates.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_versions.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_versions.stack.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_wallets.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/apiextensions.k8s.io_v1_customresourcedefinition_webhooks.formance.com.yamlis excluded by!**/*.yaml,!**/gen/**components/operator/helm/templates/gen/rbac.authorization.k8s.io_v1_clusterrole_manager-role.yamlis excluded by!**/*.yaml,!**/gen/**
Files selected for processing (15)
- components/operator/Earthfile (1 hunks)
- components/operator/Makefile (1 hunks)
- components/operator/PROJECT (1 hunks)
- components/operator/api/formance.com/v1beta1/brokerconsumer_types.go (1 hunks)
- components/operator/api/formance.com/v1beta1/zz_generated.deepcopy.go (1 hunks)
- components/operator/docs/crd.md (4 hunks)
- components/operator/internal/resources/brokerconsumers/controller.go (1 hunks)
- components/operator/internal/resources/brokerconsumers/create.go (1 hunks)
- components/operator/internal/resources/brokertopicconsumers/controller.go (1 hunks)
- components/operator/internal/resources/orchestrations/controller.go (4 hunks)
- components/operator/internal/resources/orchestrations/deployments.go (2 hunks)
- components/operator/internal/resources/webhooks/controller.go (4 hunks)
- components/operator/internal/resources/webhooks/deployment.go (6 hunks)
- components/operator/internal/tests/brokerconsumer_controller_test.go (3 hunks)
- components/operator/internal/tests/orchestration_controller_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (13)
- components/operator/Earthfile
- components/operator/Makefile
- components/operator/PROJECT
- components/operator/api/formance.com/v1beta1/brokerconsumer_types.go
- components/operator/api/formance.com/v1beta1/zz_generated.deepcopy.go
- components/operator/internal/resources/brokerconsumers/controller.go
- components/operator/internal/resources/brokerconsumers/create.go
- components/operator/internal/resources/orchestrations/controller.go
- components/operator/internal/resources/orchestrations/deployments.go
- components/operator/internal/resources/webhooks/controller.go
- components/operator/internal/resources/webhooks/deployment.go
- components/operator/internal/tests/brokerconsumer_controller_test.go
- components/operator/internal/tests/orchestration_controller_test.go
Additional comments not posted (5)
components/operator/internal/resources/brokertopicconsumers/controller.go (2)
33-69: TheReconcilefunction has been refactored to streamline the management of broker topics and consumers. A few observations and suggestions:
Error Handling and Logging: When listing broker topic consumers (lines 36-40), consider adding logging before returning the error. This can help with debugging and understanding the context in which errors occur.
Efficiency in Service Management: The loop for appending services to
t.Spec.Services(lines 50-60) could potentially be optimized. If the list of services grows large, this approach might become inefficient due to the nested loops. Consider using a map or set data structure to track existing services for faster lookups.Owner References: Setting owner references (line 64) is crucial for Kubernetes garbage collection. Ensure that the owner references set here correctly reflect the desired ownership hierarchy, especially in the context of the new
BrokerConsumermodel.Error Handling Post-CreateOrUpdate: After the
CreateOrUpdatecall (lines 43-67), the error is returned directly (line 68). It might be beneficial to add additional context to the error or perform specific error handling based on the error type.Overall, the changes align with the PR's objectives to simplify and consolidate the management of broker consumers. However, considering the above points could further enhance the code's maintainability and performance.
73-75: Theinitfunction setup includes a simple index forBrokerTopicConsumerbased onqueriedBy. This is a good practice for optimizing lookups. However, ensure that thequeriedByfield is always populated and unique as expected, to prevent any potential issues with the indexing mechanism.components/operator/docs/crd.md (3)
16-16: The addition ofBrokerConsumerto the Resource Types list is noted. Ensure that the corresponding detailed documentation forBrokerConsumeris complete and accurate further down in the document.
243-258: The documentation forBrokerConsumeris well-structured, following the expected format for API schema definitions. However, ensure that the fields described underBrokerConsumerSpecare comprehensive and accurately reflect the properties of theBrokerConsumermodel.
261-277: TheBrokerConsumerSpecsection is correctly documented, providing clear information on the desired state ofBrokerConsumer. It's important to verify that all fields (stack,services,queriedBy) are fully described and match the implementation in the codebase.
Summary by CodeRabbit
New Features
BrokerConsumerto manage broker consumers in a Kubernetes environment.Enhancements
controller-genversion fromv0.13.0tov0.14.0.BrokerConsumertype.Refactor
BrokerTopicConsumertoBrokerConsumeracross various files, reflecting a shift in functionality and focus.consumerstoconsumerto align with new naming conventions.Documentation
BrokerConsumerentity and its specifications in the API schema definitions.