Skip to content

Commit 5c3f787

Browse files
danehansdanehans
committed
Refactors logging to be compatible with controller-runtime
1 parent b116fe1 commit 5c3f787

File tree

12 files changed

+116
-92
lines changed

12 files changed

+116
-92
lines changed

‎cmd/cluster-ingress-operator/main.go

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,12 @@ import (
77

88
"github.com/openshift/cluster-ingress-operator/pkg/dns"
99
awsdns "github.com/openshift/cluster-ingress-operator/pkg/dns/aws"
10+
logf "github.com/openshift/cluster-ingress-operator/pkg/log"
1011
"github.com/openshift/cluster-ingress-operator/pkg/operator"
1112
operatorconfig "github.com/openshift/cluster-ingress-operator/pkg/operator/config"
1213

1314
configv1 "github.com/openshift/api/config/v1"
1415

15-
"github.com/sirupsen/logrus"
16-
1716
corev1 "k8s.io/api/core/v1"
1817

1918
"k8s.io/apimachinery/pkg/types"
@@ -32,54 +31,64 @@ const (
3231
cloudCredentialsSecretName = "cloud-credentials"
3332
)
3433

34+
var log = logf.Logger.WithName("entrypoint")
35+
3536
func main() {
3637
// Get a kube client.
3738
kubeConfig, err := config.GetConfig()
3839
if err != nil {
39-
logrus.Fatalf("failed to get kube config: %v", err)
40+
log.Error(err, "failed to get kube config")
41+
os.Exit(1)
4042
}
4143
kubeClient, err := operator.Client(kubeConfig)
4244
if err != nil {
43-
logrus.Fatalf("failed to create kube client: %v", err)
45+
log.Error(err, "failed to create kube client")
46+
os.Exit(1)
4447
}
4548

4649
// Collect operator configuration.
4750
operatorNamespace, ok := os.LookupEnv("WATCH_NAMESPACE")
4851
if !ok {
49-
logrus.Fatalf("WATCH_NAMESPACE environment variable is required")
52+
log.Error(fmt.Errorf("missing environment variable"), "'WATCH_NAMESPACE' environment variable must be set")
53+
os.Exit(1)
5054
}
5155
routerImage := os.Getenv("IMAGE")
5256
if len(routerImage) == 0 {
53-
logrus.Fatalf("IMAGE environment variable is required")
57+
log.Error(fmt.Errorf("missing environment variable"), "'IMAGE' environment variable must be set")
58+
os.Exit(1)
5459
}
5560

5661
// Retrieve the cluster infrastructure config.
5762
infraConfig := &configv1.Infrastructure{}
5863
err = kubeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, infraConfig)
5964
if err != nil {
60-
logrus.Fatalf("failed to get infrastructure 'config': %v", err)
65+
log.Error(err, "failed to get infrastructure 'config'")
66+
os.Exit(1)
6167
}
6268

6369
// Retrieve the cluster ingress config.
6470
ingressConfig := &configv1.Ingress{}
6571
err = kubeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, ingressConfig)
6672
if err != nil {
67-
logrus.Fatalf("failed to get ingress 'cluster': %v", err)
73+
log.Error(err, "failed to get ingress 'cluster'")
74+
os.Exit(1)
6875
}
6976
if len(ingressConfig.Spec.Domain) == 0 {
70-
logrus.Warnln("cluster ingress configuration has an empty domain; default ClusterIngress will have empty ingressDomain")
77+
log.Info("cluster ingress configuration has an empty domain; default ClusterIngress will have empty ingressDomain")
7178
}
7279

7380
dnsConfig := &configv1.DNS{}
7481
err = kubeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, dnsConfig)
7582
if err != nil {
76-
logrus.Fatalf("failed to get dns 'cluster': %v", err)
83+
log.Error(err, "failed to get dns 'cluster'")
84+
os.Exit(1)
7785
}
7886

7987
// Set up the DNS manager.
8088
dnsManager, err := createDNSManager(kubeClient, operatorNamespace, infraConfig, dnsConfig)
8189
if err != nil {
82-
logrus.Fatalf("failed to create DNS manager: %v", err)
90+
log.Error(err, "failed to create DNS manager")
91+
os.Exit(1)
8392
}
8493

8594
// Set up and start the operator.
@@ -91,10 +100,12 @@ func main() {
91100
}
92101
op, err := operator.New(operatorConfig, dnsManager, kubeConfig)
93102
if err != nil {
94-
logrus.Fatalf("failed to create operator: %v", err)
103+
log.Error(err, "failed to create operator")
104+
os.Exit(1)
95105
}
96106
if err := op.Start(signals.SetupSignalHandler()); err != nil {
97-
logrus.Fatal(err)
107+
log.Error(err, "failed to start operator")
108+
os.Exit(1)
98109
}
99110
}
100111

‎pkg/dns/aws/dns.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ import (
66
"strings"
77
"sync"
88

9-
"github.com/sirupsen/logrus"
10-
119
"github.com/openshift/cluster-ingress-operator/pkg/dns"
10+
logf "github.com/openshift/cluster-ingress-operator/pkg/log"
1211

1312
"github.com/aws/aws-sdk-go/aws"
1413
"github.com/aws/aws-sdk-go/aws/arn"
@@ -25,7 +24,10 @@ import (
2524
configv1 "github.com/openshift/api/config/v1"
2625
)
2726

28-
var _ dns.Manager = &Manager{}
27+
var (
28+
_ dns.Manager = &Manager{}
29+
log = logf.Logger.WithName("aws-dns-manager")
30+
)
2931

3032
// Manager provides AWS DNS record management. In this implementation, calling
3133
// Ensure will create records in any zone specified in the DNS configuration.
@@ -79,15 +81,15 @@ func NewManager(config Config) (*Manager, error) {
7981

8082
region := aws.StringValue(sess.Config.Region)
8183
if len(region) > 0 {
82-
logrus.Infof("using region %q from shared config", region)
84+
log.Info("using region from shared config", "region name", region)
8385
} else {
8486
metadata := ec2metadata.New(sess)
8587
discovered, err := metadata.Region()
8688
if err != nil {
8789
return nil, fmt.Errorf("couldn't get region from metadata: %v", err)
8890
}
8991
region = discovered
90-
logrus.Infof("discovered region %q from metadata", region)
92+
log.Info("discovered region from metadata", "region name", region)
9193
}
9294

9395
return &Manager{
@@ -167,7 +169,7 @@ func (m *Manager) getZoneID(zoneConfig configv1.DNSZone) (string, error) {
167169

168170
// Update the cache
169171
m.idsToTags[id] = zoneConfig.Tags
170-
logrus.Infof("found zone id %q for tags %v", id, zoneConfig.Tags)
172+
log.Info("found hosted zone using tags", "zone id", id, "tags", zoneConfig.Tags)
171173

172174
return id, nil
173175
}
@@ -218,7 +220,7 @@ func (m *Manager) change(record *dns.Record, action action) error {
218220
return fmt.Errorf("failed to describe load balancers: %v", err)
219221
}
220222
for _, lb := range loadBalancers.LoadBalancerDescriptions {
221-
logrus.Debugf("found load balancer: %s (DNS name: %s, zone: %s)", aws.StringValue(lb.LoadBalancerName), aws.StringValue(lb.DNSName), aws.StringValue(lb.CanonicalHostedZoneNameID))
223+
log.Info("found load balancer", "name", aws.StringValue(lb.LoadBalancerName), "dns name", aws.StringValue(lb.DNSName), "hosted zone ID", aws.StringValue(lb.CanonicalHostedZoneNameID))
222224
if aws.StringValue(lb.CanonicalHostedZoneName) == target {
223225
targetHostedZoneID = aws.StringValue(lb.CanonicalHostedZoneNameID)
224226
break
@@ -235,7 +237,7 @@ func (m *Manager) change(record *dns.Record, action action) error {
235237
key := zoneID + domain + target
236238
// Only process updates once for now because we're not diffing.
237239
if m.updatedRecords.Has(key) && action == upsertAction {
238-
logrus.Infof("skipping update for record %v", record)
240+
log.Info("skipping DNS record update", "record", record)
239241
return nil
240242
}
241243
err = m.updateAlias(domain, zoneID, target, targetHostedZoneID, string(action))
@@ -245,10 +247,10 @@ func (m *Manager) change(record *dns.Record, action action) error {
245247
switch action {
246248
case upsertAction:
247249
m.updatedRecords.Insert(key)
248-
logrus.Infof("updated DNS record: %v", record)
250+
log.Info("upserted DNS record", "record", record)
249251
case deleteAction:
250252
m.updatedRecords.Delete(key)
251-
logrus.Infof("deleted DNS record: %v", record)
253+
log.Info("deleted DNS record", "record", record)
252254
}
253255
return nil
254256
}
@@ -278,6 +280,6 @@ func (m *Manager) updateAlias(domain, zoneID, target, targetHostedZoneID, action
278280
if err != nil {
279281
return fmt.Errorf("couldn't update DNS record in zone %s: %v", zoneID, err)
280282
}
281-
logrus.Infof("updated DNS record in zone %s, %s -> %s: %v", zoneID, domain, target, resp)
283+
log.Info("updated DNS record", "zone id", zoneID, "domain", domain, "target", target, "response", resp)
282284
return nil
283285
}

‎pkg/log/log.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package log
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/go-logr/logr"
7+
"github.com/go-logr/zapr"
8+
"go.uber.org/zap"
9+
"sigs.k8s.io/controller-runtime/pkg/runtime/log"
10+
)
11+
12+
// Logger is a simple logging interface for Go.
13+
var Logger logr.Logger
14+
15+
func init() {
16+
// Build a zap development logger.
17+
zapLog, err := zap.NewDevelopment()
18+
if err != nil {
19+
panic(fmt.Sprintf("error building logger: %v", err))
20+
}
21+
// zapr defines an implementation of the Logger
22+
// interface built on top of Zap (go.uber.org/zap).
23+
Logger = zapr.NewLogger(zapLog).WithName("cluster-ingress-operator")
24+
Logger.Info("started zapr logger")
25+
26+
// Set a concrete logging implementation for all
27+
// controller-runtime deferred Loggers.
28+
log.SetLogger(Logger)
29+
}

‎pkg/operator/controller/controller.go

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ import (
55
"fmt"
66
"time"
77

8-
"github.com/sirupsen/logrus"
9-
108
ingressv1alpha1 "github.com/openshift/cluster-ingress-operator/pkg/apis/ingress/v1alpha1"
119
"github.com/openshift/cluster-ingress-operator/pkg/dns"
10+
logf "github.com/openshift/cluster-ingress-operator/pkg/log"
1211
"github.com/openshift/cluster-ingress-operator/pkg/manifests"
1312
"github.com/openshift/cluster-ingress-operator/pkg/util/slice"
1413

@@ -38,6 +37,8 @@ const (
3837
ClusterIngressFinalizer = "ingress.openshift.io/default-cluster-ingress"
3938
)
4039

40+
var log = logf.Logger.WithName("controller")
41+
4142
// New creates the operator controller from configuration. This is the
4243
// controller that handles all the logic for implementing ingress based on
4344
// ClusterIngress resources.
@@ -81,27 +82,19 @@ type reconciler struct {
8182
// namespace, and will do all the work to ensure the clusteringress is in the
8283
// desired state.
8384
func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, error) {
84-
result, err := r.reconcile(request)
85-
if err != nil {
86-
// TODO: We're not setting up the controller-runtime logger, so errors we
87-
// bubble out are not being logged. For now, log them here, but we need to
88-
// redo our logging and wire up the controller-runtime logger because who
89-
// knows what else is being eaten.
90-
logrus.Errorf("error: %v", err)
91-
}
92-
return result, err
85+
return r.reconcile(request)
9386
}
9487

9588
func (r *reconciler) reconcile(request reconcile.Request) (reconcile.Result, error) {
9689
// TODO: Should this be another controller?
9790
defer func() {
9891
err := r.syncOperatorStatus()
9992
if err != nil {
100-
logrus.Infof("failed to sync operator status: %v", err)
93+
log.Info("failed to sync operator status", "error", err)
10194
}
10295
}()
10396

104-
logrus.Infof("reconciling request: %v", request)
97+
log.Info("reconciling", "request", request)
10598

10699
// Get the current ingress state.
107100
ingress := &ingressv1alpha1.ClusterIngress{}
@@ -193,13 +186,13 @@ func (r *reconciler) ensureIngressDeleted(ingress *ingressv1alpha1.ClusterIngres
193186
if err != nil {
194187
return fmt.Errorf("failed to finalize load balancer service for %s: %v", ingress.Name, err)
195188
}
196-
logrus.Infof("finalized load balancer service for ingress %s", ingress.Name)
189+
log.Info("finalized load balancer service for ingress", "namespace", ingress.Namespace, "name", ingress.Name)
197190

198191
err = r.ensureRouterDeleted(ingress)
199192
if err != nil {
200193
return fmt.Errorf("failed to delete deployment for ingress %s: %v", ingress.Name, err)
201194
}
202-
logrus.Infof("deleted deployment for ingress %s", ingress.Name)
195+
log.Info("deleted deployment for ingress", "namespace", ingress.Namespace, "name", ingress.Name)
203196

204197
// Clean up the finalizer to allow the clusteringress to be deleted.
205198
updated := ingress.DeepCopy()
@@ -227,7 +220,7 @@ func (r *reconciler) ensureRouterNamespace() error {
227220
}
228221
err = r.Client.Create(context.TODO(), cr)
229222
if err == nil {
230-
logrus.Infof("created router cluster role %s", cr.Name)
223+
log.Info("created router cluster role", "name", cr.Name)
231224
} else if !errors.IsAlreadyExists(err) {
232225
return fmt.Errorf("failed to create router cluster role %s: %v", cr.Name, err)
233226
}
@@ -244,7 +237,7 @@ func (r *reconciler) ensureRouterNamespace() error {
244237
}
245238
err = r.Client.Create(context.TODO(), ns)
246239
if err == nil {
247-
logrus.Infof("created router namespace %s", ns.Name)
240+
log.Info("created router namespace", "name", ns.Name)
248241
} else if !errors.IsAlreadyExists(err) {
249242
return fmt.Errorf("failed to create router namespace %s: %v", ns.Name, err)
250243
}
@@ -261,7 +254,7 @@ func (r *reconciler) ensureRouterNamespace() error {
261254
}
262255
err = r.Client.Create(context.TODO(), sa)
263256
if err == nil {
264-
logrus.Infof("created router service account %s/%s", sa.Namespace, sa.Name)
257+
log.Info("created router service account", "namespace", sa.Namespace, "name", sa.Name)
265258
} else if !errors.IsAlreadyExists(err) {
266259
return fmt.Errorf("failed to create router service account %s/%s: %v", sa.Namespace, sa.Name, err)
267260
}
@@ -278,7 +271,7 @@ func (r *reconciler) ensureRouterNamespace() error {
278271
}
279272
err = r.Client.Create(context.TODO(), crb)
280273
if err == nil {
281-
logrus.Infof("created router cluster role binding %s", crb.Name)
274+
log.Info("created router cluster role binding", "name", crb.Name)
282275
} else if !errors.IsAlreadyExists(err) {
283276
return fmt.Errorf("failed to create router cluster role binding %s: %v", crb.Name, err)
284277
}
@@ -303,7 +296,7 @@ func (r *reconciler) ensureRouterForIngress(ci *ingressv1alpha1.ClusterIngress,
303296

304297
err = r.Client.Create(context.TODO(), current)
305298
if err == nil {
306-
logrus.Infof("created router deployment %s/%s", current.Namespace, current.Name)
299+
log.Info("created router deployment", "namespace", current.Namespace, "name", current.Name)
307300
} else if !errors.IsAlreadyExists(err) {
308301
return fmt.Errorf("failed to create router deployment %s/%s: %v", current.Namespace, current.Name, err)
309302
}
@@ -312,7 +305,7 @@ func (r *reconciler) ensureRouterForIngress(ci *ingressv1alpha1.ClusterIngress,
312305
if changed, updated := deploymentConfigChanged(current, expected); changed {
313306
err = r.Client.Update(context.TODO(), updated)
314307
if err == nil {
315-
logrus.Infof("updated router deployment %s/%s", updated.Namespace, updated.Name)
308+
log.Info("updated router deployment", "namespace", updated.Namespace, "name", updated.Name)
316309
current = updated
317310
} else {
318311
return fmt.Errorf("failed to update router deployment %s/%s, %v", updated.Namespace, updated.Name, err)
@@ -384,7 +377,7 @@ func (r *reconciler) ensureMetricsIntegration(ci *ingressv1alpha1.ClusterIngress
384377
statsSecret.SetOwnerReferences([]metav1.OwnerReference{deploymentRef})
385378
err = r.Client.Create(context.TODO(), statsSecret)
386379
if err == nil {
387-
logrus.Infof("created router stats secret %s/%s", statsSecret.Namespace, statsSecret.Name)
380+
log.Info("created router stats secret", "namespace", statsSecret.Namespace, "name", statsSecret.Name)
388381
} else if !errors.IsAlreadyExists(err) {
389382
return fmt.Errorf("failed to create router stats secret %s/%s: %v", statsSecret.Namespace, statsSecret.Name, err)
390383
}
@@ -401,7 +394,7 @@ func (r *reconciler) ensureMetricsIntegration(ci *ingressv1alpha1.ClusterIngress
401394
}
402395
err = r.Client.Create(context.TODO(), cr)
403396
if err == nil {
404-
logrus.Infof("created router metrics cluster role %s", cr.Name)
397+
log.Info("created router metrics cluster role", "name", cr.Name)
405398
} else if !errors.IsAlreadyExists(err) {
406399
return fmt.Errorf("failed to create router metrics cluster role %s: %v", cr.Name, err)
407400
}
@@ -418,7 +411,7 @@ func (r *reconciler) ensureMetricsIntegration(ci *ingressv1alpha1.ClusterIngress
418411
}
419412
err = r.Client.Create(context.TODO(), crb)
420413
if err == nil {
421-
logrus.Infof("created router metrics cluster role binding %s", crb.Name)
414+
log.Info("created router metrics cluster role binding", "name", crb.Name)
422415
} else if !errors.IsAlreadyExists(err) {
423416
return fmt.Errorf("failed to create router metrics cluster role binding %s: %v", crb.Name, err)
424417
}
@@ -435,7 +428,7 @@ func (r *reconciler) ensureMetricsIntegration(ci *ingressv1alpha1.ClusterIngress
435428
}
436429
err = r.Client.Create(context.TODO(), mr)
437430
if err == nil {
438-
logrus.Infof("created router metrics role %s", mr.Name)
431+
log.Info("created router metrics role", "name", mr.Name)
439432
} else if !errors.IsAlreadyExists(err) {
440433
return fmt.Errorf("failed to create router metrics role %s: %v", mr.Name, err)
441434
}
@@ -452,7 +445,7 @@ func (r *reconciler) ensureMetricsIntegration(ci *ingressv1alpha1.ClusterIngress
452445
}
453446
err = r.Client.Create(context.TODO(), mrb)
454447
if err == nil {
455-
logrus.Infof("created router metrics role binding %s", mrb.Name)
448+
log.Info("created router metrics role binding", "name", mrb.Name)
456449
} else if !errors.IsAlreadyExists(err) {
457450
return fmt.Errorf("failed to create router metrics role binding %s: %v", mrb.Name, err)
458451
}
@@ -498,7 +491,7 @@ func (r *reconciler) ensureInternalRouterServiceForIngress(ci *ingressv1alpha1.C
498491
svc.SetOwnerReferences([]metav1.OwnerReference{deploymentRef})
499492
err = r.Client.Create(context.TODO(), svc)
500493
if err == nil {
501-
logrus.Infof("created router service %s/%s", svc.Namespace, svc.Name)
494+
log.Info("created router service", "namespace", svc.Namespace, "name", svc.Name)
502495
} else if !errors.IsAlreadyExists(err) {
503496
return nil, fmt.Errorf("failed to create router service %s/%s: %v", svc.Namespace, svc.Name, err)
504497
}

0 commit comments

Comments
 (0)