-
Notifications
You must be signed in to change notification settings - Fork 126
Docs for MCP registry CRD #2027
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
66bc3f5
docs for MCP registry CRD
dmartinol 1b6953f
Update cmd/thv-operator/REGISTRY.md
dmartinol 9d52f50
Update cmd/thv-operator/REGISTRY.md
dmartinol 8ebcdcd
Merge branch 'stacklok:main' into registry-docs
dmartinol 90ed359
updated docs from latest changes
dmartinol 7b7dd2e
integrated comments
dmartinol 165c54e
rebased
dmartinol b175127
Merge branch 'stacklok:main' into registry-docs
dmartinol File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,44 +1,111 @@ | ||
# Design & Decisions | ||
|
||
This document aims to help fill in gaps of any decision that are made around the design of the ToolHive Operator. | ||
This document captures architectural decisions and design patterns for the ToolHive Operator. | ||
|
||
## CRD Attribute vs `PodTemplateSpec` | ||
## Operator Design Principles | ||
|
||
### CRD Attribute vs `PodTemplateSpec` | ||
|
||
When building operators, the decision of when to use a `podTemplateSpec` and when to use a CRD attribute is always disputed. For the ToolHive Operator we have a defined rule of thumb. | ||
|
||
### Use Dedicated CRD Attributes For: | ||
#### Use Dedicated CRD Attributes For: | ||
- **Business logic** that affects your operator's behavior | ||
- **Validation requirements** (ranges, formats, constraints) | ||
- **Validation requirements** (ranges, formats, constraints) | ||
- **Cross-resource coordination** (affects Services, ConfigMaps, etc.) | ||
- **Operator decision making** (triggers different reconciliation paths) | ||
|
||
```yaml | ||
spec: | ||
version: "13.4" # Affects operator logic | ||
replicas: 3 # Affects scaling behavior | ||
backupSchedule: "0 2 * * *" # Needs validation | ||
``` | ||
|
||
### Use PodTemplateSpec For: | ||
#### Use PodTemplateSpec For: | ||
- **Infrastructure concerns** (node selection, resources, affinity) | ||
- **Sidecar containers** | ||
- **Sidecar containers** | ||
- **Standard Kubernetes pod configuration** | ||
- **Things a cluster admin would typically configure** | ||
|
||
```yaml | ||
spec: | ||
podTemplate: | ||
spec: | ||
nodeSelector: | ||
disktype: ssd | ||
containers: | ||
- name: sidecar | ||
image: monitoring:latest | ||
``` | ||
|
||
## Quick Decision Test: | ||
#### Quick Decision Test: | ||
1. **"Does this affect my operator's reconciliation logic?"** -> Dedicated attribute | ||
2. **"Is this standard Kubernetes pod configuration?"** -> PodTemplateSpec | ||
2. **"Is this standard Kubernetes pod configuration?"** -> PodTemplateSpec | ||
3. **"Do I need to validate this beyond basic Kubernetes validation?"** -> Dedicated attribute | ||
|
||
This gives you a clean API for core functionality while maintaining flexibility for infrastructure concerns. | ||
## MCPRegistry Architecture Decisions | ||
|
||
### Status Management Design | ||
|
||
**Decision**: Use batched status updates via StatusCollector pattern instead of individual field updates. | ||
|
||
**Rationale**: | ||
- Prevents race conditions between multiple status updates | ||
- Reduces API server load with fewer update calls | ||
- Ensures consistent status across reconciliation cycles | ||
- Handles resource version conflicts gracefully | ||
|
||
**Implementation**: StatusCollector interface collects all changes and applies them atomically. | ||
|
||
### Sync Operation Design | ||
|
||
**Decision**: Separate sync decision logic from sync execution with clear interfaces. | ||
|
||
**Rationale**: | ||
- Testability: Mock sync decisions independently from execution | ||
- Flexibility: Different sync strategies without changing core logic | ||
- Maintainability: Clear separation of concerns | ||
|
||
**Key Patterns**: | ||
- Idempotent operations for safe retry | ||
- Manual vs automatic sync distinction | ||
- Data preservation on failures | ||
|
||
### Storage Architecture | ||
|
||
**Decision**: Abstract storage via StorageManager interface with ConfigMap as default implementation. | ||
|
||
**Rationale**: | ||
- Future flexibility: Easy addition of new storage backends (OCI, databases) | ||
- Testability: Mock storage for unit tests | ||
- Consistency: Single interface for all storage operations | ||
|
||
**Current Implementation**: ConfigMap-based with owner references for automatic cleanup. | ||
|
||
### Registry API Service Pattern | ||
|
||
**Decision**: Deploy individual API service per MCPRegistry rather than shared service. | ||
|
||
**Rationale**: | ||
- **Isolation**: Each registry has independent lifecycle and scaling | ||
- **Security**: Per-registry access control possible | ||
- **Reliability**: Failure of one registry doesn't affect others | ||
- **Lifecycle Management**: Automatic cleanup via owner references | ||
|
||
**Trade-offs**: More resources consumed but better isolation and security. | ||
|
||
### Error Handling Strategy | ||
|
||
**Decision**: Structured error types with progressive retry backoff. | ||
|
||
**Rationale**: | ||
- Different error types need different handling strategies | ||
- Progressive backoff prevents thundering herd problems | ||
- Structured errors enable better observability | ||
|
||
**Implementation**: 5m initial retry, exponential backoff with cap, manual sync bypass. | ||
|
||
### Performance Design Decisions | ||
|
||
#### Resource Optimization | ||
- **Status Updates**: Batched to reduce API calls (implemented) | ||
- **Source Fetching**: Planned caching to avoid repeated downloads | ||
- **API Deployment**: Lazy creation only when needed (implemented) | ||
|
||
#### Memory Management | ||
- **Git Operations**: Shallow clones to minimize disk usage (implemented) | ||
- **Large Registries**: Stream processing planned for future | ||
- **Status Objects**: Efficient field-level updates (implemented) | ||
|
||
### Security Architecture | ||
|
||
#### Permission Model | ||
Minimal required permissions following principle of least privilege: | ||
- ConfigMaps: For storage management | ||
- Services/Deployments: For API service management | ||
- MCPRegistry: For status updates | ||
|
||
#### Network Security | ||
Optional network policies for registry API access control in security-sensitive environments. |
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.