Skip to content

Commit d4a15d8

Browse files
feat: require tokenRateLimits and remove tokenRateLimitRef for 3.4 (opendatahub-io#644)
## Summary Related to - https://redhat.atlassian.net/browse/RHOAIENG-55976 This PR makes `tokenRateLimits` required on each `MaaSSubscription` model reference and removes the unused `tokenRateLimitRef` field from the CRD. ## Changes ### API Changes - ✅ Removed `tokenRateLimitRef` field from `ModelSubscriptionRef` - ✅ Made `tokenRateLimits` required with `MinItems=1` validation - ✅ Regenerated CRD manifests and deepcopy code ### Tests - ✅ Fixed 4 unit tests (added missing field indexes) - ✅ All tests passing - ✅ Live cluster validation complete ### Documentation - ✅ Updated CRD reference docs - ✅ Updated architecture overview - ✅ Created migration guide: `docs/content/migration/tokenratelimits-required-3.4.md` - ✅ Added v3.4.0 release notes with breaking change notice ## Live Cluster Testing Results (https://console-openshift-console.apps.ci-ln-hdzds3t-76ef8.aws-4.ci.openshift.org/) Deployed custom controller and verified: - ✅ Subscriptions **without** `tokenRateLimits` → **REJECTED** with "Required value" error - ✅ Subscriptions **with** `tokenRateLimits` → **ACCEPTED** - ✅ Subscriptions with old `tokenRateLimitRef` → **REJECTED** with "unknown field" error - ✅ Controller reconciliation working correctly - ✅ No RBAC errors ## Migration Impact **Before (3.3 and earlier):** ```yaml modelRefs: - name: my-model namespace: llm tokenRateLimits: # Optional - limit: 1000 window: 1m tokenRateLimitRef: "some-ref" # Unused by controller ``` **After (3.4+):** ```yaml modelRefs: - name: my-model namespace: llm tokenRateLimits: # REQUIRED - at least one - limit: 1000 window: 1m # tokenRateLimitRef removed ``` ## Migration Guide See detailed migration instructions in `docs/content/migration/tokenratelimits-required-3.4.md` Users must add inline `tokenRateLimits` to all `MaaSSubscription` resources before upgrading to 3.4. ## Acceptance Criteria All criteria from the user story met: - [x] `tokenRateLimitRef` not present in API - [x] Each `modelRefs` entry must include non-empty `tokenRateLimits` - [x] Validation rejects non-compliant resources with clear messages - [x] Tests cover required limits, no `tokenRateLimitRef` references - [x] Generated CRD YAML matches types - [x] Documentation updated - [x] Migration guide provided - [x] Breaking change documented in release notes ## Files Changed - `maas-controller/api/maas/v1alpha1/maassubscription_types.go` - API types - `deployment/base/maas-controller/crd/bases/maas.opendatahub.io_maassubscriptions.yaml` - CRD manifest - `maas-controller/api/maas/v1alpha1/zz_generated.deepcopy.go` - Generated code - `maas-controller/pkg/controller/maas/maassubscription_controller_test.go` - Test fixes - `docs/content/configuration-and-management/maas-controller-overview.md` - Architecture docs - `docs/content/reference/crds/maas-subscription.md` - CRD reference - `docs/content/migration/tokenratelimits-required-3.4.md` - Migration guide (new) - `docs/content/release-notes/index.md` - Release notes 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Breaking Changes** * Rate limit configuration for each model subscription is now mandatory and must be specified inline rather than referenced externally * At least one rate limit entry is required per model * Rate limit values must be greater than zero * **Documentation** * Updated configuration and CRD documentation to reflect new rate limit requirements <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 6b2d7e4 commit d4a15d8

File tree

7 files changed

+22
-18
lines changed

7 files changed

+22
-18
lines changed

deployment/base/maas-controller/crd/bases/maas.opendatahub.io_maassubscriptions.yaml

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,6 @@ spec:
7676
maxLength: 63
7777
minLength: 1
7878
type: string
79-
tokenRateLimitRef:
80-
description: TokenRateLimitRef references an existing TokenRateLimit
81-
resource
82-
type: string
8379
tokenRateLimits:
8480
description: TokenRateLimits defines token-based rate limits
8581
for this model
@@ -89,6 +85,7 @@ spec:
8985
limit:
9086
description: Limit is the maximum number of tokens allowed
9187
format: int64
88+
minimum: 1
9289
type: integer
9390
window:
9491
description: Window is the time window (e.g., "1m", "1h",
@@ -99,10 +96,12 @@ spec:
9996
- limit
10097
- window
10198
type: object
99+
minItems: 1
102100
type: array
103101
required:
104102
- name
105103
- namespace
104+
- tokenRateLimits
106105
type: object
107106
minItems: 1
108107
type: array

docs/content/configuration-and-management/maas-controller-overview.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ erDiagram
192192

193193
- **MaaSModelRef**: `spec.modelRef.kind` = LLMInferenceService or ExternalModel; `spec.modelRef.name` = name of the referenced model resource.
194194
- **MaaSAuthPolicy**: `spec.modelRefs` (list of ModelRef objects with name and namespace), `spec.subjects` (groups, users).
195-
- **MaaSSubscription**: `spec.owner` (groups, users), `spec.modelRefs` (list of ModelSubscriptionRef objects with name, namespace, and either `tokenRateLimits` array or `tokenRateLimitRef` reference to define per-model rate limits).
195+
- **MaaSSubscription**: `spec.owner` (groups, users), `spec.modelRefs` (list of ModelSubscriptionRef objects with name, namespace, and required `tokenRateLimits` array to define per-model rate limits).
196196

197197
---
198198

docs/content/reference/crds/maas-subscription.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ Defines a subscription plan with per-model token rate limits. Creates Kuadrant T
2424
|-------|------|----------|-------------|
2525
| name | string | Yes | Name of the MaaSModelRef |
2626
| namespace | string | Yes | Namespace where the MaaSModelRef lives |
27-
| tokenRateLimits | []TokenRateLimit | No | Token-based rate limits for this model |
28-
| tokenRateLimitRef | string | No | Reference to an existing TokenRateLimit resource |
27+
| tokenRateLimits | []TokenRateLimit | Yes | Token-based rate limits for this model (at least one required) |
2928
| billingRate | BillingRate | No | Cost per token |
3029

3130
## TokenRateLimit

docs/content/release-notes/index.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
# Release Notes
22

3+
## v3.4.0
4+
5+
### Major Changes
6+
7+
Version 3.4.0 introduces new CRDs and API resources that are not compatible with previous versions. All MaaS custom resources (`MaaSModelRef`, `MaaSAuthPolicy`, `MaaSSubscription`) are new in this release.
8+
9+
**Migration:** See the overall migration plan for detailed upgrade instructions from previous versions.
10+
11+
---
12+
313
## v0.1.0
414

515
*Initial release.*

maas-controller/api/maas/v1alpha1/maassubscription_types.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,8 @@ type ModelSubscriptionRef struct {
6464
Namespace string `json:"namespace"`
6565

6666
// TokenRateLimits defines token-based rate limits for this model
67-
// +optional
68-
TokenRateLimits []TokenRateLimit `json:"tokenRateLimits,omitempty"`
69-
70-
// TokenRateLimitRef references an existing TokenRateLimit resource
71-
// +optional
72-
TokenRateLimitRef *string `json:"tokenRateLimitRef,omitempty"`
67+
// +kubebuilder:validation:MinItems=1
68+
TokenRateLimits []TokenRateLimit `json:"tokenRateLimits"`
7369

7470
// BillingRate defines the cost per token
7571
// +optional
@@ -79,6 +75,7 @@ type ModelSubscriptionRef struct {
7975
// TokenRateLimit defines a token rate limit
8076
type TokenRateLimit struct {
8177
// Limit is the maximum number of tokens allowed
78+
// +kubebuilder:validation:Minimum=1
8279
Limit int64 `json:"limit"`
8380

8481
// Window is the time window (e.g., "1m", "1h", "24h")

maas-controller/api/maas/v1alpha1/zz_generated.deepcopy.go

Lines changed: 0 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

maas-controller/pkg/controller/maas/maassubscription_controller_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@ func TestMaaSSubscriptionReconciler_RemoveModelRef(t *testing.T) {
397397
WithRESTMapper(testRESTMapper()).
398398
WithObjects(modelRefA, modelRefB, routeA, routeB, sub).
399399
WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
400+
WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
400401
Build()
401402

402403
r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
@@ -492,6 +493,7 @@ func TestMaaSSubscriptionReconciler_RemoveModelRef_Aggregation(t *testing.T) {
492493
WithRESTMapper(testRESTMapper()).
493494
WithObjects(modelRefA, modelRefB, routeA, routeB, sub1, sub2).
494495
WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
496+
WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
495497
Build()
496498

497499
r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
@@ -717,6 +719,7 @@ func TestMaaSSubscriptionReconciler_SimplifiedTRLP(t *testing.T) {
717719
WithRESTMapper(testRESTMapper()).
718720
WithObjects(model, route, maasSub).
719721
WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
722+
WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
720723
Build()
721724

722725
r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
@@ -810,6 +813,7 @@ func TestMaaSSubscriptionReconciler_MultipleSubscriptionsSimplified(t *testing.T
810813
WithRESTMapper(testRESTMapper()).
811814
WithObjects(model, route, subA, subB).
812815
WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
816+
WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
813817
Build()
814818

815819
r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}

0 commit comments

Comments
 (0)