1
0
Fork 0

refactor: Logs & errors review.

- log & error: remove format if not necessary, add if necessary.
- add constants for k8s annotations.
- fix typos
This commit is contained in:
Fernandez Ludovic 2017-05-26 17:03:14 +02:00 committed by Ludovic Fernandez
parent 994e135368
commit cbccdd51c5
26 changed files with 125 additions and 128 deletions

View file

@ -9,7 +9,6 @@ import (
"time"
"github.com/BurntSushi/ty/fun"
"github.com/Sirupsen/logrus"
"github.com/cenk/backoff"
"github.com/containous/traefik/job"
"github.com/containous/traefik/log"
@ -96,7 +95,7 @@ func (p *CatalogProvider) watchServices(stopCh <-chan struct{}) <-chan map[strin
data, catalogMeta, err := catalog.Services(catalogOptions)
if err != nil {
log.WithError(err).Errorf("Failed to list services")
log.WithError(err).Error("Failed to list services")
return
}
@ -105,7 +104,7 @@ func (p *CatalogProvider) watchServices(stopCh <-chan struct{}) <-chan map[strin
// (intentionally there is no interest in the received data).
_, healthMeta, err := health.State("passing", healthOptions)
if err != nil {
log.WithError(err).Errorf("Failed to retrieve health checks")
log.WithError(err).Error("Failed to retrieve health checks")
return
}
@ -133,7 +132,7 @@ func (p *CatalogProvider) healthyNodes(service string) (catalogUpdate, error) {
opts := &api.QueryOptions{}
data, _, err := health.Service(service, "", true, opts)
if err != nil {
log.WithError(err).Errorf("Failed to fetch details of " + service)
log.WithError(err).Errorf("Failed to fetch details of %s", service)
return catalogUpdate{}, err
}
@ -285,9 +284,7 @@ func (p *CatalogProvider) getNodes(index map[string][]string) ([]catalogUpdate,
name := strings.ToLower(service)
if !strings.Contains(name, " ") && !visited[name] {
visited[name] = true
log.WithFields(logrus.Fields{
"service": name,
}).Debug("Fetching service")
log.WithField("service", name).Debug("Fetching service")
healthy, err := p.healthyNodes(name)
if err != nil {
return nil, err

View file

@ -2,7 +2,7 @@ package docker
import (
"context"
"errors"
"fmt"
"math"
"net"
"net/http"
@ -705,7 +705,7 @@ func getLabel(container dockerData, label string) (string, error) {
return value, nil
}
}
return "", errors.New("Label not found:" + label)
return "", fmt.Errorf("label not found: %s", label)
}
func getLabels(container dockerData, labels []string) (map[string]string, error) {
@ -715,7 +715,7 @@ func getLabels(container dockerData, labels []string) (map[string]string, error)
foundLabel, err := getLabel(container, label)
// Error out only if one of them is defined.
if err != nil {
globalErr = errors.New("Label not found: " + label)
globalErr = fmt.Errorf("label not found: %s", label)
continue
}
foundLabels[label] = foundLabel

View file

@ -469,7 +469,7 @@ func TestDockerGetLabel(t *testing.T) {
}{
{
container: containerJSON(),
expected: "Label not found:",
expected: "label not found:",
},
{
container: containerJSON(labels(map[string]string{
@ -507,7 +507,7 @@ func TestDockerGetLabels(t *testing.T) {
{
container: containerJSON(),
expectedLabels: map[string]string{},
expectedError: "Label not found:",
expectedError: "label not found:",
},
{
container: containerJSON(labels(map[string]string{
@ -516,7 +516,7 @@ func TestDockerGetLabels(t *testing.T) {
expectedLabels: map[string]string{
"foo": "fooz",
},
expectedError: "Label not found: bar",
expectedError: "label not found: bar",
},
{
container: containerJSON(labels(map[string]string{

View file

@ -415,7 +415,7 @@ func TestSwarmGetLabel(t *testing.T) {
}{
{
service: swarmService(),
expected: "Label not found:",
expected: "label not found:",
networks: map[string]*docker.NetworkResource{},
},
{
@ -439,7 +439,7 @@ func TestSwarmGetLabel(t *testing.T) {
}
} else {
if label != "bar" {
t.Errorf("expected label 'bar', got %s", label)
t.Errorf("expected label 'bar', got '%s'", label)
}
}
})
@ -456,7 +456,7 @@ func TestSwarmGetLabels(t *testing.T) {
{
service: swarmService(),
expectedLabels: map[string]string{},
expectedError: "Label not found:",
expectedError: "label not found:",
networks: map[string]*docker.NetworkResource{},
},
{
@ -466,7 +466,7 @@ func TestSwarmGetLabels(t *testing.T) {
expectedLabels: map[string]string{
"foo": "fooz",
},
expectedError: "Label not found: bar",
expectedError: "label not found: bar",
networks: map[string]*docker.NetworkResource{},
},
{

View file

@ -40,7 +40,7 @@ type dynamoClient struct {
// createClient configures aws credentials and creates a dynamoClient
func (p *Provider) createClient() (*dynamoClient, error) {
log.Infof("Creating Provider client...")
log.Info("Creating Provider client...")
sess := session.New()
if p.Region == "" {
return nil, errors.New("no Region provided for Provider")
@ -105,14 +105,14 @@ func (p *Provider) loadDynamoConfig(client *dynamoClient) (*types.Configuration,
// verify the type of each item by checking to see if it has
// the corresponding type, backend or frontend map
if backend, exists := item["backend"]; exists {
log.Debugf("Unmarshaling backend from Provider...")
log.Debug("Unmarshaling backend from Provider...")
tmpBackend := &types.Backend{}
err = dynamodbattribute.Unmarshal(backend, tmpBackend)
if err != nil {
log.Errorf(err.Error())
} else {
backends[*item["name"].S] = tmpBackend
log.Debugf("Backend from Provider unmarshalled successfully")
log.Debug("Backend from Provider unmarshalled successfully")
}
} else if frontend, exists := item["frontend"]; exists {
log.Debugf("Unmarshaling frontend from Provider...")
@ -122,7 +122,7 @@ func (p *Provider) loadDynamoConfig(client *dynamoClient) (*types.Configuration,
log.Errorf(err.Error())
} else {
frontends[*item["name"].S] = tmpFrontend
log.Debugf("Frontend from Provider unmarshalled successfully")
log.Debug("Frontend from Provider unmarshalled successfully")
}
} else {
log.Warnf("Error in format of Provider Item: %v", item)
@ -176,7 +176,7 @@ func (p *Provider) Provide(configurationChan chan<- types.ConfigMessage, pool *s
reload := time.NewTicker(time.Second * time.Duration(p.RefreshSeconds))
defer reload.Stop()
for {
log.Debugf("Watching Provider...")
log.Debug("Watching Provider...")
select {
case <-reload.C:
configuration, err := p.loadDynamoConfig(aws)

View file

@ -29,6 +29,10 @@ const (
annotationFrontendRuleType = "traefik.frontend.rule.type"
ruleTypePathPrefix = "PathPrefix"
annotationKubernetesIngressClass = "kubernetes.io/ingress.class"
annotationKubernetesAuthRealm = "ingress.kubernetes.io/auth-realm"
annotationKubernetesAuthType = "ingress.kubernetes.io/auth-type"
annotationKubernetesAuthSecret = "ingress.kubernetes.io/auth-secret"
annotationKubernetesWhitelistSourceRange = "ingress.kubernetes.io/whitelist-source-range"
)
@ -127,11 +131,11 @@ func (p *Provider) loadIngresses(k8sClient Client) (*types.Configuration, error)
ingresses := k8sClient.GetIngresses(p.Namespaces)
templateObjects := types.Configuration{
map[string]*types.Backend{},
map[string]*types.Frontend{},
Backends: map[string]*types.Backend{},
Frontends: map[string]*types.Frontend{},
}
for _, i := range ingresses {
ingressClass := i.Annotations["kubernetes.io/ingress.class"]
ingressClass := i.Annotations[annotationKubernetesIngressClass]
if !shouldProcessIngress(ingressClass) {
continue
@ -139,7 +143,7 @@ func (p *Provider) loadIngresses(k8sClient Client) (*types.Configuration, error)
for _, r := range i.Spec.Rules {
if r.HTTP == nil {
log.Warnf("Error in ingress: HTTP is nil")
log.Warn("Error in ingress: HTTP is nil")
continue
}
for _, pa := range r.HTTP.Paths {
@ -166,7 +170,7 @@ func (p *Provider) loadIngresses(k8sClient Client) (*types.Configuration, error)
default:
log.Warnf("Unknown value '%s' for traefik.frontend.passHostHeader, falling back to %s", passHostHeaderAnnotation, PassHostHeader)
}
if realm := i.Annotations["ingress.kubernetes.io/auth-realm"]; realm != "" && realm != traefikDefaultRealm {
if realm := i.Annotations[annotationKubernetesAuthRealm]; realm != "" && realm != traefikDefaultRealm {
return nil, errors.New("no realm customization supported")
}
@ -291,14 +295,14 @@ func (p *Provider) loadIngresses(k8sClient Client) (*types.Configuration, error)
}
func handleBasicAuthConfig(i *v1beta1.Ingress, k8sClient Client) ([]string, error) {
authType, exists := i.Annotations["ingress.kubernetes.io/auth-type"]
authType, exists := i.Annotations[annotationKubernetesAuthType]
if !exists {
return nil, nil
}
if strings.ToLower(authType) != "basic" {
return nil, fmt.Errorf("unsupported auth-type: %q", authType)
}
authSecret := i.Annotations["ingress.kubernetes.io/auth-secret"]
authSecret := i.Annotations[annotationKubernetesAuthSecret]
if authSecret == "" {
return nil, errors.New("auth-secret annotation must be set")
}

View file

@ -108,7 +108,7 @@ func (p *Provider) Provide(configurationChan chan<- types.ConfigMessage, pool *s
case <-stop:
return
case event := <-update:
log.Debug("Provider event receveived", event)
log.Debug("Provider event received", event)
configuration := p.loadMarathonConfig()
if configuration != nil {
configurationChan <- types.ConfigMessage{

View file

@ -112,11 +112,11 @@ func (p *Provider) Provide(configurationChan chan<- types.ConfigMessage, pool *s
}
notify := func(err error, time time.Duration) {
log.Errorf("mesos connection error %+v, retrying in %s", err, time)
log.Errorf("Mesos connection error %+v, retrying in %s", err, time)
}
err := backoff.RetryNotify(safe.OperationWithRecover(operation), job.NewBackOff(backoff.NewExponentialBackOff()), notify)
if err != nil {
log.Errorf("Cannot connect to mesos server %+v", err)
log.Errorf("Cannot connect to Mesos server %+v", err)
}
return nil
}
@ -141,7 +141,7 @@ func (p *Provider) loadMesosConfig() *types.Configuration {
t := records.NewRecordGenerator(time.Duration(p.StateTimeoutSecond) * time.Second)
sj, err := t.FindMaster(p.Masters...)
if err != nil {
log.Errorf("Failed to create a client for mesos, error: %s", err)
log.Errorf("Failed to create a client for Mesos, error: %s", err)
return nil
}
tasks := p.taskRecords(sj)
@ -197,11 +197,11 @@ func labels(task state.Task, key string) string {
func mesosTaskFilter(task state.Task, exposedByDefaultFlag bool) bool {
if len(task.DiscoveryInfo.Ports.DiscoveryPorts) == 0 {
log.Debugf("Filtering mesos task without port %s", task.Name)
log.Debugf("Filtering Mesos task without port %s", task.Name)
return false
}
if !isMesosApplicationEnabled(task, exposedByDefaultFlag) {
log.Debugf("Filtering disabled mesos task %s", task.DiscoveryInfo.Name)
log.Debugf("Filtering disabled Mesos task %s", task.DiscoveryInfo.Name)
return false
}
@ -209,20 +209,20 @@ func mesosTaskFilter(task state.Task, exposedByDefaultFlag bool) bool {
portIndexLabel := labels(task, "traefik.portIndex")
portValueLabel := labels(task, "traefik.port")
if portIndexLabel != "" && portValueLabel != "" {
log.Debugf("Filtering mesos task %s specifying both traefik.portIndex and traefik.port labels", task.Name)
log.Debugf("Filtering Mesos task %s specifying both traefik.portIndex and traefik.port labels", task.Name)
return false
}
if portIndexLabel != "" {
index, err := strconv.Atoi(labels(task, "traefik.portIndex"))
if err != nil || index < 0 || index > len(task.DiscoveryInfo.Ports.DiscoveryPorts)-1 {
log.Debugf("Filtering mesos task %s with unexpected value for traefik.portIndex label", task.Name)
log.Debugf("Filtering Mesos task %s with unexpected value for traefik.portIndex label", task.Name)
return false
}
}
if portValueLabel != "" {
port, err := strconv.Atoi(labels(task, "traefik.port"))
if err != nil {
log.Debugf("Filtering mesos task %s with unexpected value for traefik.port label", task.Name)
log.Debugf("Filtering Mesos task %s with unexpected value for traefik.port label", task.Name)
return false
}
@ -235,14 +235,14 @@ func mesosTaskFilter(task state.Task, exposedByDefaultFlag bool) bool {
}
if !foundPort {
log.Debugf("Filtering mesos task %s without a matching port for traefik.port label", task.Name)
log.Debugf("Filtering Mesos task %s without a matching port for traefik.port label", task.Name)
return false
}
}
//filter healthchecks
if task.Statuses != nil && len(task.Statuses) > 0 && task.Statuses[0].Healthy != nil && !*task.Statuses[0].Healthy {
log.Debugf("Filtering mesos task %s with bad healthcheck", task.DiscoveryInfo.Name)
log.Debugf("Filtering Mesos task %s with bad healthcheck", task.DiscoveryInfo.Name)
return false
}
@ -274,7 +274,7 @@ func (p *Provider) getLabel(task state.Task, label string) (string, error) {
func (p *Provider) getPort(task state.Task, applications []state.Task) string {
application, err := getMesos(task, applications)
if err != nil {
log.Errorf("Unable to get mesos application from task %s", task.DiscoveryInfo.Name)
log.Errorf("Unable to get Mesos application from task %s", task.DiscoveryInfo.Name)
return ""
}
@ -296,7 +296,7 @@ func (p *Provider) getPort(task state.Task, applications []state.Task) string {
func (p *Provider) getWeight(task state.Task, applications []state.Task) string {
application, errApp := getMesos(task, applications)
if errApp != nil {
log.Errorf("Unable to get mesos application from task %s", task.DiscoveryInfo.Name)
log.Errorf("Unable to get Mesos application from task %s", task.DiscoveryInfo.Name)
return "0"
}
@ -316,7 +316,7 @@ func (p *Provider) getDomain(task state.Task) string {
func (p *Provider) getProtocol(task state.Task, applications []state.Task) string {
application, errApp := getMesos(task, applications)
if errApp != nil {
log.Errorf("Unable to get mesos application from task %s", task.DiscoveryInfo.Name)
log.Errorf("Unable to get Mesos application from task %s", task.DiscoveryInfo.Name)
return "http"
}
if label, err := p.getLabel(application, "traefik.protocol"); err == nil {
@ -358,7 +358,7 @@ func (p *Provider) getFrontendRule(task state.Task) string {
func (p *Provider) getBackend(task state.Task, applications []state.Task) string {
application, errApp := getMesos(task, applications)
if errApp != nil {
log.Errorf("Unable to get mesos application from task %s", task.DiscoveryInfo.Name)
log.Errorf("Unable to get Mesos application from task %s", task.DiscoveryInfo.Name)
return ""
}
return p.getFrontendBackend(application)
@ -392,9 +392,9 @@ func detectMasters(zk string, masters []string) <-chan []string {
if zk != "" {
log.Debugf("Starting master detector for ZK ", zk)
if md, err := detector.New(zk); err != nil {
log.Errorf("failed to create master detector: %v", err)
log.Errorf("Failed to create master detector: %v", err)
} else if err := md.Detect(detect.NewMasters(masters, changed)); err != nil {
log.Errorf("failed to initialize master detector: %v", err)
log.Errorf("Failed to initialize master detector: %v", err)
}
} else {
changed <- masters

View file

@ -141,7 +141,7 @@ func TestGetConfiguration(t *testing.T) {
t.Fatalf("Shouldn't have error out, got %v", err)
}
if configuration == nil {
t.Fatalf("Configuration should not be nil, but was")
t.Fatal("Configuration should not be nil, but was")
}
}
@ -203,15 +203,15 @@ func TestGetConfigurationReturnsCorrectMaxConnConfiguration(t *testing.T) {
t.Fatalf("Shouldn't have error out, got %v", err)
}
if configuration == nil {
t.Fatalf("Configuration should not be nil, but was")
t.Fatal("Configuration should not be nil, but was")
}
if configuration.Backends["backend1"].MaxConn.Amount != 10 {
t.Fatalf("Configuration did not parse MaxConn.Amount properly")
t.Fatal("Configuration did not parse MaxConn.Amount properly")
}
if configuration.Backends["backend1"].MaxConn.ExtractorFunc != "request.host" {
t.Fatalf("Configuration did not parse MaxConn.ExtractorFunc properly")
t.Fatal("Configuration did not parse MaxConn.ExtractorFunc properly")
}
}
@ -224,7 +224,7 @@ func TestNilClientTLS(t *testing.T) {
}
_, err := provider.TLS.CreateTLSConfig()
if err != nil {
t.Fatalf("CreateTLSConfig should assume that consumer does not want a TLS configuration if input is nil")
t.Fatal("CreateTLSConfig should assume that consumer does not want a TLS configuration if input is nil")
}
}
@ -371,12 +371,12 @@ func TestDefaultFuncMap(t *testing.T) {
t.Fatalf("Shouldn't have error out, got %v", err)
}
if configuration == nil {
t.Fatalf("Configuration should not be nil, but was")
t.Fatal("Configuration should not be nil, but was")
}
if _, ok := configuration.Backends["backend1"]; !ok {
t.Fatalf("backend1 should exists, but it not")
t.Fatal("backend1 should exists, but it not")
}
if _, ok := configuration.Frontends["frontend-1"]; !ok {
t.Fatalf("Frontend frontend-1 should exists, but it not")
t.Fatal("Frontend frontend-1 should exists, but it not")
}
}