@@ -18,6 +18,7 @@ package maas
1818
1919import (
2020 "context"
21+ "errors"
2122 "fmt"
2223 "strings"
2324 "testing"
@@ -31,6 +32,8 @@ import (
3132 ctrl "sigs.k8s.io/controller-runtime"
3233 "sigs.k8s.io/controller-runtime/pkg/client"
3334 "sigs.k8s.io/controller-runtime/pkg/client/fake"
35+ "sigs.k8s.io/controller-runtime/pkg/client/interceptor"
36+ "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3437 gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
3538
3639 maasv1alpha1 "github.com/opendatahub-io/models-as-a-service/maas-controller/api/maas/v1alpha1"
@@ -1346,3 +1349,203 @@ func TestMaaSSubscriptionReconciler_WindowValuesInTRLP(t *testing.T) {
13461349 })
13471350 }
13481351}
1352+
1353+ // permanentUpdateError returns an apierrors.StatusError that isPermanentError classifies
1354+ // as non-recoverable (admission webhook rejection).
1355+ func permanentUpdateError () error {
1356+ return & apierrors.StatusError {ErrStatus : metav1.Status {
1357+ Status : metav1 .StatusFailure ,
1358+ Code : 422 ,
1359+ Reason : metav1 .StatusReasonInvalid ,
1360+ Message : "admission webhook denied the request: invalid window format" ,
1361+ }}
1362+ }
1363+
1364+ // TestHandleDeletion_PermanentError_FallsBackToDeleteTRLP verifies that when
1365+ // reconcileTRLPForModel fails with a permanent error (e.g. admission webhook rejects
1366+ // the TRLP update), the handler falls back to force-deleting the TRLP and removes
1367+ // the finalizer so the subscription deletion is not permanently blocked.
1368+ func TestHandleDeletion_PermanentError_FallsBackToDeleteTRLP (t * testing.T ) {
1369+ const (
1370+ modelName = "shared-model"
1371+ namespace = "default"
1372+ trlpName = "maas-trlp-" + modelName
1373+ )
1374+
1375+ model := & maasv1alpha1.MaaSModelRef {
1376+ ObjectMeta : metav1.ObjectMeta {Name : modelName , Namespace : namespace },
1377+ Spec : maasv1alpha1.MaaSModelSpec {ModelRef : maasv1alpha1.ModelReference {Kind : "ExternalModel" , Name : modelName }},
1378+ }
1379+ route := & gatewayapiv1.HTTPRoute {
1380+ ObjectMeta : metav1.ObjectMeta {Name : modelName , Namespace : namespace },
1381+ }
1382+ existingTRLP := newPreexistingTRLP (trlpName , namespace , modelName , map [string ]string {})
1383+ subA := newMaaSSubscription ("sub-a" , namespace , "team-a" , modelName , 100 )
1384+ subA .Finalizers = []string {maasSubscriptionFinalizer }
1385+ subB := newMaaSSubscription ("sub-b" , namespace , "team-b" , modelName , 200 )
1386+
1387+ c := fake .NewClientBuilder ().
1388+ WithScheme (scheme ).
1389+ WithRESTMapper (testRESTMapper ()).
1390+ WithObjects (model , route , subA , subB , existingTRLP ).
1391+ WithIndex (& maasv1alpha1.MaaSSubscription {}, "spec.modelRef" , subscriptionModelRefIndexer ).
1392+ WithInterceptorFuncs (interceptor.Funcs {
1393+ Update : func (ctx context.Context , cl client.WithWatch , obj client.Object , opts ... client.UpdateOption ) error {
1394+ if u , ok := obj .(* unstructured.Unstructured ); ok && u .GetKind () == "TokenRateLimitPolicy" {
1395+ return permanentUpdateError ()
1396+ }
1397+ return cl .Update (ctx , obj , opts ... )
1398+ },
1399+ }).
1400+ Build ()
1401+
1402+ if err := c .Delete (context .Background (), subA ); err != nil {
1403+ t .Fatalf ("Delete sub-a: %v" , err )
1404+ }
1405+
1406+ r := & MaaSSubscriptionReconciler {Client : c , Scheme : scheme }
1407+ req := ctrl.Request {NamespacedName : types.NamespacedName {Name : "sub-a" , Namespace : namespace }}
1408+ if _ , err := r .Reconcile (context .Background (), req ); err != nil {
1409+ t .Fatalf ("Reconcile: unexpected error: %v" , err )
1410+ }
1411+
1412+ // TRLP should be deleted via the fallback deleteModelTRLP path
1413+ got := & unstructured.Unstructured {}
1414+ got .SetGroupVersionKind (schema.GroupVersionKind {Group : "kuadrant.io" , Version : "v1alpha1" , Kind : "TokenRateLimitPolicy" })
1415+ if err := c .Get (context .Background (), types.NamespacedName {Name : trlpName , Namespace : namespace }, got ); ! apierrors .IsNotFound (err ) {
1416+ t .Errorf ("expected TRLP to be deleted via fallback, but got err: %v" , err )
1417+ }
1418+
1419+ // Finalizer should be removed
1420+ var sub maasv1alpha1.MaaSSubscription
1421+ if err := c .Get (context .Background (), types.NamespacedName {Name : "sub-a" , Namespace : namespace }, & sub ); ! apierrors .IsNotFound (err ) {
1422+ t .Errorf ("expected sub-a to be fully deleted (finalizer removed), got err: %v" , err )
1423+ }
1424+ }
1425+
1426+ // TestHandleDeletion_TransientError_RequeuesForRetry verifies that transient errors
1427+ // (timeouts, network flakes) during deletion are returned for requeue rather than
1428+ // triggering force-delete, which could collaterally damage shared TRLPs.
1429+ func TestHandleDeletion_TransientError_RequeuesForRetry (t * testing.T ) {
1430+ const (
1431+ modelName = "shared-model"
1432+ namespace = "default"
1433+ trlpName = "maas-trlp-" + modelName
1434+ )
1435+
1436+ model := & maasv1alpha1.MaaSModelRef {
1437+ ObjectMeta : metav1.ObjectMeta {Name : modelName , Namespace : namespace },
1438+ Spec : maasv1alpha1.MaaSModelSpec {ModelRef : maasv1alpha1.ModelReference {Kind : "ExternalModel" , Name : modelName }},
1439+ }
1440+ route := & gatewayapiv1.HTTPRoute {
1441+ ObjectMeta : metav1.ObjectMeta {Name : modelName , Namespace : namespace },
1442+ }
1443+ existingTRLP := newPreexistingTRLP (trlpName , namespace , modelName , map [string ]string {})
1444+ subA := newMaaSSubscription ("sub-a" , namespace , "team-a" , modelName , 100 )
1445+ subA .Finalizers = []string {maasSubscriptionFinalizer }
1446+ subB := newMaaSSubscription ("sub-b" , namespace , "team-b" , modelName , 200 )
1447+
1448+ // Transient error: generic timeout, not an admission rejection
1449+ c := fake .NewClientBuilder ().
1450+ WithScheme (scheme ).
1451+ WithRESTMapper (testRESTMapper ()).
1452+ WithObjects (model , route , subA , subB , existingTRLP ).
1453+ WithIndex (& maasv1alpha1.MaaSSubscription {}, "spec.modelRef" , subscriptionModelRefIndexer ).
1454+ WithInterceptorFuncs (interceptor.Funcs {
1455+ Update : func (ctx context.Context , cl client.WithWatch , obj client.Object , opts ... client.UpdateOption ) error {
1456+ if u , ok := obj .(* unstructured.Unstructured ); ok && u .GetKind () == "TokenRateLimitPolicy" {
1457+ return errors .New ("simulated transient API server timeout" )
1458+ }
1459+ return cl .Update (ctx , obj , opts ... )
1460+ },
1461+ }).
1462+ Build ()
1463+
1464+ if err := c .Delete (context .Background (), subA ); err != nil {
1465+ t .Fatalf ("Delete sub-a: %v" , err )
1466+ }
1467+
1468+ r := & MaaSSubscriptionReconciler {Client : c , Scheme : scheme }
1469+ req := ctrl.Request {NamespacedName : types.NamespacedName {Name : "sub-a" , Namespace : namespace }}
1470+ _ , err := r .Reconcile (context .Background (), req )
1471+ if err == nil {
1472+ t .Fatal ("Reconcile should return error on transient failure for requeue" )
1473+ }
1474+
1475+ // TRLP should NOT be deleted — transient errors don't trigger force-delete
1476+ got := & unstructured.Unstructured {}
1477+ got .SetGroupVersionKind (schema.GroupVersionKind {Group : "kuadrant.io" , Version : "v1alpha1" , Kind : "TokenRateLimitPolicy" })
1478+ if getErr := c .Get (context .Background (), types.NamespacedName {Name : trlpName , Namespace : namespace }, got ); getErr != nil {
1479+ t .Errorf ("TRLP should still exist after transient error, but got: %v" , getErr )
1480+ }
1481+
1482+ // Finalizer should NOT be removed — requeue will retry
1483+ var sub maasv1alpha1.MaaSSubscription
1484+ if getErr := c .Get (context .Background (), types.NamespacedName {Name : "sub-a" , Namespace : namespace }, & sub ); getErr != nil {
1485+ t .Fatalf ("sub-a should still exist with finalizer, got: %v" , getErr )
1486+ }
1487+ if ! controllerutil .ContainsFinalizer (& sub , maasSubscriptionFinalizer ) {
1488+ t .Error ("sub-a finalizer should still be present after transient error" )
1489+ }
1490+ }
1491+
1492+ // TestHandleDeletion_PermanentError_BothFail_StillRemovesFinalizer verifies that
1493+ // when both TRLP reconciliation and force-delete fail with permanent errors, the
1494+ // finalizer is still removed so the subscription is not stuck forever.
1495+ func TestHandleDeletion_PermanentError_BothFail_StillRemovesFinalizer (t * testing.T ) {
1496+ const (
1497+ modelName = "shared-model"
1498+ namespace = "default"
1499+ trlpName = "maas-trlp-" + modelName
1500+ )
1501+
1502+ model := & maasv1alpha1.MaaSModelRef {
1503+ ObjectMeta : metav1.ObjectMeta {Name : modelName , Namespace : namespace },
1504+ Spec : maasv1alpha1.MaaSModelSpec {ModelRef : maasv1alpha1.ModelReference {Kind : "ExternalModel" , Name : modelName }},
1505+ }
1506+ route := & gatewayapiv1.HTTPRoute {
1507+ ObjectMeta : metav1.ObjectMeta {Name : modelName , Namespace : namespace },
1508+ }
1509+ existingTRLP := newPreexistingTRLP (trlpName , namespace , modelName , map [string ]string {})
1510+ subA := newMaaSSubscription ("sub-a" , namespace , "team-a" , modelName , 100 )
1511+ subA .Finalizers = []string {maasSubscriptionFinalizer }
1512+ subB := newMaaSSubscription ("sub-b" , namespace , "team-b" , modelName , 200 )
1513+
1514+ // Permanent error on Update + fail List so deleteModelTRLP also fails
1515+ c := fake .NewClientBuilder ().
1516+ WithScheme (scheme ).
1517+ WithRESTMapper (testRESTMapper ()).
1518+ WithObjects (model , route , subA , subB , existingTRLP ).
1519+ WithIndex (& maasv1alpha1.MaaSSubscription {}, "spec.modelRef" , subscriptionModelRefIndexer ).
1520+ WithInterceptorFuncs (interceptor.Funcs {
1521+ Update : func (ctx context.Context , cl client.WithWatch , obj client.Object , opts ... client.UpdateOption ) error {
1522+ if u , ok := obj .(* unstructured.Unstructured ); ok && u .GetKind () == "TokenRateLimitPolicy" {
1523+ return permanentUpdateError ()
1524+ }
1525+ return cl .Update (ctx , obj , opts ... )
1526+ },
1527+ List : func (ctx context.Context , cl client.WithWatch , list client.ObjectList , opts ... client.ListOption ) error {
1528+ if ul , ok := list .(* unstructured.UnstructuredList ); ok && ul .GetKind () == "TokenRateLimitPolicyList" {
1529+ return permanentUpdateError ()
1530+ }
1531+ return cl .List (ctx , list , opts ... )
1532+ },
1533+ }).
1534+ Build ()
1535+
1536+ if err := c .Delete (context .Background (), subA ); err != nil {
1537+ t .Fatalf ("Delete sub-a: %v" , err )
1538+ }
1539+
1540+ r := & MaaSSubscriptionReconciler {Client : c , Scheme : scheme }
1541+ req := ctrl.Request {NamespacedName : types.NamespacedName {Name : "sub-a" , Namespace : namespace }}
1542+ if _ , err := r .Reconcile (context .Background (), req ); err != nil {
1543+ t .Fatalf ("Reconcile should not return error on permanent failure (finalizer must be removed), got: %v" , err )
1544+ }
1545+
1546+ // Finalizer should be removed despite all cleanup failures
1547+ var sub maasv1alpha1.MaaSSubscription
1548+ if err := c .Get (context .Background (), types.NamespacedName {Name : "sub-a" , Namespace : namespace }, & sub ); ! apierrors .IsNotFound (err ) {
1549+ t .Errorf ("expected sub-a to be fully deleted (finalizer removed despite permanent failures), got err: %v" , err )
1550+ }
1551+ }
0 commit comments