From 3ea4f61e75348d286a750586b4f6707f13cd98aa Mon Sep 17 00:00:00 2001 From: Yi Tao Date: Tue, 24 Sep 2024 11:01:37 +0800 Subject: [PATCH] add KongTarget sample and do not delete when upstream being deleted --- config/samples/konnect_kongtarget.yaml | 46 +++++++++++++++++ controller/konnect/errors.go | 12 +++++ controller/konnect/ops/ops_kongtarget.go | 49 ++----------------- controller/konnect/reconciler_generic.go | 32 +++++++----- controller/konnect/reconciler_upstreamref.go | 5 +- .../konnect/reconciler_upstreamref_test.go | 2 +- 6 files changed, 87 insertions(+), 59 deletions(-) create mode 100644 config/samples/konnect_kongtarget.yaml diff --git a/config/samples/konnect_kongtarget.yaml b/config/samples/konnect_kongtarget.yaml new file mode 100644 index 000000000..c1671088a --- /dev/null +++ b/config/samples/konnect_kongtarget.yaml @@ -0,0 +1,46 @@ +kind: KonnectAPIAuthConfiguration +apiVersion: konnect.konghq.com/v1alpha1 +metadata: + name: konnect-api-auth-dev-1 + namespace: default +spec: + type: token + token: kpat_XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX + serverURL: us.api.konghq.com +--- +kind: KonnectGatewayControlPlane +apiVersion: konnect.konghq.com/v1alpha1 +metadata: + name: test1 + namespace: default +spec: + name: test1 + labels: + app: test1 + key1: test1 + konnect: + authRef: + name: konnect-api-auth-dev-1 +--- +kind: KongUpstream +apiVersion: configuration.konghq.com/v1alpha1 +metadata: + name: upstream-1 + namespace: default +spec: + name: upstream-1 + controlPlaneRef: + type: konnectNamespacedRef + konnectNamespacedRef: + name: test1 +--- +kind: KongTarget +apiVersion: configuration.konghq.com/v1alpha1 +metadata: + name: target-1 + namespace: default +spec: + upstreamRef: + name: upstream-1 + target: "10.0.0.1" + weight: 100 \ No newline at end of file diff --git a/controller/konnect/errors.go b/controller/konnect/errors.go index e56883c79..bdb3c07aa 100644 --- a/controller/konnect/errors.go +++ b/controller/konnect/errors.go @@ -72,3 +72,15 @@ type ReferencedKongUpstreamIsBeingDeleted struct { func (e ReferencedKongUpstreamIsBeingDeleted) Error() string { return fmt.Sprintf("referenced Kong Upstream %s is being deleted", e.Reference) } + +// ReferencedKongUpstreamDoesNotExist is an error type that is returned when +// a Konnect entity references a Kong Upstream which does not exist. +type ReferencedKongUpstreamDoesNotExist struct { + Reference types.NamespacedName + Err error +} + +// Error implements the error interface. +func (e ReferencedKongUpstreamDoesNotExist) Error() string { + return fmt.Sprintf("referenced Kong Upstream %s does not exist: %v", e.Reference, e.Err) +} diff --git a/controller/konnect/ops/ops_kongtarget.go b/controller/konnect/ops/ops_kongtarget.go index 14c994f02..bb01ea60b 100644 --- a/controller/konnect/ops/ops_kongtarget.go +++ b/controller/konnect/ops/ops_kongtarget.go @@ -11,13 +11,9 @@ import ( sdkkonnectops "github.com/Kong/sdk-konnect-go/models/operations" sdkkonnecterrs "github.com/Kong/sdk-konnect-go/models/sdkerrors" "github.com/samber/lo" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/kong/gateway-operator/controller/konnect/conditions" - k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes" - configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" "github.com/kong/kubernetes-configuration/pkg/metadata" ) @@ -42,30 +38,12 @@ func createTarget( }) if errWrapped := wrapErrIfKonnectOpFailed(err, CreateOp, target); errWrapped != nil { - k8sutils.SetCondition( - k8sutils.NewConditionWithGeneration( - conditions.KonnectEntityProgrammedConditionType, - metav1.ConditionFalse, - "FailedToCreate", - errWrapped.Error(), - target.GetGeneration(), - ), - target, - ) + SetKonnectEntityProgrammedConditionFalse(target, "FailedToCreate", errWrapped.Error()) return errWrapped } target.Status.Konnect.SetKonnectID(*resp.Target.ID) - k8sutils.SetCondition( - k8sutils.NewConditionWithGeneration( - conditions.KonnectEntityProgrammedConditionType, - metav1.ConditionTrue, - conditions.KonnectEntityProgrammedReasonProgrammed, - "", - target.GetGeneration(), - ), - target, - ) + SetKonnectEntityProgrammedCondition(target) return nil } @@ -91,30 +69,11 @@ func updateTarget( }) if errWrapped := wrapErrIfKonnectOpFailed(err, UpdateOp, target); errWrapped != nil { - k8sutils.SetCondition( - k8sutils.NewConditionWithGeneration( - conditions.KonnectEntityProgrammedConditionType, - metav1.ConditionFalse, - "FailedToUpdate", - errWrapped.Error(), - target.GetGeneration(), - ), - target, - ) + SetKonnectEntityProgrammedConditionFalse(target, "FailedToUpdate", errWrapped.Error()) return errWrapped } - k8sutils.SetCondition( - k8sutils.NewConditionWithGeneration( - conditions.KonnectEntityProgrammedConditionType, - metav1.ConditionTrue, - conditions.KonnectEntityProgrammedReasonProgrammed, - "", - target.GetGeneration(), - ), - target, - ) - + SetKonnectEntityProgrammedCondition(target) return nil } diff --git a/controller/konnect/reconciler_generic.go b/controller/konnect/reconciler_generic.go index b712be850..26b806174 100644 --- a/controller/konnect/reconciler_generic.go +++ b/controller/konnect/reconciler_generic.go @@ -214,20 +214,28 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile( // If a type has a KongUpstream ref (KongTarget), handle it. res, err = handleKongUpstreamRef(ctx, r.Client, ent) if err != nil { - if !errors.As(err, &ReferencedKongUpstreamIsBeingDeleted{}) { - return ctrl.Result{}, err - } - - // If the referenced KongUpstream is being deleted (has a non zero deletion timestamp) - // then we remove the entity if it has not been deleted yet (deletion timestamp is zero). - // We do this because Konnect blocks deletion of entities like Services/Upstreams - // if they contain entities like Routes/Targets. - if ent.GetDeletionTimestamp().IsZero() { - if err := r.Client.Delete(ctx, ent); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to delete %s: %w", client.ObjectKeyFromObject(ent), err) + // If the referenced KongUpstream is not found or is being deleted + // and the object is being deleted, remove the finalizer and let the + // deletion proceed without trying to delete the entity from Konnect + // as the KongUpstream deletion will take care of it on the Konnect side. + if errors.As(err, &ReferencedKongUpstreamIsBeingDeleted{}) || + errors.As(err, &ReferencedKongUpstreamDoesNotExist{}) { + if !ent.GetDeletionTimestamp().IsZero() { + if controllerutil.RemoveFinalizer(ent, KonnectCleanupFinalizer) { + if err := r.Client.Update(ctx, ent); err != nil { + if k8serrors.IsConflict(err) { + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to remove finalizer %s: %w", KonnectCleanupFinalizer, err) + } + log.Debug(logger, "finalizer removed as the owning KongUpstream is being deleted or is already gone", ent, + "finalizer", KonnectCleanupFinalizer, + ) + } } - return ctrl.Result{}, nil } + + return ctrl.Result{}, err } else if res.Requeue { return res, nil } diff --git a/controller/konnect/reconciler_upstreamref.go b/controller/konnect/reconciler_upstreamref.go index 3a7ccb242..f3b6b1f26 100644 --- a/controller/konnect/reconciler_upstreamref.go +++ b/controller/konnect/reconciler_upstreamref.go @@ -63,7 +63,10 @@ func handleKongUpstreamRef[T constraints.SupportedKonnectEntityType, TEnt constr return res, errStatus } - return ctrl.Result{}, fmt.Errorf("can't get the referenced KongUpstream %s: %w", nn, err) + return ctrl.Result{}, ReferencedKongUpstreamDoesNotExist{ + Reference: nn, + Err: err, + } } // If referenced KongUpstream is being deleted, return an error so that we diff --git a/controller/konnect/reconciler_upstreamref_test.go b/controller/konnect/reconciler_upstreamref_test.go index 54997a989..20629ba2d 100644 --- a/controller/konnect/reconciler_upstreamref_test.go +++ b/controller/konnect/reconciler_upstreamref_test.go @@ -258,7 +258,7 @@ func TestHandleUpstreamRef(t *testing.T) { }, }, expectError: true, - expectErrorContains: "can't get the referenced KongUpstream", + expectErrorContains: "referenced Kong Upstream default/upstream-nonexist does not exist", updatedEntAssertions: []func(*configurationv1alpha1.KongTarget) (bool, string){ func(kt *configurationv1alpha1.KongTarget) (bool, string) { return lo.ContainsBy(kt.Status.Conditions, func(c metav1.Condition) bool {