Skip to content

Commit da7d705

Browse files
anirudhprasad-sapPavan-SAP
anirudhprasad-sap
authored and
Pavan-SAP
committed
[Feat] Operator: Optional Session Affinity added
Provide an option to enable session affinity for CAP applications during version upgrades to ensure user sessions remain intact.
1 parent 7a6b17e commit da7d705

19 files changed

+1880
-210
lines changed

‎internal/controller/reconcile-capapplication.go‎

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/sap/cap-operator/internal/util"
1818
"github.com/sap/cap-operator/pkg/apis/sme.sap.com/v1alpha1"
19+
"golang.org/x/sync/errgroup"
1920
corev1 "k8s.io/api/core/v1"
2021
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
2122
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -134,7 +135,7 @@ func (c *Controller) handleCAPApplicationDependentResources(ctx context.Context,
134135
return
135136
}
136137

137-
// step 6 - check and set consistent status
138+
// step 6 - check and set consistent status; check for newer versions and trigger tenant networking updates
138139
return c.verifyApplicationConsistent(ctx, ca)
139140
}
140141

@@ -151,10 +152,11 @@ func (c *Controller) verifyApplicationConsistent(ctx context.Context, ca *v1alph
151152
}
152153

153154
// Check for newer CAPApplicationVersion
154-
return nil, c.checkNewCAPApplicationVersion(ctx, ca)
155+
return nil, c.checkNewCavAndTenantNetworking(ctx, ca)
155156
}
156157

157-
func (c *Controller) checkNewCAPApplicationVersion(ctx context.Context, ca *v1alpha1.CAPApplication) error {
158+
func (c *Controller) checkNewCavAndTenantNetworking(ctx context.Context, ca *v1alpha1.CAPApplication) error {
159+
// Get the latest CAV for the tenant
158160
cav, err := c.getLatestReadyCAPApplicationVersion(ca, false)
159161
if err != nil {
160162
return err
@@ -165,31 +167,28 @@ func (c *Controller) checkNewCAPApplicationVersion(ctx context.Context, ca *v1al
165167
if err != nil || len(tenants) == 0 {
166168
return err
167169
}
170+
171+
netUpdGrp := errgroup.Group{}
168172
updated := false
169173
for _, tenant := range tenants {
170-
if tenant.Spec.VersionUpgradeStrategy == v1alpha1.VersionUpgradeStrategyTypeNever {
171-
// Skip non relevant tenants
172-
continue
173-
}
174-
if tenant.Status.State == v1alpha1.CAPTenantStateProvisioning || tenant.Status.State == v1alpha1.CAPTenantStateUpgrading || tenant.Status.State == v1alpha1.CAPTenantStateDeleting {
175-
// Skip tenants that are not ready or not in processing or not in error
176-
continue
174+
if tenant.Status.CurrentCAPApplicationVersionInstance != "" {
175+
t := tenant
176+
netUpdGrp.Go(func() error {
177+
return c.reconcileTenantNetworking(ctx, t, t.Status.CurrentCAPApplicationVersionInstance, ca)
178+
})
177179
}
178-
// Assume we may have to update the tenant and prepare a copy
179-
cat := tenant.DeepCopy()
180-
181-
// Check version of tenant
182-
if cat.Spec.Version != cav.Spec.Version {
183-
// update CAPTenant Spec to point to the latest version
184-
cat.Spec.Version = cav.Spec.Version
185-
// Trigger update on CAPTenant (modifies Generation) --> which would reconcile the tenant
186-
if _, err = c.crdClient.SmeV1alpha1().CAPTenants(ca.Namespace).Update(ctx, cat, metav1.UpdateOptions{}); err != nil {
187-
return fmt.Errorf("could not update %s %s.%s: %w", v1alpha1.CAPTenantKind, cat.Namespace, cat.Name, err)
188-
}
189-
c.Event(tenant, ca, corev1.EventTypeNormal, CAPTenantEventAutoVersionUpdate, EventActionUpgrade, fmt.Sprintf("version updated to %s for initiating tenant upgrade", cav.Spec.Version))
180+
181+
if upd, err := c.checkForTenantVersionUpgrade(ctx, ca, cav, tenant); err != nil {
182+
return err
183+
} else if upd {
190184
updated = true
191185
}
192186
}
187+
188+
if err = netUpdGrp.Wait(); err != nil {
189+
return fmt.Errorf("failed to reconcile tenant networking: %w", err)
190+
}
191+
193192
if updated {
194193
msg := fmt.Sprintf("new version %s.%s was used to trigger tenant upgrades", cav.Namespace, cav.Name)
195194
ca.SetStatusWithReadyCondition(v1alpha1.CAPApplicationStateProcessing, metav1.ConditionFalse, CAPApplicationEventNewCAVTriggeredTenantUpgrade, msg)
@@ -200,6 +199,33 @@ func (c *Controller) checkNewCAPApplicationVersion(ctx context.Context, ca *v1al
200199
return nil
201200
}
202201

202+
func (c *Controller) checkForTenantVersionUpgrade(ctx context.Context, ca *v1alpha1.CAPApplication, cav *v1alpha1.CAPApplicationVersion, tenant *v1alpha1.CAPTenant) (bool, error) {
203+
if tenant.Spec.VersionUpgradeStrategy == v1alpha1.VersionUpgradeStrategyTypeNever {
204+
// Skip non relevant tenants
205+
return false, nil
206+
}
207+
if tenant.Status.State == v1alpha1.CAPTenantStateProvisioning || tenant.Status.State == v1alpha1.CAPTenantStateUpgrading || tenant.Status.State == v1alpha1.CAPTenantStateDeleting {
208+
// Skip tenants that are not ready or not in processing or not in error
209+
return false, nil
210+
}
211+
212+
// Assume we may have to update the tenant and prepare a copy
213+
cat := tenant.DeepCopy()
214+
215+
// Check version of tenant
216+
if cat.Spec.Version != cav.Spec.Version {
217+
// update CAPTenant Spec to point to the latest version
218+
cat.Spec.Version = cav.Spec.Version
219+
// Trigger update on CAPTenant (modifies Generation) --> which would reconcile the tenant
220+
if _, err := c.crdClient.SmeV1alpha1().CAPTenants(ca.Namespace).Update(ctx, cat, metav1.UpdateOptions{}); err != nil {
221+
return false, fmt.Errorf("could not update %s %s.%s: %w", v1alpha1.CAPTenantKind, cat.Namespace, cat.Name, err)
222+
}
223+
c.Event(tenant, ca, corev1.EventTypeNormal, CAPTenantEventAutoVersionUpdate, EventActionUpgrade, fmt.Sprintf("version updated to %s for initiating tenant upgrade", cav.Spec.Version))
224+
return true, nil
225+
}
226+
return false, nil
227+
}
228+
203229
func (c *Controller) checkAdditionalConditions(ca *v1alpha1.CAPApplication, result *ReconcileResult, err error) (*ReconcileResult, error) {
204230
// In case of explicit Reconcile or errors return back with the original result
205231
if result != nil || err != nil {

‎internal/controller/reconcile-capapplication_test.go‎

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ func TestController_handleCAPApplicationConsistent_Case2(t *testing.T) {
375375
}
376376

377377
func TestController_handleCAPApplicationConsistent_Case3(t *testing.T) {
378-
reconcileTestItem(
378+
err := reconcileTestItem(
379379
context.TODO(), t,
380380
QueueItem{Key: ResourceCAPApplication, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01"}},
381381
TestData{
@@ -389,35 +389,13 @@ func TestController_handleCAPApplicationConsistent_Case3(t *testing.T) {
389389
"testdata/capapplication/cat-consumer-upg-never-ready.yaml",
390390
"testdata/common/credential-secrets.yaml",
391391
},
392-
expectedResources: "testdata/capapplication/ca-31.expected.yaml",
393-
backlogItems: []string{
394-
"ERP4SMEPREPWORKAPPPLAT-2881",
395-
},
392+
expectError: true,
396393
},
397394
)
398-
}
399395

400-
func TestController_handleCAPApplicationConsistent_Case4(t *testing.T) {
401-
reconcileTestItem(
402-
context.TODO(), t,
403-
QueueItem{Key: ResourceCAPApplication, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01"}},
404-
TestData{
405-
description: "Consistent state with a CAV name update",
406-
initialResources: []string{
407-
"testdata/common/domain-ready.yaml",
408-
"testdata/common/cluster-domain-ready.yaml",
409-
"testdata/capapplication/ca-32.initial.yaml",
410-
"testdata/capapplication/cav-name-modified-ready.yaml",
411-
"testdata/capapplication/cat-provider-no-finalizers-ready.yaml",
412-
"testdata/capapplication/cat-consumer-no-finalizers-ready.yaml",
413-
"testdata/common/credential-secrets.yaml",
414-
},
415-
expectedResources: "testdata/capapplication/ca-32.expected.yaml",
416-
backlogItems: []string{
417-
"ERP4SMEPREPWORKAPPPLAT-2881",
418-
},
419-
},
420-
)
396+
if err.Error() != "failed to reconcile tenant networking: capapplicationversion.sme.sap.com \"test-cap-01-cav-v1\" not found" {
397+
t.Error("Wrong error message")
398+
}
421399
}
422400

423401
func TestController_handleCAPApplicationConsistent_Case5(t *testing.T) {
@@ -481,12 +459,11 @@ func TestCAPApplicationConsistentWithNewCAPApplicationVersionTenantUpdateError(t
481459
"testdata/capapplication/cat-provider-no-finalizers-ready.yaml",
482460
"testdata/common/credential-secrets.yaml",
483461
},
484-
expectError: true,
485-
mockErrorForResources: []ResourceAction{{Verb: "update", Group: "sme.sap.com", Version: "v1alpha1", Resource: "captenants", Namespace: "*", Name: "*"}},
462+
expectError: true,
486463
},
487464
)
488-
if err.Error() != "could not update CAPTenant default.test-cap-01-provider: mocked api error (captenants.sme.sap.com/v1alpha1)" {
489-
t.Error("error message is different from expected")
465+
if err.Error() != "failed to reconcile tenant networking: capapplicationversion.sme.sap.com \"test-cap-01-cav-v1\" not found" {
466+
t.Error("Wrong error message")
490467
}
491468
}
492469

@@ -535,7 +512,7 @@ func TestAdditionalConditionsWithTenantDeletingUpgradeStrategyNever(t *testing.T
535512
}
536513

537514
func TestController_handleCAPApplicationConsistent_versionUpgrade(t *testing.T) {
538-
reconcileTestItem(
515+
err := reconcileTestItem(
539516
context.TODO(), t,
540517
QueueItem{Key: ResourceCAPApplication, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01"}},
541518
TestData{
@@ -549,9 +526,13 @@ func TestController_handleCAPApplicationConsistent_versionUpgrade(t *testing.T)
549526
"testdata/capapplication/cat-consumer-upgrading.yaml",
550527
"testdata/common/credential-secrets.yaml",
551528
},
552-
expectedResources: "testdata/capapplication/ca-31.expected.yaml",
529+
expectError: true,
553530
},
554531
)
532+
533+
if err.Error() != "failed to reconcile tenant networking: capapplicationversion.sme.sap.com \"test-cap-01-cav-v1\" not found" {
534+
t.Error("Wrong error message")
535+
}
555536
}
556537

557538
func TestCA_ServicesOnly_Consistent(t *testing.T) {

‎internal/controller/reconcile-captenant.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ var handleCompletedProvisioningUpgradeOperation = func(ctx context.Context, c *C
163163
return NewReconcileResultWithResource(ResourceCAPTenant, cat.Name, cat.Namespace, 10*time.Second), nil
164164
}
165165

166+
// set current CAPApplicationVersionInstance and update previous versions
167+
// required for tenant networking reconciliation as it relies on the current and previous version in the status of the tenant
168+
cat.SetStatusCAPApplicationVersion(ctop.Spec.CAPApplicationVersionInstance)
169+
166170
// check and reconcile tenant virtual service
167171
// adjust virtual service only when tenant is finalizing (after provisioning or upgrade)
168172
err = c.reconcileTenantNetworking(ctx, cat, ctop.Spec.CAPApplicationVersionInstance, ca)
@@ -172,7 +176,6 @@ var handleCompletedProvisioningUpgradeOperation = func(ctx context.Context, c *C
172176

173177
// the ObservedGeneration of the tenant should be updated here (when Ready)
174178
cat.SetStatusWithReadyCondition(target.state, target.conditionStatus, target.conditionReason, message)
175-
cat.SetStatusCAPApplicationVersion(ctop.Spec.CAPApplicationVersionInstance)
176179
return getTenantReconcileResultConsideringDeletion(cat, nil), nil
177180
}
178181

@@ -242,7 +245,11 @@ func (c *Controller) reconcileCAPTenant(ctx context.Context, item QueueItem, _ i
242245
}
243246

244247
if cat.DeletionTimestamp == nil && cat.Status.CurrentCAPApplicationVersionInstance != "" {
245-
err = c.reconcileTenantNetworking(ctx, cat, cat.Status.CurrentCAPApplicationVersionInstance, nil)
248+
ca, caGetErr := c.getCachedCAPApplication(cat.Namespace, cat.Spec.CAPApplicationInstance)
249+
if caGetErr != nil {
250+
return nil, caGetErr
251+
}
252+
err = c.reconcileTenantNetworking(ctx, cat, cat.Status.CurrentCAPApplicationVersionInstance, ca)
246253
if err == nil {
247254
util.LogInfo("Tenant processing completed", string(Ready), cat, nil, "tenantId", cat.Spec.TenantId, "version", cat.Spec.Version)
248255
}

‎internal/controller/reconcile-captenant_test.go‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,3 +576,158 @@ func TestCAPTenantDeprovisioningVersionNotReady(t *testing.T) {
576576
},
577577
)
578578
}
579+
580+
func TestCAPTenantProvisioningCompletedWithSessionAffinityEnabled(t *testing.T) {
581+
reconcileTestItem(
582+
context.TODO(), t,
583+
QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}},
584+
TestData{
585+
description: "captenant provisioning operation completed (creates virtual service and destination rule) with session affinity enabled",
586+
initialResources: []string{
587+
"testdata/common/domain-ready.yaml",
588+
"testdata/common/cluster-domain-ready.yaml",
589+
"testdata/common/capapplication-session-affinity.yaml",
590+
"testdata/common/capapplicationversion-v1.yaml",
591+
"testdata/captenant/cat-04.initial.yaml",
592+
},
593+
expectedResources: "testdata/captenant/cat-with-session-affinity-dr-vs.yaml",
594+
},
595+
)
596+
}
597+
598+
func TestCAPTenantProvisioningCompletedWithSessionAffinityEnabledAndVsheaders(t *testing.T) {
599+
reconcileTestItem(
600+
context.TODO(), t,
601+
QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}},
602+
TestData{
603+
description: "captenant provisioning operation completed (creates virtual service and destination rule) with session affinity enabled and virtual service headers",
604+
initialResources: []string{
605+
"testdata/common/domain-ready.yaml",
606+
"testdata/common/cluster-domain-ready.yaml",
607+
"testdata/common/capapplication-session-affinity-vs-headers.yaml",
608+
"testdata/common/capapplicationversion-v1.yaml",
609+
"testdata/captenant/cat-04.initial.yaml",
610+
},
611+
expectedResources: "testdata/captenant/cat-with-session-affinity-dr-vs-headers.yaml",
612+
},
613+
)
614+
}
615+
616+
func TestCAPTenantProvisioningCompletedWithSessionAffinityEnabledCustomLogout(t *testing.T) {
617+
reconcileTestItem(
618+
context.TODO(), t,
619+
QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}},
620+
TestData{
621+
description: "captenant provisioning operation completed (creates virtual service and destination rule) with session affinity enabled using custom logout routes",
622+
initialResources: []string{
623+
"testdata/common/domain-ready.yaml",
624+
"testdata/common/cluster-domain-ready.yaml",
625+
"testdata/common/capapplication-session-affinity.yaml",
626+
"testdata/common/capapplicationversion-v1-custom-logout-endpoint.yaml",
627+
"testdata/captenant/cat-04.initial.yaml",
628+
},
629+
expectedResources: "testdata/captenant/cat-with-session-affinity-dr-vs-logout-endpoint.yaml",
630+
},
631+
)
632+
}
633+
634+
func TestCAPTenantUpgradeOperationCompletedWithSessionAffinityEnabled(t *testing.T) {
635+
reconcileTestItem(
636+
context.TODO(), t,
637+
QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}},
638+
TestData{
639+
description: "captenant upgrade operation completed expecting virtual service, destination rule adjustments with session affinity enabled",
640+
initialResources: []string{
641+
"testdata/common/domain-ready.yaml",
642+
"testdata/common/cluster-domain-ready.yaml",
643+
"testdata/common/capapplication-session-affinity.yaml",
644+
"testdata/common/capapplicationversion-v1.yaml",
645+
"testdata/common/capapplicationversion-v2.yaml",
646+
"testdata/captenant/provider-tenant-vs-v1.yaml",
647+
"testdata/captenant/provider-tenant-dr-v1.yaml",
648+
"testdata/captenant/cat-13.initial.yaml",
649+
},
650+
expectedResources: "testdata/captenant/cat-with-session-affinity-dr-vs-upgrade.yaml",
651+
},
652+
)
653+
}
654+
655+
func TestCAPTenantUpgradedThirdTimeWithSessionAffinityEnabled(t *testing.T) {
656+
reconcileTestItem(
657+
context.TODO(), t,
658+
QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}},
659+
TestData{
660+
description: "captenant upgraded third time - expecting virtual service, destination rule adjustments by removing config corresponding to v1 and by adding config for v3",
661+
initialResources: []string{
662+
"testdata/common/domain-ready.yaml",
663+
"testdata/common/cluster-domain-ready.yaml",
664+
"testdata/common/capapplication-session-affinity.yaml",
665+
"testdata/common/capapplicationversion-v1.yaml",
666+
"testdata/common/capapplicationversion-v2.yaml",
667+
"testdata/common/capapplicationversion-v3.yaml",
668+
"testdata/captenant/cat-with-session-affinity-dr-vs-upgrade-to-cav-v3.yaml",
669+
},
670+
expectedResources: "testdata/captenant/cat-with-session-affinity-dr-vs-upgrade-to-cav-v3.expected.yaml",
671+
},
672+
)
673+
}
674+
675+
func TestCAPTenantUpgradeOperationCompletedWithSessionAffinityEnabledAndPreviousCAVRemoved(t *testing.T) {
676+
reconcileTestItem(
677+
context.TODO(), t,
678+
QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}},
679+
TestData{
680+
description: "captenant upgraded - expecting virtual service, destination rule adjustments after removing previous cav v1",
681+
initialResources: []string{
682+
"testdata/common/domain-ready.yaml",
683+
"testdata/common/cluster-domain-ready.yaml",
684+
"testdata/common/capapplication-session-affinity.yaml",
685+
"testdata/common/capapplicationversion-v2.yaml",
686+
"testdata/captenant/cat-with-session-affinity-dr-vs-upgrade.yaml",
687+
},
688+
expectedResources: "testdata/captenant/cat-with-session-affinity-dr-vs-prev-cav-removed.yaml",
689+
},
690+
)
691+
}
692+
693+
func TestCAPTenantUpgradeOperationCompletedWithSessionAffinitySwitchedFromEnabledToDisabled(t *testing.T) {
694+
reconcileTestItem(
695+
context.TODO(), t,
696+
QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}},
697+
TestData{
698+
description: "captenant upgraded - expecting virtual service, destination rule adjustments after switching session affinity from enabled to disabled",
699+
initialResources: []string{
700+
"testdata/common/domain-ready.yaml",
701+
"testdata/common/cluster-domain-ready.yaml",
702+
"testdata/common/capapplication.yaml",
703+
"testdata/common/capapplicationversion-v1.yaml",
704+
"testdata/common/capapplicationversion-v2.yaml",
705+
"testdata/captenant/cat-with-session-affinity-dr-vs-upgrade.yaml",
706+
},
707+
expectedResources: "testdata/captenant/cat-with-session-affinity-disabled-dr-vs.yaml",
708+
},
709+
)
710+
}
711+
712+
func TestCAPTenantUpgradeOperationCompletedWithSessionAffinityEnabledAndPreviousCAVRemovedButDRDeletionFailed(t *testing.T) {
713+
err := reconcileTestItem(
714+
context.TODO(), t,
715+
QueueItem{Key: ResourceCAPTenant, ResourceKey: NamespacedResourceKey{Namespace: "default", Name: "test-cap-01-provider"}},
716+
TestData{
717+
description: "captenant upgraded - expecting virtual service, destination rule adjustments after removing previous cav v1 but DR deletion fails for some reason",
718+
initialResources: []string{
719+
"testdata/common/domain-ready.yaml",
720+
"testdata/common/cluster-domain-ready.yaml",
721+
"testdata/common/capapplication-session-affinity.yaml",
722+
"testdata/common/capapplicationversion-v2.yaml",
723+
"testdata/captenant/cat-with-session-affinity-dr-vs-upgrade.yaml",
724+
},
725+
mockErrorForResources: []ResourceAction{{Verb: "delete", Group: "networking.istio.io", Version: "v1", Resource: "destinationrules", Namespace: "*", Name: "test-cap-01-provider-test-cap-01-cav-v1"}},
726+
expectError: true,
727+
},
728+
)
729+
730+
if err.Error() != "mocked api error (destinationrules.networking.istio.io/v1)" {
731+
t.Error("error message is different from expected, actual:", err.Error())
732+
}
733+
}

0 commit comments

Comments
 (0)