Skip to content

Commit

Permalink
feat: enhance logging
Browse files Browse the repository at this point in the history 
see https://gravitee.atlassian.net/browse/GKO-68
  • Loading branch information
@a-cordier
a-cordier committed Dec 24, 2024
1 parent cb4226b commit e08c559
Show file tree
Hide file tree
Showing 25 changed files with 468 additions and 232 deletions.
23 changes: 11 additions & 12 deletions controllers/apim/apidefinition/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/errors"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/event"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/k8s"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/log"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/template"

util "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
Expand All @@ -33,7 +34,6 @@ import (

"github.com/gravitee-io/gravitee-kubernetes-operator/controllers/apim/apidefinition/internal"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/log"
)

const requeueAfterTime = time.Second * 5
Expand All @@ -53,7 +53,6 @@ func Reconcile(
}

func reconcileApiTemplate(ctx context.Context, apiDefinition core.ApiDefinitionObject) (ctrl.Result, error) {
logger := log.FromContext(ctx)
_, err := util.CreateOrUpdate(ctx, k8s.GetClient(), apiDefinition, func() error {
dc, _ := apiDefinition.DeepCopyObject().(core.ApiDefinitionObject)
if err := template.Compile(ctx, dc); err != nil {
Expand All @@ -70,26 +69,26 @@ func reconcileApiTemplate(ctx context.Context, apiDefinition core.ApiDefinitionO
})

if err != nil {
logger.Error(err, "Failed to sync API definition template")
log.Error(ctx, err, "Failed to sync API definition template", log.KeyValues(apiDefinition)...)
return ctrl.Result{RequeueAfter: requeueAfterTime}, err
}

logger.Info("template synced successfully.", "template:", apiDefinition.GetName())
log.Debug(ctx, "Ingress template synced successfully.", log.KeyValues(apiDefinition)...)

if err := internal.UpdateStatusSuccess(ctx, apiDefinition); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}

func reconcileApiDefinition(
ctx context.Context,
apiDefinition core.ApiDefinitionObject,
events *event.Recorder,
) (ctrl.Result, error) {
logger := log.FromContext(ctx)
dc, _ := apiDefinition.DeepCopyObject().(core.ApiDefinitionObject)

_, reconcileErr := util.CreateOrUpdate(ctx, k8s.GetClient(), apiDefinition, func() error {
_, err := util.CreateOrUpdate(ctx, k8s.GetClient(), apiDefinition, func() error {
util.AddFinalizer(apiDefinition, core.ApiDefinitionFinalizer)
k8s.AddAnnotation(apiDefinition, core.LastSpecHashAnnotation, apiDefinition.GetSpec().Hash())

Expand Down Expand Up @@ -120,20 +119,20 @@ func reconcileApiDefinition(
return ctrl.Result{}, err
}

if reconcileErr == nil {
logger.Info("API definition has been reconciled")
if err == nil {
log.InfoEndReconcile(ctx, apiDefinition)
return ctrl.Result{}, internal.UpdateStatusSuccess(ctx, apiDefinition)
}

if err := internal.UpdateStatusFailure(ctx, apiDefinition); err != nil {
return ctrl.Result{}, err
}

if errors.IsRecoverable(reconcileErr) {
logger.Error(reconcileErr, "Requeuing reconcile")
return ctrl.Result{RequeueAfter: requeueAfterTime}, reconcileErr
if errors.IsRecoverable(err) {
log.ErrorRequeuingReconcile(ctx, err, apiDefinition)
return ctrl.Result{RequeueAfter: requeueAfterTime}, err
}

logger.Error(reconcileErr, "Aborting reconcile")
log.ErrorAbortingReconcile(ctx, err, apiDefinition)
return ctrl.Result{}, nil
}
9 changes: 0 additions & 9 deletions controllers/apim/apidefinition/internal/api_resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/gravitee-io/gravitee-kubernetes-operator/api/v1alpha1"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/k8s"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/template"
"sigs.k8s.io/controller-runtime/pkg/log"
)

func resolveResources(ctx context.Context, resources []*base.ResourceOrRef) error {
Expand All @@ -46,14 +45,6 @@ func resolveIfRef(ctx context.Context, resourceOrRef *base.ResourceOrRef) error
namespacedName := resourceOrRef.Ref.NamespacedName()
resource := new(v1alpha1.ApiResource)

log.FromContext(ctx).Info(
"Looking for api resource from",
"namespace",
namespacedName.Namespace,
"name",
namespacedName.Name,
)

if err := k8s.GetClient().Get(ctx, namespacedName, resource); err != nil {
return err
}
Expand Down
14 changes: 6 additions & 8 deletions controllers/apim/apidefinition/internal/config_map.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import (
"github.com/gravitee-io/gravitee-kubernetes-operator/api/v1alpha1"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/core"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/k8s"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/log"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
)

const (
Expand All @@ -47,12 +47,12 @@ func updateConfigMap(
) error {
if api.Spec.State == base.StateStopped {
if err := deleteConfigMap(ctx, api); err != nil {
log.FromContext(ctx).Error(err, "Unable to delete ConfigMap from API definition")
log.Error(ctx, err, "Unable to delete config map for API definition", log.KeyValues(api)...)
return err
}
} else {
if err := saveConfigMap(ctx, api); err != nil {
log.FromContext(ctx).Error(err, "Unable to create or update ConfigMap from API definition")
log.Error(ctx, err, "Unable to create or update config map for API definition", log.KeyValues(api)...)
return err
}
}
Expand Down Expand Up @@ -126,21 +126,19 @@ func saveConfigMap(

err = k8s.GetClient().Get(ctx, lookupKey, currentApiDefinition)
if errors.IsNotFound(err) {
log.FromContext(ctx).Info("Creating config map for API.", "name", apiDefinition.GetName())
log.Debug(ctx, "Creating config map for API", log.KeyValues(apiDefinition)...)
return k8s.GetClient().Create(ctx, cm)
}

if err != nil {
return err
}

// Only update the config map if resource version has changed (means api definition has changed).
if currentApiDefinition.Data[definitionVersionKey] != apiDefinition.GetResourceVersion() {
log.FromContext(ctx).Info("Updating ConfigMap", "name", apiDefinition.GetName())
log.Debug(ctx, "Updating config map", log.KeyValues(apiDefinition)...)
return k8s.GetClient().Update(ctx, cm)
}

log.FromContext(ctx).Info("No change detected on API. Skipped.", "name", apiDefinition.GetName())
return nil
}

Expand All @@ -152,6 +150,6 @@ func deleteConfigMap(ctx context.Context, api client.Object) error {
},
}

log.FromContext(ctx).Info("Deleting Config Map associated to API if exists")
log.Debug(ctx, "Deleting any config map associated to the API", log.KeyValues(api)...)
return client.IgnoreNotFound(k8s.GetClient().Delete(ctx, configMap))
}
22 changes: 12 additions & 10 deletions controllers/apim/apidefinition/internal/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ import (
v4 "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/v4"

"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/gravitee-io/gravitee-kubernetes-operator/api/v1alpha1"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/apim"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/errors"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/log"
)

func CreateOrUpdate(ctx context.Context, apiDefinition client.Object) error {
Expand Down Expand Up @@ -63,7 +63,7 @@ func createOrUpdateV2(ctx context.Context, apiDefinition *v1alpha1.ApiDefinition
return nil
}

log.FromContext(ctx).Info("Syncing API with APIM")
log.Debug(ctx, "Syncing API definition with control plane", log.KeyValues(apiDefinition)...)

apimClient, apimErr := apim.FromContextRef(ctx, spec.Context, apiDefinition.GetNamespace())
if apimErr != nil {
Expand All @@ -88,6 +88,8 @@ func createOrUpdateV2(ctx context.Context, apiDefinition *v1alpha1.ApiDefinition
return err
}

log.Debug(ctx, "API successfully synced with control plane", log.KeyValues(apiDefinition)...)

return nil
}

Expand All @@ -97,14 +99,14 @@ func createOrUpdateV4(ctx context.Context, apiDefinition *v1alpha1.ApiV4Definiti
spec := &cp.Spec

if err := resolveResources(ctx, spec.Resources); err != nil {
log.FromContext(ctx).Error(err, "Unable to resolve API resources from references")
log.Error(ctx, err, "Unable to resolve API resources from references", log.KeyValues(apiDefinition)...)
return err
}

spec.DefinitionContext = v4.NewDefaultKubernetesContext().MergeWith(spec.DefinitionContext)

if spec.Context != nil {
log.FromContext(ctx).Info("Syncing API with APIM")
log.Debug(ctx, "Syncing API definition with control plane", log.KeyValues(apiDefinition)...)
apimClient, err := apim.FromContextRef(ctx, spec.Context, apiDefinition.GetNamespace())
if err != nil {
return err
Expand All @@ -117,22 +119,22 @@ func createOrUpdateV4(ctx context.Context, apiDefinition *v1alpha1.ApiV4Definiti
return err
}
apiDefinition.Status.Status = *status
log.FromContext(ctx).WithValues("id", spec.ID).Info("API successfully synced with APIM")
log.Debug(ctx, "API successfully synced with control plane", log.KeyValues(apiDefinition)...)
} else {
cp.PopulateIDs(nil)
}

if spec.DefinitionContext.SyncFrom == v4.OriginManagement || spec.State == base.StateStopped {
log.FromContext(ctx).Info(
"Deleting config map as API is not managed by operator or is stopped",
"syncFrom", spec.DefinitionContext.SyncFrom,
"state", spec.State,
log.Debug(
ctx,
"Deleting config map as API definition is not synced from the cluster or API is stopped",
log.KeyValues(apiDefinition, "state", spec.State, "synced-from", spec.DefinitionContext.SyncFrom)...,
)
if err := deleteConfigMap(ctx, cp); err != nil {
return err
}
} else {
log.FromContext(ctx).Info("Saving config map")
log.Debug(ctx, "Saving config map for API definition", log.KeyValues(apiDefinition)...)
if err := saveConfigMap(ctx, cp); err != nil {
return err
}
Expand Down
11 changes: 5 additions & 6 deletions controllers/apim/apiresource/apiresource_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"

"github.com/gravitee-io/gravitee-kubernetes-operator/internal/k8s"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/log"

"github.com/gravitee-io/gravitee-kubernetes-operator/internal/template"

Expand All @@ -33,7 +34,6 @@ import (
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/gravitee-io/gravitee-kubernetes-operator/api/v1alpha1"
"github.com/gravitee-io/gravitee-kubernetes-operator/controllers/apim/apiresource/internal"
Expand All @@ -56,7 +56,6 @@ type Reconciler struct {
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)
apiResource := &v1alpha1.ApiResource{}
if err := r.Get(ctx, req.NamespacedName, apiResource); err != nil {
return ctrl.Result{}, client.IgnoreNotFound(err)
Expand All @@ -66,7 +65,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu

dc := apiResource.DeepCopy()

_, reconcileErr := util.CreateOrUpdate(ctx, r.Client, apiResource, func() error {
_, err := util.CreateOrUpdate(ctx, r.Client, apiResource, func() error {
util.AddFinalizer(apiResource, core.ApiResourceFinalizer)
k8s.AddAnnotation(apiResource, core.LastSpecHashAnnotation, hash.Calculate(&apiResource.Spec))

Expand Down Expand Up @@ -97,13 +96,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
return ctrl.Result{}, err
}

if reconcileErr == nil {
logger.Info("API Resource has been reconciled")
if err == nil {
log.InfoEndReconcile(ctx, apiResource)
return ctrl.Result{}, nil
}

// There was an error reconciling the Management Context
return ctrl.Result{}, reconcileErr
return ctrl.Result{}, err
}

// SetupWithManager sets up the controller with the Manager.
Expand Down
24 changes: 13 additions & 11 deletions controllers/apim/application/application_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/hash"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/indexer"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/k8s"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/log"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/predicate"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/template"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/watch"
Expand All @@ -40,7 +41,6 @@ import (
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
)

const requeueAfterTime = time.Second * 5
Expand All @@ -61,8 +61,6 @@ type Reconciler struct {
// +kubebuilder:rbac:groups=gravitee.io,resources=applications/finalizers,verbs=update
// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)

application := &v1alpha1.Application{}
if err := r.Get(ctx, req.NamespacedName, application); err != nil {
return ctrl.Result{}, client.IgnoreNotFound(err)
Expand All @@ -71,13 +69,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
events := event.NewRecorder(r.Recorder)

if application.Spec.Context == nil {
logger.Error(fmt.Errorf("no context is provided, no attempt will be made to sync with APIM"), "Aborting reconcile")
log.ErrorAbortingReconcile(
ctx,
fmt.Errorf("no context is provided, no attempt will be made to sync with APIM"),
application,
)
return ctrl.Result{}, nil
}

dc := application.DeepCopy()

_, reconcileErr := util.CreateOrUpdate(ctx, r.Client, application, func() error {
_, err := util.CreateOrUpdate(ctx, r.Client, application, func() error {
util.AddFinalizer(application, core.ApplicationFinalizer)
k8s.AddAnnotation(application, core.LastSpecHashAnnotation, hash.Calculate(&application.Spec))

Expand Down Expand Up @@ -110,8 +112,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu

application.SetFinalizers(dc.GetFinalizers())

if reconcileErr == nil {
logger.Info("Application has been reconciled")
if err == nil {
log.InfoEndReconcile(ctx, application)
return ctrl.Result{}, internal.UpdateStatusSuccess(ctx, application)
}

Expand All @@ -120,12 +122,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
return ctrl.Result{}, err
}

if errors.IsRecoverable(reconcileErr) {
logger.Error(reconcileErr, "Requeuing reconcile")
return ctrl.Result{RequeueAfter: requeueAfterTime}, reconcileErr
if errors.IsRecoverable(err) {
log.ErrorRequeuingReconcile(ctx, err, application)
return ctrl.Result{RequeueAfter: requeueAfterTime}, err
}

logger.Error(reconcileErr, "Aborting reconcile")
log.ErrorAbortingReconcile(ctx, err, application)
return ctrl.Result{}, nil
}

Expand Down
Loading

0 comments on commit e08c559

Please sign in to comment.