-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: informer general framework and engine/discovery interface definition #1314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the Informer Framework from client-go to support ResourceEngine and ResourceDiscovery for list-watch functionality. It adds comprehensive event handling capabilities through an EventBus component and refactors the resource registration system to support better runtime object handling.
- Implements an EventBus component for event distribution between components
- Adds a comprehensive controller framework with Informer and ResourceListerWatcher interfaces
- Refactors resource registration from kind-only to schema-based with factory functions
Reviewed Changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/resourcegen/main.go | Updates resource template generation to include DeepCopyObject methods and factory functions |
| pkg/core/resource/model/resource.go | Adds k8s runtime.Object interface requirement to Resource interface |
| pkg/core/resource/model/registry.go | Refactors from ResourceKindRegistry to ResourceSchemaRegistry with factory functions |
| pkg/core/events/eventbus.go | Completely rewrites EventBus interface to support resource change events |
| pkg/core/events/component.go | Implements EventBus as a runtime component with synchronous event processing |
| pkg/core/controller/*.go | Adds new Informer framework for list-watch operations |
| pkg/core/engine/*.go | Implements ResourceEngine component with factory pattern |
| pkg/core/discovery/*.go | Implements ResourceDiscovery component with informer support |
| pkg/core/bootstrap/bootstrap.go | Adds EventBus initialization to the bootstrap process |
| Various resource types | Adds DeepCopyObject methods and factory functions to all resource types |
Comments suppressed due to low confidence (2)
pkg/core/discovery/component.go:1
- Missing init() function registration for the discoveryComponent. The component is not being registered with the runtime, so it won't be available for initialization.
/*
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
@锦涛 @凤瑞 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the Informer Framework from client-go to support ResourceEngine and ResourceDiscovery components for list-watch functionality. The changes establish a comprehensive event-driven architecture for resource management in the Dubbo admin system.
- Refactors resource registration to use factory functions and schema registry
- Introduces EventBus component for event distribution with ResourceChangedEvent
- Implements Informer pattern for watching resource changes with controller framework
Reviewed Changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/resourcegen/main.go | Updates resource code generation template to support DeepCopyObject and new factory patterns |
| pkg/core/resource/model/resource.go | Adds k8sruntime.Object interface requirement to Resource |
| pkg/core/resource/model/registry.go | Refactors from resource kind registry to schema registry with factory functions |
| pkg/core/resource/apis//v1alpha1/_types.go | Implements DeepCopyObject methods and new resource factory functions |
| pkg/core/events/eventbus.go | Replaces subscriber-based eventbus with resource-kind subscription manager |
| pkg/core/events/component.go | New EventBus component implementation with synchronous event processing |
| pkg/core/controller/informer.go | Implements informer pattern for watching and caching resource changes |
| pkg/core/engine/component.go | ResourceEngine component that manages informers for infrastructure resources |
| pkg/core/discovery/component.go | ResourceDiscovery component for service discovery with informers |
| pkg/core/bootstrap/bootstrap.go | Adds EventBus initialization to bootstrap sequence |
| pkg/config/app/admin.go | Updates discovery config to support multiple discovery configurations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
1kasa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM,The entire PR has an elegant code style, but I have a question: Why are the load order of the discovery/Component, engine/Component, and EventBus/Component components all set to math.MaxInt? Doesn't this make their startup order uncertain? Is there another way to ensure ordering? Or is ordering not really that important?
I'm very curious about this and would like to hear your thoughts.
At first, the start order of each component is hardcoded in bootstrap, so the start order is certain and ordered. The originial purpose of |
Understood, thank you for your answer |



Please provide a description of this PR:
Introduce Informer Framwork from client-go to support ResourceEngine and ResourceDiscovery for list-watch.
To help us figure out who should review this PR, please put an X in all the areas that this PR affects.
Please check any characteristics that apply to this pull request.