From 8550716542c728ccd4d1a44f752222abb0c719e6 Mon Sep 17 00:00:00 2001 From: Burak Sekili Date: Wed, 12 Jul 2023 19:36:48 +0300 Subject: [PATCH 1/5] Fetch the resource before updating it Signed-off-by: Burak Sekili --- controllers/apidefinition_controller.go | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/controllers/apidefinition_controller.go b/controllers/apidefinition_controller.go index e22fdd506..de25f1176 100644 --- a/controllers/apidefinition_controller.go +++ b/controllers/apidefinition_controller.go @@ -194,11 +194,20 @@ func (r *ApiDefinitionReconciler) Reconcile(ctx context.Context, req ctrl.Reques upstreamRequestStruct.Spec.CollectLoopingTarget() // If this is not set, means it is a new object, set it first - if desired.Status.ApiID == "" { - return r.create(ctx, upstreamRequestStruct) + switch desired.Status.ApiID { + case "": + err = r.create(ctx, upstreamRequestStruct) + default: + err = r.update(ctx, upstreamRequestStruct) } + if err != nil { + return err + } + + _ = r.Client.Get(ctx, client.ObjectKeyFromObject(desired), desired) // nolint:errcheck - return r.update(ctx, upstreamRequestStruct) + desired.Spec = upstreamRequestStruct.Spec + return nil }) var transactionInfo *tykv1alpha1.TransactionInfo @@ -461,11 +470,6 @@ func (r *ApiDefinitionReconciler) create(ctx context.Context, desired *tykv1alph status.ApiID = *desired.Spec.APIID status.LatestTykSpecHash = calculateHash(apiOnTyk) status.LatestCRDSpecHash = calculateHash(desired.Spec) - status.LatestTransaction = tykv1alpha1.TransactionInfo{ - Time: metav1.Now(), - Status: tykv1alpha1.Successful, - Error: "", - } }, ) if err != nil { @@ -539,11 +543,6 @@ func (r *ApiDefinitionReconciler) update(ctx context.Context, desired *tykv1alph func(status *tykv1alpha1.ApiDefinitionStatus) { status.LatestTykSpecHash = calculateHash(apiOnTyk) status.LatestCRDSpecHash = calculateHash(desired.Spec) - status.LatestTransaction = tykv1alpha1.TransactionInfo{ - Time: metav1.Now(), - Status: tykv1alpha1.Successful, - Error: "", - } }, ) if err != nil { From f4f312519e9f78e16f807686570fc61e049f3eab Mon Sep 17 00:00:00 2001 From: Burak Sekili Date: Wed, 12 Jul 2023 21:55:24 +0300 Subject: [PATCH 2/5] copy object onto new one Signed-off-by: Burak Sekili --- controllers/apidefinition_controller.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/controllers/apidefinition_controller.go b/controllers/apidefinition_controller.go index de25f1176..2c6dcfca3 100644 --- a/controllers/apidefinition_controller.go +++ b/controllers/apidefinition_controller.go @@ -204,9 +204,10 @@ func (r *ApiDefinitionReconciler) Reconcile(ctx context.Context, req ctrl.Reques return err } - _ = r.Client.Get(ctx, client.ObjectKeyFromObject(desired), desired) // nolint:errcheck + latestDesired := &tykv1alpha1.ApiDefinition{} + _ = r.Client.Get(ctx, client.ObjectKeyFromObject(desired), latestDesired) // nolint:errcheck + desired.ObjectMeta = latestDesired.ObjectMeta - desired.Spec = upstreamRequestStruct.Spec return nil }) From 121400dc9285692cbf2e05ce52b2ff62df62760b Mon Sep 17 00:00:00 2001 From: Burak Sekili Date: Thu, 13 Jul 2023 12:39:28 +0300 Subject: [PATCH 3/5] Use Patch instead of update requests Signed-off-by: Burak Sekili --- controllers/apidefinition_controller.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/controllers/apidefinition_controller.go b/controllers/apidefinition_controller.go index 2c6dcfca3..371e2931c 100644 --- a/controllers/apidefinition_controller.go +++ b/controllers/apidefinition_controller.go @@ -204,10 +204,6 @@ func (r *ApiDefinitionReconciler) Reconcile(ctx context.Context, req ctrl.Reques return err } - latestDesired := &tykv1alpha1.ApiDefinition{} - _ = r.Client.Get(ctx, client.ObjectKeyFromObject(desired), latestDesired) // nolint:errcheck - desired.ObjectMeta = latestDesired.ObjectMeta - return nil }) @@ -236,7 +232,7 @@ func (r *ApiDefinitionReconciler) Reconcile(ctx context.Context, req ctrl.Reques ctx, desired.Namespace, model.Target{Namespace: &desired.Namespace, Name: desired.Name}, - false, + true, func(status *tykv1alpha1.ApiDefinitionStatus) { status.LatestTransaction = *transactionInfo }, ) }) @@ -868,7 +864,7 @@ func (r *ApiDefinitionReconciler) updateStatus( fn(&api.Status) - return r.Status().Update(ctx, &api) + return r.Status().Patch(ctx, &api, client.MergeFrom(api.DeepCopy())) } // breakSubgraphLink breaks the link between given ApiDefinition and Subgraph object it refers to. If pass is specified, From 83e0a323188e08de5bbc4d4929d4b28a6837e96d Mon Sep 17 00:00:00 2001 From: Burak Sekili Date: Thu, 13 Jul 2023 12:41:51 +0300 Subject: [PATCH 4/5] Bring Status check to back Signed-off-by: Burak Sekili --- controllers/apidefinition_controller.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/controllers/apidefinition_controller.go b/controllers/apidefinition_controller.go index 371e2931c..285fb5962 100644 --- a/controllers/apidefinition_controller.go +++ b/controllers/apidefinition_controller.go @@ -194,17 +194,11 @@ func (r *ApiDefinitionReconciler) Reconcile(ctx context.Context, req ctrl.Reques upstreamRequestStruct.Spec.CollectLoopingTarget() // If this is not set, means it is a new object, set it first - switch desired.Status.ApiID { - case "": - err = r.create(ctx, upstreamRequestStruct) - default: - err = r.update(ctx, upstreamRequestStruct) - } - if err != nil { - return err + if desired.Status.ApiID == "" { + return r.create(ctx, upstreamRequestStruct) } - return nil + return r.update(ctx, upstreamRequestStruct) }) var transactionInfo *tykv1alpha1.TransactionInfo From 4d52666b26ac6c54bdd014a9103b9681e5a415cc Mon Sep 17 00:00:00 2001 From: Burak Sekili Date: Thu, 13 Jul 2023 15:59:08 +0300 Subject: [PATCH 5/5] Move status update out of CreateOrUpdate Signed-off-by: Burak Sekili --- controllers/apidefinition_controller.go | 55 ++++++++++++------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/controllers/apidefinition_controller.go b/controllers/apidefinition_controller.go index 285fb5962..590ac14a1 100644 --- a/controllers/apidefinition_controller.go +++ b/controllers/apidefinition_controller.go @@ -222,10 +222,35 @@ func (r *ApiDefinitionReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Reconciler must return the error observed by CreateOrUpdate() function since the mutator given to CreateOrUpdate // returns special custom error such as ErrMultipleLinkSubGraph. errK8s := retry.RetryOnConflict(retry.DefaultRetry, func() error { + namespace := upstreamRequestStruct.Namespace + target := model.Target{Namespace: &namespace, Name: upstreamRequestStruct.Name} + + if desired.Status.ApiID == "" { + apiId := "" + if upstreamRequestStruct.Spec.APIID != nil { + apiId = *upstreamRequestStruct.Spec.APIID + } + + apiOnTyk, _ := klient.Universal.Api().Get(ctx, apiId) //nolint:errcheck + + return r.updateStatus( + ctx, + desired.Namespace, + target, + true, + func(status *tykv1alpha1.ApiDefinitionStatus) { + status.ApiID = apiId + status.LatestTykSpecHash = calculateHash(apiOnTyk) + status.LatestCRDSpecHash = calculateHash(upstreamRequestStruct.Spec) + status.LatestTransaction = *transactionInfo + }, + ) + } + return r.updateStatus( ctx, desired.Namespace, - model.Target{Namespace: &desired.Namespace, Name: desired.Name}, + target, true, func(status *tykv1alpha1.ApiDefinitionStatus) { status.LatestTransaction = *transactionInfo }, ) @@ -447,32 +472,6 @@ func (r *ApiDefinitionReconciler) create(ctx context.Context, desired *tykv1alph return err } - apiOnTyk, _ := klient.Universal.Api().Get(ctx, *desired.Spec.APIID) //nolint:errcheck - - namespace := desired.Namespace - target := model.Target{Namespace: &namespace, Name: desired.Name} - - err = r.updateStatus( - ctx, - desired.Namespace, - target, - false, - func(status *tykv1alpha1.ApiDefinitionStatus) { - status.ApiID = *desired.Spec.APIID - status.LatestTykSpecHash = calculateHash(apiOnTyk) - status.LatestCRDSpecHash = calculateHash(desired.Spec) - }, - ) - if err != nil { - r.Log.Error( - err, - "Failed to update Status ID", - "ApiDefinition", client.ObjectKeyFromObject(desired).String(), - ) - - return err - } - return nil } @@ -858,7 +857,7 @@ func (r *ApiDefinitionReconciler) updateStatus( fn(&api.Status) - return r.Status().Patch(ctx, &api, client.MergeFrom(api.DeepCopy())) + return r.Status().Update(ctx, &api) } // breakSubgraphLink breaks the link between given ApiDefinition and Subgraph object it refers to. If pass is specified,