Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enhance logging #969

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: enhance logging #969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions controllers/apim/apidefinition/controller.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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