Skip to content

Commit

Permalink
fix(konnect): make setting status conditions not update on every reco…
Browse files Browse the repository at this point in the history 
…nciliation
  • Loading branch information
@pmalek
pmalek committed Oct 21, 2024
1 parent 0129f25 commit 03aca68
Show file tree
Hide file tree
Showing 8 changed files with 399 additions and 75 deletions.
13 changes: 7 additions & 6 deletions controller/konnect/reconciler_certificateref.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/kong/gateway-operator/controller/konnect/constraints"
"github.com/kong/gateway-operator/controller/pkg/patch"
k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"

configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1"
Expand Down Expand Up @@ -50,7 +51,7 @@ func handleKongCertificateRef[T constraints.SupportedKonnectEntityType, TEnt con
}
err := cl.Get(ctx, nn, cert)
if err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KongCertificateRefValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -79,7 +80,7 @@ func handleKongCertificateRef[T constraints.SupportedKonnectEntityType, TEnt con
cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, cert)
if !ok || cond.Status != metav1.ConditionTrue {
ent.SetKonnectID("")
if res, err := updateStatusWithCondition(
if res, err := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KongCertificateRefValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -111,7 +112,7 @@ func handleKongCertificateRef[T constraints.SupportedKonnectEntityType, TEnt con
sni.Status.Konnect.CertificateID = cert.GetKonnectID()
}

if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KongCertificateRefValidConditionType,
metav1.ConditionTrue,
Expand All @@ -133,7 +134,7 @@ func handleKongCertificateRef[T constraints.SupportedKonnectEntityType, TEnt con
}
cp, err := getCPForRef(ctx, cl, cpRef, ent.GetNamespace())
if err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionFalse,
Expand All @@ -156,7 +157,7 @@ func handleKongCertificateRef[T constraints.SupportedKonnectEntityType, TEnt con

cond, ok = k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, cp)
if !ok || cond.Status != metav1.ConditionTrue || cond.ObservedGeneration != cp.GetGeneration() {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionFalse,
Expand All @@ -173,7 +174,7 @@ func handleKongCertificateRef[T constraints.SupportedKonnectEntityType, TEnt con
resource.SetControlPlaneID(cp.Status.ID)
}

if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionTrue,
Expand Down
68 changes: 18 additions & 50 deletions controller/konnect/reconciler_generic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/kong/gateway-operator/controller/konnect/constraints"
"github.com/kong/gateway-operator/controller/konnect/ops"
"github.com/kong/gateway-operator/controller/pkg/log"
"github.com/kong/gateway-operator/controller/pkg/patch"
"github.com/kong/gateway-operator/pkg/consts"
k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"

Expand Down Expand Up @@ -336,7 +337,7 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(
var apiAuth konnectv1alpha1.KonnectAPIAuthConfiguration
if err := r.Client.Get(ctx, apiAuthRef, &apiAuth); err != nil {
if k8serrors.IsNotFound(err) {
if res, err := updateStatusWithCondition(
if res, err := patch.StatusWithCondition(
ctx, r.Client, ent,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationResolvedRefConditionType,
metav1.ConditionFalse,
Expand All @@ -349,7 +350,7 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(
return ctrl.Result{}, nil
}

if res, err := updateStatusWithCondition(
if res, err := patch.StatusWithCondition(
ctx, r.Client, ent,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationResolvedRefConditionType,
metav1.ConditionFalse,
Expand All @@ -367,7 +368,7 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(
cond.Status != metav1.ConditionTrue ||
cond.ObservedGeneration != ent.GetGeneration() ||
cond.Reason != konnectv1alpha1.KonnectEntityAPIAuthConfigurationResolvedRefReasonResolvedRef {
if res, err := updateStatusWithCondition(
if res, err := patch.StatusWithCondition(
ctx, r.Client, ent,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationResolvedRefConditionType,
metav1.ConditionTrue,
Expand All @@ -386,7 +387,7 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(

// If it's invalid then set the "APIAuthValid" status condition on
// the entity to False with "Invalid" reason.
if res, err := updateStatusWithCondition(
if res, err := patch.StatusWithCondition(
ctx, r.Client, ent,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationValidConditionType,
metav1.ConditionFalse,
Expand All @@ -408,7 +409,7 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(
cond.ObservedGeneration != ent.GetGeneration() ||
cond.Message != conditionMessageReferenceKonnectAPIAuthConfigurationValid(apiAuthRef) {

if res, err := updateStatusWithCondition(
if res, err := patch.StatusWithCondition(
ctx, r.Client, ent,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationValidConditionType,
metav1.ConditionTrue,
Expand All @@ -422,7 +423,7 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(

token, err := getTokenFromKonnectAPIAuthConfiguration(ctx, r.Client, &apiAuth)
if err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, r.Client, &apiAuth,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -458,7 +459,7 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(

if controllerutil.RemoveFinalizer(ent, KonnectCleanupFinalizer) {
if err := ops.Delete[T, TEnt](ctx, sdk, r.Client, ent); err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, r.Client, ent,
konnectv1alpha1.KonnectEntityProgrammedConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -522,7 +523,7 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(
}

// Regardless of the error, set the status as it can contain the Konnect ID
// and/or status conditions set in Create() and that ID will be needed to
// and/or status ops set in Create() and that ID will be needed to
// update/delete the object in Konnect.
if err := r.Client.Status().Patch(ctx, ent, client.MergeFrom(obj)); err != nil {
if k8serrors.IsConflict(err) {
Expand Down Expand Up @@ -589,39 +590,6 @@ type EntityWithControlPlaneRef interface {
GetControlPlaneID() string
}

func updateStatusWithCondition[T interface {
client.Object
k8sutils.ConditionsAware
}](
ctx context.Context,
client client.Client,
ent T,
conditionType consts.ConditionType,
conditionStatus metav1.ConditionStatus,
conditionReason consts.ConditionReason,
conditionMessage string,
) (ctrl.Result, error) {
k8sutils.SetCondition(
k8sutils.NewConditionWithGeneration(
conditionType,
conditionStatus,
conditionReason,
conditionMessage,
ent.GetGeneration(),
),
ent,
)

if err := client.Status().Update(ctx, ent); err != nil {
if k8serrors.IsConflict(err) {
return ctrl.Result{Requeue: true}, nil
}
return ctrl.Result{}, fmt.Errorf("failed to update status with %s condition: %w", conditionType, err)
}

return ctrl.Result{}, nil
}

func getCPForRef(
ctx context.Context,
cl client.Client,
Expand Down Expand Up @@ -819,7 +787,7 @@ func handleKongConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constr
}

if err := cl.Get(ctx, nn, &consumer); err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KongConsumerRefValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -847,7 +815,7 @@ func handleKongConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constr
cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, &consumer)
if !ok || cond.Status != metav1.ConditionTrue {
ent.SetKonnectID("")
if res, err := updateStatusWithCondition(
if res, err := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KongConsumerRefValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -882,7 +850,7 @@ func handleKongConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constr
)
}

if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KongConsumerRefValidConditionType,
metav1.ConditionTrue,
Expand All @@ -901,7 +869,7 @@ func handleKongConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constr
}
cp, err := getCPForRef(ctx, cl, cpRef, ent.GetNamespace())
if err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionFalse,
Expand All @@ -921,7 +889,7 @@ func handleKongConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constr

cond, ok = k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, cp)
if !ok || cond.Status != metav1.ConditionTrue || cond.ObservedGeneration != cp.GetGeneration() {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionFalse,
Expand All @@ -938,7 +906,7 @@ func handleKongConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constr
resource.SetControlPlaneID(cp.Status.ID)
}

if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionTrue,
Expand Down Expand Up @@ -1047,7 +1015,7 @@ func handleControlPlaneRef[T constraints.SupportedKonnectEntityType, TEnt constr
nn.Namespace = cpRef.KonnectNamespacedRef.Namespace
}
if err := cl.Get(ctx, nn, &cp); err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionFalse,
Expand All @@ -1067,7 +1035,7 @@ func handleControlPlaneRef[T constraints.SupportedKonnectEntityType, TEnt constr

cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, &cp)
if !ok || cond.Status != metav1.ConditionTrue || cond.ObservedGeneration != cp.GetGeneration() {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -1111,7 +1079,7 @@ func handleControlPlaneRef[T constraints.SupportedKonnectEntityType, TEnt constr
resource.SetControlPlaneID(cp.Status.ID)
}

if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.ControlPlaneRefValidConditionType,
metav1.ConditionTrue,
Expand Down
9 changes: 5 additions & 4 deletions controller/konnect/reconciler_keysetref.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"

"github.com/kong/gateway-operator/controller/konnect/constraints"
"github.com/kong/gateway-operator/controller/pkg/patch"
k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"

configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1"
Expand All @@ -34,7 +35,7 @@ func handleKongKeySetRef[T constraints.SupportedKonnectEntityType, TEnt constrai
if key.Status.Konnect != nil && key.Status.Konnect.GetKeySetID() != "" {
// Reset the KeySetID in the status and set the condition to True.
key.Status.Konnect.KeySetID = ""
if res, err := updateStatusWithCondition(ctx, cl, key,
if res, err := patch.StatusWithCondition(ctx, cl, key,
konnectv1alpha1.KeySetRefValidConditionType,
metav1.ConditionTrue,
konnectv1alpha1.KeySetRefReasonValid,
Expand Down Expand Up @@ -70,7 +71,7 @@ func handleKongKeySetRef[T constraints.SupportedKonnectEntityType, TEnt constrai
Namespace: ent.GetNamespace(),
}
if err := cl.Get(ctx, nn, &keySet); err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KeySetRefValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -102,7 +103,7 @@ func handleKongKeySetRef[T constraints.SupportedKonnectEntityType, TEnt constrai
// Verify that the KongKeySet is programmed.
cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, &keySet)
if !ok || cond.Status != metav1.ConditionTrue {
if res, err := updateStatusWithCondition(
if res, err := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KeySetRefValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -133,7 +134,7 @@ func handleKongKeySetRef[T constraints.SupportedKonnectEntityType, TEnt constrai
key.Status.Konnect.KeySetID = keySet.Status.Konnect.GetKonnectID()
}

if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KeySetRefValidConditionType,
metav1.ConditionTrue,
Expand Down
7 changes: 4 additions & 3 deletions controller/konnect/reconciler_konnectapiauth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/kong/gateway-operator/controller/konnect/ops"
"github.com/kong/gateway-operator/controller/pkg/log"
"github.com/kong/gateway-operator/controller/pkg/patch"
k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"

konnectv1alpha1 "github.com/kong/kubernetes-configuration/api/konnect/v1alpha1"
Expand Down Expand Up @@ -114,7 +115,7 @@ func (r *KonnectAPIAuthConfigurationReconciler) Reconcile(

token, err := getTokenFromKonnectAPIAuthConfiguration(ctx, r.client, &apiAuth)
if err != nil {
if res, errStatus := updateStatusWithCondition(
if res, errStatus := patch.StatusWithCondition(
ctx, r.client, &apiAuth,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -154,7 +155,7 @@ func (r *KonnectAPIAuthConfigurationReconciler) Reconcile(
apiAuth.Status.OrganizationID = ""
apiAuth.Status.ServerURL = serverURL.String()

res, errUpdate := updateStatusWithCondition(
res, errUpdate := patch.StatusWithCondition(
ctx, r.client, &apiAuth,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationValidConditionType,
metav1.ConditionFalse,
Expand Down Expand Up @@ -192,7 +193,7 @@ func (r *KonnectAPIAuthConfigurationReconciler) Reconcile(
apiAuth.Status.OrganizationID = *respOrg.MeOrganization.ID
apiAuth.Status.ServerURL = serverURL.String()

res, err := updateStatusWithCondition(
res, err := patch.StatusWithCondition(
ctx, r.client, &apiAuth,
konnectv1alpha1.KonnectEntityAPIAuthConfigurationValidConditionType,
metav1.ConditionTrue,
Expand Down
Loading

0 comments on commit 03aca68

Please sign in to comment.