Skip to content

Commit

Permalink
refactor: replace glog in externaldns & certmanager (#6586)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pdabelf5
pdabelf5 authored Oct 3, 2024
1 parent 3253295 commit f034346
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 58 deletions.
30 changes: 18 additions & 12 deletions internal/certmanager/cm_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
cm_informers "github.com/cert-manager/cert-manager/pkg/client/informers/externalversions"
cmlisters "github.com/cert-manager/cert-manager/pkg/client/listers/certmanager/v1"
controllerpkg "github.com/cert-manager/cert-manager/pkg/controller"
"github.com/golang/glog"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/runtime"
kubeinformers "k8s.io/client-go/informers"
Expand All @@ -36,6 +35,7 @@ import (
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/workqueue"

nl "github.com/nginxinc/kubernetes-ingress/internal/logger"
conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
k8s_nginx "github.com/nginxinc/kubernetes-ingress/pkg/client/clientset/versioned"
vsinformers "github.com/nginxinc/kubernetes-ingress/pkg/client/informers/externalversions"
Expand Down Expand Up @@ -136,7 +136,8 @@ func (c *CmController) addHandlers(nsi *namespacedInformer) {
}

func (c *CmController) processItem(ctx context.Context, key string) error {
glog.V(3).Infof("processing virtual server resource ")
l := nl.LoggerFromContext(ctx)
nl.Debugf(l, "processing virtual server resource ")
namespace, name, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
runtime.HandleError(fmt.Errorf("invalid resource key: %s", key))
Expand Down Expand Up @@ -236,7 +237,9 @@ func (c *CmController) Run(stopCh <-chan struct{}) {
ctx, cancel := context.WithCancel(c.ctx)
defer cancel()

glog.Infof("Starting cert-manager control loop")
l := nl.LoggerFromContext(ctx)

nl.Info(l, "Starting cert-manager control loop")

var mustSync []cache.InformerSynced
for _, ig := range c.informerGroup {
Expand All @@ -245,17 +248,17 @@ func (c *CmController) Run(stopCh <-chan struct{}) {
}
// wait for all the informer caches we depend on are synced

glog.V(3).Infof("Waiting for %d caches to sync", len(mustSync))
nl.Debugf(l, "Waiting for %d caches to sync", len(mustSync))
if !cache.WaitForNamedCacheSync(ControllerName, stopCh, mustSync...) {
glog.Fatal("error syncing cm queue")
nl.Fatal(l, "error syncing cm queue")
}

glog.V(3).Infof("Queue is %v", c.queue.Len())
nl.Debugf(l, "Queue is %v", c.queue.Len())

go c.runWorker(ctx)

<-stopCh
glog.V(3).Infof("shutting down queue as workqueue signaled shutdown")
nl.Debugf(l, "shutting down queue as workqueue signaled shutdown")
for _, ig := range c.informerGroup {
ig.stop()
}
Expand All @@ -276,7 +279,8 @@ func (nsi *namespacedInformer) stop() {
// processItem function in order to read and process a message on the
// workqueue.
func (c *CmController) runWorker(ctx context.Context) {
glog.V(3).Infof("processing items on the workqueue")
l := nl.LoggerFromContext(ctx)
nl.Debugf(l, "processing items on the workqueue")
for {
obj, shutdown := c.queue.Get()
if shutdown {
Expand All @@ -294,19 +298,20 @@ func (c *CmController) runWorker(ctx context.Context) {

err := c.processItem(ctx, key)
if err != nil {
glog.V(3).Infof("Re-queuing item due to error processing: %v", err)
nl.Debugf(l, "Re-queuing item due to error processing: %v", err)
c.queue.AddRateLimited(obj)
return
}
glog.V(3).Infof("finished processing work item")
nl.Debugf(l, "finished processing work item")
c.queue.Forget(obj)
}()
}
}

// AddNewNamespacedInformer adds watchers for a new namespace
func (c *CmController) AddNewNamespacedInformer(ns string) {
glog.V(3).Infof("Adding or Updating cert-manager Watchers for Namespace: %v", ns)
l := nl.LoggerFromContext(c.ctx)
nl.Debugf(l, "Adding or Updating cert-manager Watchers for Namespace: %v", ns)
nsi := getNamespacedInformer(ns, c.informerGroup)
if nsi == nil {
nsi = c.newNamespacedInformer(ns)
Expand All @@ -319,7 +324,8 @@ func (c *CmController) AddNewNamespacedInformer(ns string) {

// RemoveNamespacedInformer removes watchers for a namespace we are no longer watching
func (c *CmController) RemoveNamespacedInformer(ns string) {
glog.V(3).Infof("Deleting cert-manager Watchers for Deleted Namespace: %v", ns)
l := nl.LoggerFromContext(c.ctx)
nl.Debugf(l, "Deleting cert-manager Watchers for Deleted Namespace: %v", ns)
nsi := getNamespacedInformer(ns, c.informerGroup)
if nsi != nil {
nsi.lock.Lock()
Expand Down
26 changes: 15 additions & 11 deletions internal/certmanager/sync.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ import (
cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
clientset "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned"
cmlisters "github.com/cert-manager/cert-manager/pkg/client/listers/certmanager/v1"
"github.com/golang/glog"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/tools/record"

nl "github.com/nginxinc/kubernetes-ingress/internal/logger"
vsapi "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
)

Expand Down Expand Up @@ -67,19 +67,20 @@ func SyncFnFor(
if vs.Spec.TLS == nil || vs.Spec.TLS.CertManager == nil {
return nil
}
l := nl.LoggerFromContext(ctx)
issuerName, issuerKind, issuerGroup, err := issuerForVirtualServer(vs)
if err != nil {
glog.Errorf("Failed to determine issuer to be used for VirtualServer resource: %v", err)
nl.Errorf(l, "Failed to determine issuer to be used for VirtualServer resource: %v", err)
rec.Eventf(vs, corev1.EventTypeWarning, reasonBadConfig, "Could not determine issuer for virtual server due to bad config: %s",
err)
return err
}

nsi := getNamespacedInformer(vs.GetNamespace(), ig)

newCrts, updateCrts, err := buildCertificates(nsi.cmLister, vs, issuerName, issuerKind, issuerGroup)
newCrts, updateCrts, err := buildCertificates(ctx, nsi.cmLister, vs, issuerName, issuerKind, issuerGroup)
if err != nil {
glog.Errorf("Incorrect cert-manager configuration for VirtualServer resource: %v", err)
nl.Errorf(l, "Incorrect cert-manager configuration for VirtualServer resource: %v", err)
rec.Eventf(vs, corev1.EventTypeWarning, reasonBadConfig, "Incorrect cert-manager configuration for VirtualServer resource: %s",
err)
return err
Expand All @@ -88,7 +89,7 @@ func SyncFnFor(
for _, crt := range newCrts {
_, err := cmClient.CertmanagerV1().Certificates(crt.Namespace).Create(ctx, crt, metav1.CreateOptions{})
if err != nil {
glog.Errorf("Error issuing Certificate for VirtualServer resource: %v", err)
nl.Errorf(l, "Error issuing Certificate for VirtualServer resource: %v", err)
rec.Eventf(vs, corev1.EventTypeWarning, reasonBadConfig, "Error issuing Certificate for VirtualServer resource: %s",
err)
return err
Expand All @@ -99,7 +100,7 @@ func SyncFnFor(
for _, crt := range updateCrts {
_, err := cmClient.CertmanagerV1().Certificates(crt.Namespace).Update(ctx, crt, metav1.UpdateOptions{})
if err != nil {
glog.Errorf("Error updating Certificate for VirtualServer resource: %v", err)
nl.Errorf(l, "Error updating Certificate for VirtualServer resource: %v", err)
rec.Eventf(vs, corev1.EventTypeWarning, reasonBadConfig, "Error updating Certificate for VirtualServer resource: %s",
err)
return err
Expand All @@ -117,7 +118,7 @@ func SyncFnFor(
for _, certName := range unrequiredCertNames {
err = cmClient.CertmanagerV1().Certificates(vs.GetNamespace()).Delete(ctx, certName, metav1.DeleteOptions{})
if err != nil {
glog.Errorf("Error deleting Certificate for VirtualServer resource: %v", err)
nl.Errorf(l, "Error deleting Certificate for VirtualServer resource: %v", err)
return err
}
rec.Eventf(vs, corev1.EventTypeNormal, reasonDeleteCertificate, "Successfully deleted unrequired Certificate %q", certName)
Expand All @@ -128,6 +129,7 @@ func SyncFnFor(
}

func buildCertificates(
ctx context.Context,
cmLister cmlisters.CertificateLister,
vs *vsapi.VirtualServer,
issuerName, issuerKind, issuerGroup string,
Expand Down Expand Up @@ -165,6 +167,8 @@ func buildCertificates(
},
}

l := nl.LoggerFromContext(ctx)

vs = vs.DeepCopy()

if err := translateVsSpec(crt, vs.Spec.TLS.CertManager); err != nil {
Expand All @@ -174,20 +178,20 @@ func buildCertificates(
// check if a Certificate for this TLS entry already exists, and if it
// does then skip this entry
if existingCrt != nil {
glog.V(3).Infof("certificate already exists for this object, ensuring it is up to date")
nl.Debugf(l, "certificate already exists for this object, ensuring it is up to date")

if metav1.GetControllerOf(existingCrt) == nil {
glog.V(3).Infof("certificate resource has no owner. refusing to update non-owned certificate resource for object")
nl.Debugf(l, "certificate resource has no owner. refusing to update non-owned certificate resource for object")
return nil, nil, nil
}

if !metav1.IsControlledBy(existingCrt, vs) {
glog.V(3).Infof("certificate resource is not owned by this object. refusing to update non-owned certificate resource for object")
nl.Debugf(l, "certificate resource is not owned by this object. refusing to update non-owned certificate resource for object")
return nil, nil, nil
}

if !certNeedsUpdate(existingCrt, crt) {
glog.V(3).Infof("certificate resource is already up to date for object")
nl.Debugf(l, "certificate resource is already up to date for object")
return nil, nil, nil
}

Expand Down
30 changes: 18 additions & 12 deletions internal/externaldns/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"sync"
"time"

"github.com/golang/glog"
nl "github.com/nginxinc/kubernetes-ingress/internal/logger"
conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
extdns_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/externaldns/v1"
k8s_nginx "github.com/nginxinc/kubernetes-ingress/pkg/client/clientset/versioned"
Expand Down Expand Up @@ -113,7 +113,9 @@ func (c *ExtDNSController) Run(stopCh <-chan struct{}) {
ctx, cancel := context.WithCancel(c.ctx)
defer cancel()

glog.Infof("Starting external-dns control loop")
l := nl.LoggerFromContext(ctx)

nl.Info(l, "Starting external-dns control loop")

var mustSync []cache.InformerSynced
for _, ig := range c.informerGroup {
Expand All @@ -122,17 +124,17 @@ func (c *ExtDNSController) Run(stopCh <-chan struct{}) {
}

// wait for all informer caches to be synced
glog.V(3).Infof("Waiting for %d caches to sync", len(mustSync))
nl.Debugf(l, "Waiting for %d caches to sync", len(mustSync))
if !cache.WaitForNamedCacheSync(ControllerName, stopCh, mustSync...) {
glog.Fatal("error syncing extDNS queue")
nl.Fatal(l, "error syncing extDNS queue")
}

glog.V(3).Infof("Queue is %v", c.queue.Len())
nl.Debugf(l, "Queue is %v", c.queue.Len())

go c.runWorker(ctx)

<-stopCh
glog.V(3).Infof("shutting down queue as workqueue signaled shutdown")
nl.Debugf(l, "shutting down queue as workqueue signaled shutdown")
for _, ig := range c.informerGroup {
ig.stop()
}
Expand All @@ -150,7 +152,8 @@ func (nsi *namespacedInformer) stop() {
// runWorker is a long-running function that will continually call the processItem
// function in order to read and process a message on the workqueue.
func (c *ExtDNSController) runWorker(ctx context.Context) {
glog.V(3).Infof("processing items on the workqueue")
l := nl.LoggerFromContext(ctx)
nl.Debugf(l, "processing items on the workqueue")
for {
obj, shutdown := c.queue.Get()
if shutdown {
Expand All @@ -165,11 +168,11 @@ func (c *ExtDNSController) runWorker(ctx context.Context) {
}

if err := c.processItem(ctx, key); err != nil {
glog.V(3).Infof("Re-queuing item due to error processing: %v", err)
nl.Debugf(l, "Re-queuing item due to error processing: %v", err)
c.queue.AddRateLimited(obj)
return
}
glog.V(3).Infof("finished processing work item")
nl.Debugf(l, "finished processing work item")
c.queue.Forget(obj)
}()
}
Expand All @@ -181,6 +184,7 @@ func (c *ExtDNSController) processItem(ctx context.Context, key string) error {
runtime.HandleError(fmt.Errorf("invalid resource key: %s", key))
return err
}
l := nl.LoggerFromContext(ctx)
var vs *conf_v1.VirtualServer
nsi := getNamespacedInformer(namespace, c.informerGroup)
vs, err = nsi.vsLister.VirtualServers(namespace).Get(name)
Expand All @@ -193,7 +197,7 @@ func (c *ExtDNSController) processItem(ctx context.Context, key string) error {
if err != nil {
return err
}
glog.V(3).Infof("processing virtual server resource")
nl.Debugf(l, "processing virtual server resource")
return c.sync(ctx, vs)
}

Expand Down Expand Up @@ -255,7 +259,8 @@ func getNamespacedInformer(ns string, ig map[string]*namespacedInformer) *namesp

// AddNewNamespacedInformer adds watchers for a new namespace
func (c *ExtDNSController) AddNewNamespacedInformer(ns string) {
glog.V(3).Infof("Adding or Updating cert-manager Watchers for Namespace: %v", ns)
l := nl.LoggerFromContext(c.ctx)
nl.Debugf(l, "Adding or Updating cert-manager Watchers for Namespace: %v", ns)
nsi := getNamespacedInformer(ns, c.informerGroup)
if nsi == nil {
nsi = c.newNamespacedInformer(ns)
Expand All @@ -268,7 +273,8 @@ func (c *ExtDNSController) AddNewNamespacedInformer(ns string) {

// RemoveNamespacedInformer removes watchers for a namespace we are no longer watching
func (c *ExtDNSController) RemoveNamespacedInformer(ns string) {
glog.V(3).Infof("Deleting cert-manager Watchers for Deleted Namespace: %v", ns)
l := nl.LoggerFromContext(c.ctx)
nl.Debugf(l, "Deleting cert-manager Watchers for Deleted Namespace: %v", ns)
nsi := getNamespacedInformer(ns, c.informerGroup)
if nsi != nil {
nsi.lock.Lock()
Expand Down
Loading

0 comments on commit f034346

Please sign in to comment.