Skip to content

Commit

Permalink
chore(logging) create logging package (#6304)
Browse files Browse the repository at this point in the history
* chore(*) create logging package

Move log utilities out of util into their own package, to avoid import
cycles.

* chore(*) update generator and re-generate code

---------

Co-authored-by: Travis Raines <[email protected]>
  • Loading branch information
randmonkey and rainest committed Jul 24, 2024
1 parent 44c2f86 commit cea1315
Show file tree
Hide file tree
Showing 34 changed files with 240 additions and 230 deletions.
18 changes: 9 additions & 9 deletions controllers/license/konglicense_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (

"github.com/kong/kubernetes-ingress-controller/v3/internal/controllers"
"github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/crds"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
"github.com/kong/kubernetes-ingress-controller/v3/internal/logging"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util/kubernetes/object/status"
kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1"
)
Expand Down Expand Up @@ -183,7 +183,7 @@ func (r *KongV1Alpha1KongLicenseReconciler) Reconcile(ctx context.Context, req c
}
if objectExistsInCache {
// Delete the object in the cache first.
log.V(util.DebugLevel).Info("KongLicense deleted in cluster, delete it in cache")
log.V(logging.DebugLevel).Info("KongLicense deleted in cluster, delete it in cache")
if err := r.LicenseCache.Delete(obj); err != nil {
return ctrl.Result{}, err
}
Expand All @@ -197,11 +197,11 @@ func (r *KongV1Alpha1KongLicenseReconciler) Reconcile(ctx context.Context, req c
}
return ctrl.Result{}, err
}
log.V(util.DebugLevel).Info("Reconciling resource", "name", req.Name)
log.V(logging.DebugLevel).Info("Reconciling resource", "name", req.Name)

// clean the object up if it's being deleted
if !obj.DeletionTimestamp.IsZero() && time.Now().After(obj.DeletionTimestamp.Time) {
log.V(util.DebugLevel).Info("Resource is being deleted, its configuration will be removed", "type", "KongLicense")
log.V(logging.DebugLevel).Info("Resource is being deleted, its configuration will be removed", "type", "KongLicense")

_, objectExistsInCache, err := r.LicenseCache.Get(obj)
if err != nil {
Expand Down Expand Up @@ -230,15 +230,15 @@ func (r *KongV1Alpha1KongLicenseReconciler) Reconcile(ctx context.Context, req c
// Trigger a compare on stored KongLicenses in cache and pick the newest.
chosenLicense := r.pickLicenseInCache()
if chosenLicense.Name == obj.Name {
log.V(util.DebugLevel).Info("Picked KongLicense being reconciled", "name", obj.Name)
log.V(logging.DebugLevel).Info("Picked KongLicense being reconciled", "name", obj.Name)
err := r.ensureControllerStatusConditions(ctx, obj, metav1.ConditionTrue, ConditionReasonPickedAsLatest, "")
if err != nil {
return ctrl.Result{}, err
}

oldChosenLicense := r.getChosenLicense()
if oldChosenLicense != nil && oldChosenLicense.Name != chosenLicense.Name {
r.Log.V(util.DebugLevel).Info("Originally picked KongLicense replaced", "name", oldChosenLicense.Name)
r.Log.V(logging.DebugLevel).Info("Originally picked KongLicense replaced", "name", oldChosenLicense.Name)
err := r.ensureControllerStatusConditions(ctx, oldChosenLicense, metav1.ConditionFalse, ConditionReasonReplacedByNewer, "Replaced by newer created KongLicense")
if err != nil {
return ctrl.Result{}, err
Expand All @@ -264,10 +264,10 @@ type License struct {
func (r *KongV1Alpha1KongLicenseReconciler) GetValidatedLicense() mo.Option[License] {
chosenLicense := r.getChosenLicense()
if chosenLicense == nil {
r.Log.V(util.DebugLevel).Info("No KongLicense available")
r.Log.V(logging.DebugLevel).Info("No KongLicense available")
return mo.None[License]()
}
r.Log.V(util.DebugLevel).Info("Get license from KongLicense resource", "name", chosenLicense.Name)
r.Log.V(logging.DebugLevel).Info("Get license from KongLicense resource", "name", chosenLicense.Name)
isValid := mo.None[bool]()
if r.licenseValidator != nil {
isValid = mo.Some(r.licenseValidator(chosenLicense.RawLicenseString) == nil)
Expand Down Expand Up @@ -362,7 +362,7 @@ func (r *KongV1Alpha1KongLicenseReconciler) repickLicenseOnDelete(ctx context.Co
chosenLicense := r.pickLicenseInCache()
r.setChosenLicense(chosenLicense)
if chosenLicense != nil {
r.Log.V(util.DebugLevel).Info("Picked KongLicense remaining in cache", "name", chosenLicense.Name)
r.Log.V(logging.DebugLevel).Info("Picked KongLicense remaining in cache", "name", chosenLicense.Name)
return r.ensureControllerStatusConditions(ctx, chosenLicense, metav1.ConditionTrue, ConditionReasonPickedAsLatest, "")
}
}
Expand Down
24 changes: 12 additions & 12 deletions hack/generators/controllers/networking/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ import (
ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference"
ctrlutils "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/utils"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
"github.com/kong/kubernetes-ingress-controller/v3/internal/logging"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util/kubernetes/object/status"
kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
Expand Down Expand Up @@ -632,11 +632,11 @@ func (r *{{.PackageAlias}}{{.Kind}}Reconciler) Reconcile(ctx context.Context, re
}
return ctrl.Result{}, err
}
log.V(util.DebugLevel).Info("Reconciling resource", "namespace", req.Namespace, "name", req.Name)
log.V(logging.DebugLevel).Info("Reconciling resource", "namespace", req.Namespace, "name", req.Name)
// clean the object up if it's being deleted
if !obj.DeletionTimestamp.IsZero() && time.Now().After(obj.DeletionTimestamp.Time) {
log.V(util.DebugLevel).Info("Resource is being deleted, its configuration will be removed", "type", "{{.Kind}}", "namespace", req.Namespace, "name", req.Name)
log.V(logging.DebugLevel).Info("Resource is being deleted, its configuration will be removed", "type", "{{.Kind}}", "namespace", req.Namespace, "name", req.Name)
{{if .NeedsUpdateReferences}}
// remove reference record where the {{.Kind}} is the referrer
if err := ctrlref.DeleteReferencesByReferrer(r.ReferenceIndexers, r.DataplaneClient, obj); err != nil {
Expand All @@ -663,16 +663,16 @@ func (r *{{.PackageAlias}}{{.Kind}}Reconciler) Reconcile(ctx context.Context, re
// used the class annotation and did not create a corresponding IngressClass. We only need this to determine
// if the IngressClass is default or to configure default settings, and can assume no/no additional defaults
// if none exists.
log.V(util.DebugLevel).Info("Could not retrieve IngressClass", "ingressclass", r.IngressClassName)
log.V(logging.DebugLevel).Info("Could not retrieve IngressClass", "ingressclass", r.IngressClassName)
}
}
// if the object is not configured with our ingress.class, then we need to ensure it's removed from the cache
if !ctrlutils.MatchesIngressClass(obj, r.IngressClassName, ctrlutils.IsDefaultIngressClass(class)) {
log.V(util.DebugLevel).Info("Object missing ingress class, ensuring it's removed from configuration",
log.V(logging.DebugLevel).Info("Object missing ingress class, ensuring it's removed from configuration",
"namespace", req.Namespace, "name", req.Name, "class", r.IngressClassName)
return ctrl.Result{}, r.DataplaneClient.DeleteObject(obj)
} else {
log.V(util.DebugLevel).Info("Object has matching ingress class", "namespace", req.Namespace, "name", req.Name,
log.V(logging.DebugLevel).Info("Object has matching ingress class", "namespace", req.Namespace, "name", req.Name,
"class", r.IngressClassName)
}
{{end}}
Expand Down Expand Up @@ -704,28 +704,28 @@ func (r *{{.PackageAlias}}{{.Kind}}Reconciler) Reconcile(ctx context.Context, re
// if status updates are enabled report the status for the object
if r.DataplaneClient.AreKubernetesObjectReportsEnabled() {
{{- if .IngressAddressUpdatesEnabled }}
log.V(util.DebugLevel).Info("Determining whether data-plane configuration has succeeded", "namespace", req.Namespace, "name", req.Name)
log.V(logging.DebugLevel).Info("Determining whether data-plane configuration has succeeded", "namespace", req.Namespace, "name", req.Name)
if !r.DataplaneClient.KubernetesObjectIsConfigured(obj) {
log.V(util.DebugLevel).Info("Resource not yet configured in the data-plane", "namespace", req.Namespace, "name", req.Name)
log.V(logging.DebugLevel).Info("Resource not yet configured in the data-plane", "namespace", req.Namespace, "name", req.Name)
return ctrl.Result{Requeue: true}, nil // requeue until the object has been properly configured
}
log.V(util.DebugLevel).Info("Determining gateway addresses for object status updates", "namespace", req.Namespace, "name", req.Name)
log.V(logging.DebugLevel).Info("Determining gateway addresses for object status updates", "namespace", req.Namespace, "name", req.Name)
addrs, err := r.DataplaneAddressFinder.GetLoadBalancerAddresses(ctx)
if err != nil {
return ctrl.Result{}, err
}
log.V(util.DebugLevel).Info("Found addresses for data-plane updating object status", "namespace", req.Namespace, "name", req.Name)
log.V(logging.DebugLevel).Info("Found addresses for data-plane updating object status", "namespace", req.Namespace, "name", req.Name)
updateNeeded, err := ctrlutils.UpdateLoadBalancerIngress(obj, addrs)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to update load balancer address: %w", err)
}
{{- end }}
{{- if .ProgrammedCondition.UpdatesEnabled }}
log.V(util.DebugLevel).Info("Updating programmed condition status", "namespace", req.Namespace, "name", req.Name)
log.V(logging.DebugLevel).Info("Updating programmed condition status", "namespace", req.Namespace, "name", req.Name)
configurationStatus := r.DataplaneClient.KubernetesObjectConfigurationStatus(obj)
conditions, updateNeeded := ctrlutils.EnsureProgrammedCondition(
configurationStatus,
Expand All @@ -740,7 +740,7 @@ func (r *{{.PackageAlias}}{{.Kind}}Reconciler) Reconcile(ctx context.Context, re
if updateNeeded {
return ctrl.Result{}, r.Status().Update(ctx, obj)
}
log.V(util.DebugLevel).Info("Status update not needed", "namespace", req.Namespace, "name", req.Name)
log.V(logging.DebugLevel).Info("Status update not needed", "namespace", req.Namespace, "name", req.Name)
}
{{- end}}
Expand Down
4 changes: 2 additions & 2 deletions internal/admission/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference"
"github.com/kong/kubernetes-ingress-controller/v3/internal/labels"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
"github.com/kong/kubernetes-ingress-controller/v3/internal/logging"
kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
)
Expand Down Expand Up @@ -414,7 +414,7 @@ func TestHandleSecret(t *testing.T) {
Operation: admissionv1.Update,
}

logger := testr.NewWithOptions(t, testr.Options{Verbosity: util.DebugLevel})
logger := testr.NewWithOptions(t, testr.Options{Verbosity: logging.DebugLevel})
referenceIndexer := ctrlref.NewCacheIndexers(logger)

handler := RequestHandler{
Expand Down
9 changes: 5 additions & 4 deletions internal/admission/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/kongstate"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/logging"
"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
Expand Down Expand Up @@ -237,12 +238,12 @@ func (validator KongHTTPValidator) ValidateConsumerGroup(
}
info, err := infoSvc.Get(ctx)
if err != nil {
validator.Logger.V(util.DebugLevel).Info("Failed to fetch Kong info", "error", err)
validator.Logger.V(logging.DebugLevel).Info("Failed to fetch Kong info", "error", err)
return false, ErrTextAdminAPIUnavailable, nil
}
version, err := kong.NewVersion(info.Version)
if err != nil {
validator.Logger.V(util.DebugLevel).Info("Failed to parse Kong version", "error", err)
validator.Logger.V(logging.DebugLevel).Info("Failed to parse Kong version", "error", err)
} else if !version.IsKongGatewayEnterprise() {
return false, ErrTextConsumerGroupUnsupported, nil
}
Expand Down Expand Up @@ -637,15 +638,15 @@ func (validator KongHTTPValidator) ValidateCustomEntity(ctx context.Context, ent
schemaService, hasClient := validator.AdminAPIServicesProvider.GetSchemasService()
// Skip validation on Kong gateway if we do not have available client.
if !hasClient {
logger.V(util.DebugLevel).Info("Skipped because no schema service available")
logger.V(logging.DebugLevel).Info("Skipped because no schema service available")
return true, "", nil
}

// Retrieve the schema of the entity from Kong gateway.
entityType := entity.Spec.EntityType
schema, err := schemaService.Get(ctx, entityType)
if err != nil {
logger.V(util.DebugLevel).Info("Failed to get schema of entity", "entity_type", entityType, "error", err)
logger.V(logging.DebugLevel).Info("Failed to get schema of entity", "entity_type", entityType, "error", err)
return false, fmt.Sprintf(ErrTextCustomEntityGetSchemaFailed, entityType, err), nil
}

Expand Down
12 changes: 6 additions & 6 deletions internal/clients/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
dpconf "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/config"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
"github.com/kong/kubernetes-ingress-controller/v3/internal/logging"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util/clock"
)

Expand Down Expand Up @@ -246,7 +246,7 @@ func (c *AdminAPIClientsManager) gatewayClientsReconciliationLoop() {
for {
select {
case <-c.ctx.Done():
c.logger.V(util.InfoLevel).Info("Closing AdminAPIClientsManager", "reason", c.ctx.Err())
c.logger.V(logging.InfoLevel).Info("Closing AdminAPIClientsManager", "reason", c.ctx.Err())
c.closeGatewayClientsSubscribers()
return
case discoveredAdminAPIs := <-c.discoveredAdminAPIsNotifyChan:
Expand All @@ -261,7 +261,7 @@ func (c *AdminAPIClientsManager) gatewayClientsReconciliationLoop() {
// It will adjust lists of gateway clients and notify subscribers about the change if readyGatewayClients list has
// changed.
func (c *AdminAPIClientsManager) onDiscoveredAdminAPIsNotification(discoveredAdminAPIs []adminapi.DiscoveredAdminAPI) {
c.logger.V(util.DebugLevel).Info("Received notification about Admin API addresses change")
c.logger.V(logging.DebugLevel).Info("Received notification about Admin API addresses change")

clientsChanged := c.adjustGatewayClients(discoveredAdminAPIs)
readinessChanged := c.reconcileGatewayClientsReadiness()
Expand All @@ -273,7 +273,7 @@ func (c *AdminAPIClientsManager) onDiscoveredAdminAPIsNotification(discoveredAdm
// onReadinessReconciliationTick is called on every readinessReconciliationTicker tick. It will reconcile readiness
// of all gateway clients and notify subscribers about the change if readyGatewayClients list has changed.
func (c *AdminAPIClientsManager) onReadinessReconciliationTick() {
c.logger.V(util.DebugLevel).Info("Reconciling readiness of gateway clients")
c.logger.V(logging.DebugLevel).Info("Reconciling readiness of gateway clients")

if changed := c.reconcileGatewayClientsReadiness(); changed {
c.notifyGatewayClientsSubscribers()
Expand Down Expand Up @@ -370,11 +370,11 @@ func (c *AdminAPIClientsManager) reconcileGatewayClientsReadiness() bool {

// notifyGatewayClientsSubscribers sends notifications to all subscribers that have called SubscribeToGatewayClientsChanges.
func (c *AdminAPIClientsManager) notifyGatewayClientsSubscribers() {
c.logger.V(util.DebugLevel).Info("Notifying subscribers about gateway clients change")
c.logger.V(logging.DebugLevel).Info("Notifying subscribers about gateway clients change")
for _, sub := range c.gatewayClientsChangesSubscribers {
select {
case <-c.ctx.Done():
c.logger.V(util.InfoLevel).Info("Not sending notification to subscribers as the context is done")
c.logger.V(logging.InfoLevel).Info("Not sending notification to subscribers as the context is done")
return
case sub <- struct{}{}:
}
Expand Down
10 changes: 5 additions & 5 deletions internal/clients/readiness.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
k8stypes "k8s.io/apimachinery/pkg/types"

"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
"github.com/kong/kubernetes-ingress-controller/v3/internal/logging"
)

const (
Expand Down Expand Up @@ -94,7 +94,7 @@ func (c DefaultReadinessChecker) checkPendingClient(
pendingClient adminapi.DiscoveredAdminAPI,
) (client *adminapi.Client) {
defer func() {
c.logger.V(util.DebugLevel).
c.logger.V(logging.DebugLevel).
Info(fmt.Sprintf("Checking readiness of pending client for %q", pendingClient.Address),
"ok", client != nil,
)
Expand All @@ -105,7 +105,7 @@ func (c DefaultReadinessChecker) checkPendingClient(
client, err := c.factory.CreateAdminAPIClient(ctx, pendingClient)
if err != nil {
// Despite the error reason we still want to keep the client in the pending list to retry later.
c.logger.V(util.DebugLevel).Info("Pending client is not ready yet",
c.logger.V(logging.DebugLevel).Info("Pending client is not ready yet",
"reason", err.Error(),
"address", pendingClient.Address,
)
Expand Down Expand Up @@ -142,7 +142,7 @@ func (c DefaultReadinessChecker) checkAlreadyExistingClients(ctx context.Context

func (c DefaultReadinessChecker) checkAlreadyCreatedClient(ctx context.Context, client AlreadyCreatedClient) (ready bool) {
defer func() {
c.logger.V(util.DebugLevel).Info(
c.logger.V(logging.DebugLevel).Info(
fmt.Sprintf("Checking readiness of already created client for %q", client.BaseRootURL()),
"ok", ready,
)
Expand All @@ -152,7 +152,7 @@ func (c DefaultReadinessChecker) checkAlreadyCreatedClient(ctx context.Context,
defer cancel()
if err := client.IsReady(ctx); err != nil {
// Despite the error reason we still want to keep the client in the pending list to retry later.
c.logger.V(util.DebugLevel).Info(
c.logger.V(logging.DebugLevel).Info(
"Already created client is not ready, moving to pending",
"address", client.BaseRootURL(),
"reason", err.Error(),
Expand Down
6 changes: 3 additions & 3 deletions internal/controllers/configuration/kongadminapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (

"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/controllers"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
"github.com/kong/kubernetes-ingress-controller/v3/internal/logging"
)

// KongAdminAPIServiceReconciler reconciles Kong Admin API Service Endpointslices
Expand Down Expand Up @@ -126,7 +126,7 @@ func (r *KongAdminAPIServiceReconciler) Reconcile(ctx context.Context, req ctrl.
r.Log.Info("Reconciling Admin API EndpointSlice", "namespace", req.Namespace, "name", req.Name)

if !endpoints.DeletionTimestamp.IsZero() {
r.Log.V(util.DebugLevel).Info("EndpointSlice is being deleted",
r.Log.V(logging.DebugLevel).Info("EndpointSlice is being deleted",
"type", "EndpointSlice", "namespace", req.Namespace, "name", req.Name,
)

Expand Down Expand Up @@ -177,7 +177,7 @@ func (r *KongAdminAPIServiceReconciler) Reconcile(ctx context.Context, req ctrl.
func (r *KongAdminAPIServiceReconciler) notify() {
discovered := flattenDiscoveredAdminAPIs(r.Cache)
addresses := lo.Map(discovered, func(d adminapi.DiscoveredAdminAPI, _ int) string { return d.Address })
r.Log.V(util.DebugLevel).
r.Log.V(logging.DebugLevel).
Info("Notifying about newly detected Admin APIs", "admin_apis", addresses)
r.EndpointsNotifier.Notify(discovered)
}
Expand Down
Loading

0 comments on commit cea1315

Please sign in to comment.