From 26357c650766a85b4e8d03290dde24d7d5e54841 Mon Sep 17 00:00:00 2001 From: Jelmer Snoeck Date: Sun, 24 Mar 2019 12:51:50 -0300 Subject: [PATCH 1/2] Use Logrus instead of stdlibn log --- CHANGELOG.md | 12 ++++ Gopkg.lock | 17 ++++++ cmd/ingress-monitor/main.go | 16 ++++++ internal/ingressmonitor/cmd/operator.go | 18 +++--- internal/ingressmonitor/operator.go | 75 +++++++++++++++++-------- internal/provider/logger/logger.go | 9 ++- 6 files changed, 109 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19520b4..dd1c801 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## v0.3.1 - 2019-03-24 + +### Changed + +- We're now using logrus instead of the stdlib log package. + +## v0.3.0 - 2019-01-18 + +### Added + +- Added functionality to ensure that SSL checks are enabled with StatusCake + ## v0.2.0 - 2018-10-31 ### Added diff --git a/Gopkg.lock b/Gopkg.lock index a70478a..a42db63 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -165,6 +165,14 @@ revision = "1624edc4454b8682399def8740d46db5e4362ba4" version = "v1.1.5" +[[projects]] + digest = "1:0f51cee70b0d254dbc93c22666ea2abf211af81c1701a96d04e2284b408621db" + name = "github.com/konsorten/go-windows-terminal-sequences" + packages = ["."] + pruneopts = "" + revision = "f55edac94c9bbba5d6182a4be46d86a2c9b5b50e" + version = "v1.0.2" + [[projects]] digest = "1:961dc3b1d11f969370533390fdf203813162980c858e1dabe827b60940c909a5" name = "github.com/magiconair/properties" @@ -257,6 +265,14 @@ pruneopts = "" revision = "05ee40e3a273f7245e8777337fc7b46e533a9a92" +[[projects]] + digest = "1:b73fe282e350b3ef2c71d8ff08e929e0b9670b1bb5b7fde1d3c1b4cd6e6dc8b1" + name = "github.com/sirupsen/logrus" + packages = ["."] + pruneopts = "" + revision = "dae0fa8d5b0c810a8ab733fbd5510c7cae84eca4" + version = "v1.4.0" + [[projects]] digest = "1:7ba2551c9a8de293bc575dbe2c0d862c52252d26f267f784547f059f512471c8" name = "github.com/spf13/afero" @@ -712,6 +728,7 @@ "github.com/prometheus/client_golang/prometheus", "github.com/prometheus/client_golang/prometheus/promhttp", "github.com/prometheus/client_model/go", + "github.com/sirupsen/logrus", "github.com/spf13/cobra", "github.com/spf13/viper", "k8s.io/api/core/v1", diff --git a/cmd/ingress-monitor/main.go b/cmd/ingress-monitor/main.go index b582514..449c783 100644 --- a/cmd/ingress-monitor/main.go +++ b/cmd/ingress-monitor/main.go @@ -21,12 +21,28 @@ package main import ( + "os" + "strconv" + "github.com/jelmersnoeck/ingress-monitor/internal/ingressmonitor/cmd" + "github.com/sirupsen/logrus" "github.com/spf13/viper" ) func main() { viper.SetEnvPrefix("im") + debugString, ok := os.LookupEnv("DEBUG") + if ok { + debug, err := strconv.ParseBool(debugString) + if err != nil { + panic(err) + } + + if debug { + logrus.SetLevel(logrus.DebugLevel) + } + } + cmd.Execute() } diff --git a/internal/ingressmonitor/cmd/operator.go b/internal/ingressmonitor/cmd/operator.go index 3d5b6da..b7b1117 100644 --- a/internal/ingressmonitor/cmd/operator.go +++ b/internal/ingressmonitor/cmd/operator.go @@ -1,7 +1,6 @@ package cmd import ( - "log" "os" "time" @@ -15,9 +14,10 @@ import ( "github.com/jelmersnoeck/ingress-monitor/pkg/client/generated/clientset/versioned" "github.com/prometheus/client_golang/prometheus" - + "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "k8s.io/api/core/v1" + + v1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" ) @@ -44,22 +44,22 @@ func runOperator(cmd *cobra.Command, args []string) { resync, err := time.ParseDuration(operatorFlags.ResyncPeriod) if err != nil { - log.Fatalf("Error parsing ResyncPeriod: %s", err) + logrus.WithError(err).Fatal("Error parsing ResyncPeriod") } cfg, err := clientcmd.BuildConfigFromFlags(operatorFlags.MasterURL, operatorFlags.KubeConfig) if err != nil { - log.Fatalf("Error building kubeconfig: %s", err.Error()) + logrus.WithError(err).Fatal("Error building kubeconfig") } kubeClient, err := kubernetes.NewForConfig(cfg) if err != nil { - log.Fatalf("Error building Kubernetes clientset: %s", err) + logrus.WithError(err).Fatal("Error building Kubernetes clientset") } imClient, err := versioned.NewForConfig(cfg) if err != nil { - log.Fatalf("Error building IngressMonitor clientset: %s", err) + logrus.WithError(err).Fatal("Error building IngressMonitor clientset") } // register the available providers @@ -88,11 +88,11 @@ func runOperator(cmd *cobra.Command, args []string) { resync, fact, mtrc, ) if err != nil { - log.Fatalf("Error building IngressMonitor Operator: %s", err) + logrus.WithError(err).Fatalf("Error building IngressMonitor Operator") } if err := op.Run(stopCh); err != nil { - log.Fatalf("Error running the operator: %s", err) + logrus.WithError(err).Fatalf("Error running the operator") } } diff --git a/internal/ingressmonitor/operator.go b/internal/ingressmonitor/operator.go index e8d37da..f9ac059 100644 --- a/internal/ingressmonitor/operator.go +++ b/internal/ingressmonitor/operator.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "html/template" - "log" "strings" "time" @@ -33,6 +32,7 @@ import ( "k8s.io/kubernetes/pkg/apis/extensions" "github.com/dchest/blake2b" + "github.com/sirupsen/logrus" ) const ( @@ -130,24 +130,24 @@ func (o *Operator) Run(stopCh <-chan struct{}) error { defer o.monitorQueue.ShutDown() defer o.ingressMonitorQueue.ShutDown() - log.Printf("Starting IngressMonitor Operator") + logrus.Infof("Starting IngressMonitor Operator") if err := o.connectToCluster(stopCh); err != nil { return err } - log.Printf("Starting the informers") + logrus.Infof("Starting the informers") if err := o.startInformers(stopCh); err != nil { return err } - log.Printf("Starting the workers") + logrus.Infof("Starting the workers") for i := 0; i < 4; i++ { go wait.Until(runWorker(o.processNextIngressMonitor), time.Second, stopCh) go wait.Until(runWorker(o.processNextMonitor), time.Second, stopCh) } <-stopCh - log.Printf("Stopping IngressMonitor Operator") + logrus.Infof("Stopping IngressMonitor Operator") return nil } @@ -161,7 +161,9 @@ func (o *Operator) connectToCluster(stopCh <-chan struct{}) error { return } - log.Printf("Connected to the cluster (version %s)", v) + logrus.WithFields(logrus.Fields{ + "cluster_version": v, + }).Info("Connected to the cluster") errCh <- nil }() @@ -179,7 +181,7 @@ func (o *Operator) connectToCluster(stopCh <-chan struct{}) error { func (o *Operator) startInformers(stopCh <-chan struct{}) error { for _, inf := range o.informers { - log.Printf("Starting informer %s", inf.name) + logrus.WithFields(logrus.Fields{"name": inf.name}).Info("Starting informer") go inf.informer.Run(stopCh) } @@ -187,19 +189,20 @@ func (o *Operator) startInformers(stopCh <-chan struct{}) error { return err } - log.Printf("Synced all caches") + logrus.Infof("Synced all caches") return nil } func (o *Operator) waitForCaches(stopCh <-chan struct{}) error { var syncFailed bool for _, inf := range o.informers { - log.Printf("Waiting for cache sync for %s", inf.name) + log := logrus.WithFields(logrus.Fields{"name": inf.name}) + log.Info("Waiting for cache sync for") if !cache.WaitForCacheSync(stopCh, inf.informer.HasSynced) { - log.Printf("Could not sync cache for %s", inf.name) + log.Infof("Could not sync cache for") syncFailed = true } else { - log.Printf("Synced cache for %s", inf.name) + log.Infof("Synced cache for") } } @@ -226,6 +229,9 @@ func (o *Operator) processNextMonitor() bool { } func (o *Operator) handleNextItem(name string, queue workqueue.RateLimitingInterface, handlerFunc func(string) error) bool { + log := logrus.WithFields(logrus.Fields{ + "queue_name": name, + }) obj, shutdown := queue.Get() if shutdown { @@ -241,7 +247,7 @@ func (o *Operator) handleNextItem(name string, queue workqueue.RateLimitingInter if key, ok = obj.(string); !ok { queue.Forget(obj) - log.Printf("Expected object name in %s workqueue, got %#v", name, obj) + log.Infof("Expected object name workqueue, got %#v", obj) return nil } @@ -250,12 +256,14 @@ func (o *Operator) handleNextItem(name string, queue workqueue.RateLimitingInter } queue.Forget(obj) - log.Printf("Synced '%s' in %s workqueue", key, name) + log.WithFields(logrus.Fields{ + "key": key, + }).Debug("Synced key in workqueue") return nil }(obj) if err != nil { - log.Printf(err.Error()) + log.WithError(err).Error("Error handling the queue") } return true @@ -312,32 +320,45 @@ func (o *Operator) OnDelete(obj interface{}) { cl, err := o.providerFactory.From(obj.Spec.Provider) if err != nil { - log.Printf("Could not get provider for IngressMonitor %s:%s: %s", obj.Namespace, obj.Name, err) + logDeleteErr("ingress_monitor", obj.Namespace, obj.Name, err, "could not get provider for IngressMonitor") return } if err := cl.Delete(obj.Status.ID); err != nil { - log.Printf("Could not delete IngressMonitor %s:%s: %s", obj.Namespace, obj.Name, err) + logDeleteErr("ingress_monitor", obj.Namespace, obj.Name, err, "could not delete IngressMonitor") return } case *v1alpha1.Monitor: imList, err := o.imClient.IngressMonitors(obj.Namespace). List(listOptions(map[string]string{monitorLabel: obj.Name})) if err != nil { - log.Printf("Could not list IngressMonitors for Monitors %s:%s: %s", obj.Namespace, obj.Name, err) + logDeleteErr("monitor", obj.Namespace, obj.Name, err, "could not list IngressMonitors for Monitor") return } for _, im := range imList.Items { - log.Printf("Deleting IngressMonitor `%s:%s` associated with deleted Monitor `%s:%s`", im.Namespace, im.Name, obj.Namespace, obj.Name) + ll := logrus.WithFields(logrus.Fields{ + "ingress_monitor_namespace": im.Namespace, + "ingress_monitor_name": im.Name, + "monitor_namespace": obj.Namespace, + "monitor_name": obj.Name, + }) + ll.Debug("Deleting IngressMonitor") if err := o.imClient.IngressMonitors(obj.Namespace). Delete(im.Name, &metav1.DeleteOptions{}); err != nil { - log.Printf("Could not delete IngressMonitor %s for Monitors %s:%s: %s", im.Name, obj.Namespace, obj.Name, err) + ll.WithError(err).Error("could not delete IngressMonitor for Monitor") } } } } +func logDeleteErr(prefix, ns, name string, err error, msg string) { + logrus.WithFields(logrus.Fields{ + fmt.Sprintf("%s_namespace", prefix): ns, + fmt.Sprintf("%s_name", prefix): name, + }).WithError(err).Error(msg) +} + // handleIngressMonitor handles IngressMonitors in a way that it knows how to // deal with creating and updating resources. func (o *Operator) handleIngressMonitor(key string) (err error) { @@ -435,11 +456,14 @@ func (o *Operator) garbageCollectMonitors(obj *v1alpha1.Monitor) error { // reconciliation to take care of actually removing the monitor with the // provider. if !isActive { - log.Printf("Deleting IngressMonitor %s:%s with GC", im.Namespace, im.Name) + ll := logrus.WithFields(logrus.Fields{ + "ingress_monitor_namespace": im.Namespace, + "ingress_monitor_name": im.Name, + }) + ll.Debug("Deleting IngressMonitor with GC") if err := o.imClient.IngressMonitors(im.Namespace). Delete(im.Name, &metav1.DeleteOptions{}); err != nil { - - log.Printf("Could not delete IngressMonitor %s:%s: %s", im.Namespace, im.Name, err) + ll.WithError(err).Error("Could not delete IngressMonitor") } } }) @@ -474,7 +498,7 @@ func (o *Operator) handleMonitor(key string) error { } if len(ingressList) == 0 { - log.Printf("No ingresses selected for %s:%s", obj.Namespace, obj.Name) + logrus.Infof("No ingresses selected for %s:%s", obj.Namespace, obj.Name) return nil } @@ -578,7 +602,10 @@ func (o *Operator) handleMonitor(key string) error { return fmt.Errorf("Could not ensure IngressMonitor: %s", err) } - log.Printf("Successfully synced IngressMonitor %s:%s", im.Namespace, im.Name) + logrus.WithFields(logrus.Fields{ + "ingress_monitor_namespace": im.Namespace, + "ingress_monitor_name": im.Name, + }).Debug("successfully synced IngressMonitor") } } diff --git a/internal/provider/logger/logger.go b/internal/provider/logger/logger.go index 32f00cd..28a9f49 100644 --- a/internal/provider/logger/logger.go +++ b/internal/provider/logger/logger.go @@ -1,10 +1,9 @@ package logger import ( - "log" - "github.com/jelmersnoeck/ingress-monitor/apis/ingressmonitor/v1alpha1" "github.com/jelmersnoeck/ingress-monitor/internal/provider" + "github.com/sirupsen/logrus" "k8s.io/client-go/kubernetes" ) @@ -23,21 +22,21 @@ type prov struct{} // Create logs out a create action. func (p *prov) Create(ts v1alpha1.MonitorTemplateSpec) (string, error) { - log.Printf("Creating monitor %s", ts.Name) + logrus.Infof("Creating monitor %s", ts.Name) return ts.Name, nil } // Delete logs out a delete action. func (p *prov) Delete(id string) error { - log.Printf("Deleting monitor %s", id) + logrus.Infof("Deleting monitor %s", id) return nil } // Update logs out the update information for this template spec. func (p *prov) Update(id string, ts v1alpha1.MonitorTemplateSpec) (string, error) { - log.Printf("Updating monitor %s with ID %s", ts.Name, id) + logrus.Infof("Updating monitor %s with ID %s", ts.Name, id) return id, nil } From 1c74e4c62293be84d8cfa7b691811eb2483db4b5 Mon Sep 17 00:00:00 2001 From: Jelmer Snoeck Date: Sun, 24 Mar 2019 14:12:01 -0300 Subject: [PATCH 2/2] golangci-lint instead of gometalinter --- Makefile | 7 ++----- internal/ingressmonitor/operator_test.go | 10 +--------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index a145827..d2df77a 100644 --- a/Makefile +++ b/Makefile @@ -27,13 +27,10 @@ test: vendor CGO_ENABLED=0 go test -v ./... linters: - go get -u github.com/alecthomas/gometalinter - gometalinter --install + go get -u github.com/golangci/golangci-lint/cmd/golangci-lint lint: - gometalinter --tests --disable-all --vendor --deadline=5m -e ".*generated.*" \ - -E gofmt -E golint -E gosimple -E vet -E misspell -E ineffassign -E deadcode \ - $(LINTER_PKGS) + golangci-lint run -D errcheck -D staticcheck ./... cover: vendor CGO_ENABLED=0 go test -v -coverprofile=coverage.txt -covermode=atomic ./... diff --git a/internal/ingressmonitor/operator_test.go b/internal/ingressmonitor/operator_test.go index 2e51a6e..57697a1 100644 --- a/internal/ingressmonitor/operator_test.go +++ b/internal/ingressmonitor/operator_test.go @@ -13,7 +13,7 @@ import ( imfake "github.com/jelmersnoeck/ingress-monitor/pkg/client/generated/clientset/versioned/fake" "github.com/prometheus/client_golang/prometheus" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -349,7 +349,6 @@ type operatorConfig struct { providers []runtime.Object templates []runtime.Object - monitors []runtime.Object ingressmonitors []runtime.Object crdObjects []runtime.Object } @@ -377,13 +376,6 @@ func withTemplates(obj ...runtime.Object) optionFunc { } } -func withMonitors(obj ...runtime.Object) optionFunc { - return func(op *operatorConfig) { - op.monitors = append(op.monitors, obj...) - op.crdObjects = append(op.crdObjects, obj...) - } -} - func withIngressMonitors(obj ...runtime.Object) optionFunc { return func(op *operatorConfig) { op.ingressmonitors = append(op.ingressmonitors, obj...)