Implement identifiable logging (#170)

* Implement identifiable logging As the operator moves towards being cluster-scoped it's important that users are able to quickly identify context from logs and can tell exactly which Nexus CR is the subject of logs. To that effect this change adds the "namespace" and "resource name" fields to structured logging when applicable. Additionally, standard output is now the default log output and sync operations were removed as they're not necessary due to stdout's unbuffered nature. Signed-off-by: Lucas Caparelli <[email protected]> * Make suggested changes Signed-off-by: Lucas Caparelli <[email protected]> * Add -local flag to goimports Signed-off-by: Lucas Caparelli <[email protected]>
m88i · Oct 7, 2020 · e7945a8 · e7945a8
1 parent 1d8a597
commit e7945a8
Show file tree

Hide file tree

Showing 51 changed files with 372 additions and 332 deletions.
diff --git a/hack/go-fmt.sh b/hack/go-fmt.sh
@@ -17,7 +17,7 @@
 gofmt -s -l -w cmd/ pkg/ version/
 # get the goimports binary
 command -v goimports >/dev/null || go build -o $GOPATH/bin/goimports golang.org/x/tools/cmd/goimports
-goimports -l -w cmd/ pkg/ version/
+goimports -local github.com/m88i/nexus-operator -l -w cmd/ pkg/ version/
 
 if [[ -n ${CI} ]]; then
     git diff --exit-code

diff --git a/pkg/cluster/kubernetes/events_test.go b/pkg/cluster/kubernetes/events_test.go
@@ -19,12 +19,13 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
-	"github.com/m88i/nexus-operator/pkg/test"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/test"
 )
 
 func TestRaiseInfoEventf(t *testing.T) {

diff --git a/pkg/cluster/kubernetes/ingress.go b/pkg/cluster/kubernetes/ingress.go
@@ -18,12 +18,13 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/m88i/nexus-operator/pkg/util"
 	networking "k8s.io/api/networking/v1beta1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/discovery"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/m88i/nexus-operator/pkg/util"
 )
 
 const ingressGroup = networking.GroupName

diff --git a/pkg/cluster/openshift/routes.go b/pkg/cluster/openshift/routes.go
@@ -18,12 +18,13 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/m88i/nexus-operator/pkg/util"
 	v1 "github.com/openshift/api/route/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/discovery"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/m88i/nexus-operator/pkg/util"
 )
 
 const (

diff --git a/pkg/controller/nexus/nexus_controller.go b/pkg/controller/nexus/nexus_controller.go
@@ -20,21 +20,9 @@ import (
 	"reflect"
 	"time"
 
-	"github.com/m88i/nexus-operator/pkg/cluster/kubernetes"
-	"github.com/m88i/nexus-operator/pkg/cluster/openshift"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/validation"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/server"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/update"
-	"github.com/m88i/nexus-operator/pkg/framework"
-	"github.com/m88i/nexus-operator/pkg/logger"
-	routev1 "github.com/openshift/api/route/v1"
-
 	resUtils "github.com/RHsyseng/operator-utils/pkg/resource"
 	"github.com/RHsyseng/operator-utils/pkg/resource/write"
-
-	appsv1alpha1 "github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource"
-
+	routev1 "github.com/openshift/api/route/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	networking "k8s.io/api/networking/v1beta1"
@@ -50,6 +38,16 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
+
+	appsv1alpha1 "github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/cluster/kubernetes"
+	"github.com/m88i/nexus-operator/pkg/cluster/openshift"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/validation"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/server"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/update"
+	"github.com/m88i/nexus-operator/pkg/framework"
+	"github.com/m88i/nexus-operator/pkg/logger"
 )
 
 var log = logger.GetLogger("controller_nexus")

diff --git a/pkg/controller/nexus/nexus_controller_test.go b/pkg/controller/nexus/nexus_controller_test.go
@@ -17,15 +17,10 @@ package nexus
 import (
 	"context"
 	"fmt"
-	"testing"
-
 	"reflect"
+	"testing"
 
 	resUtils "github.com/RHsyseng/operator-utils/pkg/resource"
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
-	nexusres "github.com/m88i/nexus-operator/pkg/controller/nexus/resource"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/validation"
-	"github.com/m88i/nexus-operator/pkg/test"
 	routev1 "github.com/openshift/api/route/v1"
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
@@ -37,6 +32,11 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	nexusres "github.com/m88i/nexus-operator/pkg/controller/nexus/resource"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/validation"
+	"github.com/m88i/nexus-operator/pkg/test"
 )
 
 func TestReconcileNexus_Reconcile_NoInstance(t *testing.T) {

diff --git a/pkg/controller/nexus/resource/deployment/deployment.go b/pkg/controller/nexus/resource/deployment/deployment.go
@@ -20,14 +20,14 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
-
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
 )
 
 const (

diff --git a/pkg/controller/nexus/resource/deployment/deployment_test.go b/pkg/controller/nexus/resource/deployment/deployment_test.go
@@ -18,14 +18,14 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/validation"
-
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/validation"
 )
 
 func Test_newDeployment_WithoutPersistence(t *testing.T) {

diff --git a/pkg/controller/nexus/resource/deployment/manager.go b/pkg/controller/nexus/resource/deployment/manager.go
@@ -17,29 +17,32 @@ package deployment
 import (
 	"fmt"
 	"reflect"
-
 	"strings"
 
 	"github.com/RHsyseng/operator-utils/pkg/resource"
 	"github.com/RHsyseng/operator-utils/pkg/resource/compare"
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
-	"github.com/m88i/nexus-operator/pkg/framework"
+	"go.uber.org/zap"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/framework"
+	"github.com/m88i/nexus-operator/pkg/logger"
 )
 
 var managedObjectsRef = map[string]resource.KubernetesResource{
-	"Service":    &corev1.Service{},
-	"Deployment": &appsv1.Deployment{},
+	framework.DeploymentKind: &appsv1.Deployment{},
+	framework.ServiceKind:    &corev1.Service{},
 }
 
 // Manager is responsible for creating deployment-related resources, fetching deployed ones and comparing them
 // Use with zero values will result in a panic. Use the NewManager function to get a properly initialized manager
 type Manager struct {
 	nexus  *v1alpha1.Nexus
 	client client.Client
+	log    *zap.SugaredLogger
 }
 
 // NewManager creates a deployment resources manager
@@ -48,22 +51,25 @@ func NewManager(nexus *v1alpha1.Nexus, client client.Client) *Manager {
 	return &Manager{
 		nexus:  nexus,
 		client: client,
+		log:    logger.GetLoggerWithResource("deployment_manager", nexus),
 	}
 }
 
 // GetRequiredResources returns the resources initialized by the manager
 func (m *Manager) GetRequiredResources() ([]resource.KubernetesResource, error) {
+	m.log.Debugf("Generating required %s", framework.DeploymentKind)
+	m.log.Debugf("Generating required %s", framework.ServiceKind)
 	return []resource.KubernetesResource{newDeployment(m.nexus), newService(m.nexus)}, nil
 }
 
 // GetDeployedResources returns the deployment-related resources deployed on the cluster
 func (m *Manager) GetDeployedResources() ([]resource.KubernetesResource, error) {
 	var resources []resource.KubernetesResource
 	for resType, resRef := range managedObjectsRef {
-		if err := framework.Fetch(m.client, framework.Key(m.nexus), resRef); err == nil {
+		if err := framework.Fetch(m.client, framework.Key(m.nexus), resRef, resType); err == nil {
 			resources = append(resources, resRef)
 		} else if !errors.IsNotFound(err) {
-			return nil, fmt.Errorf("could not fetch Resource %s (%s): %v", resType, m.nexus.Name, err)
+			return nil, fmt.Errorf("could not fetch %s (%s/%s): %v", resType, m.nexus.Namespace, m.nexus.Name, err)
 		}
 	}
 	return resources, nil
@@ -113,6 +119,11 @@ func deploymentEqual(deployed resource.KubernetesResource, requested resource.Ku
 
 	equal := compare.EqualPairs(pairs)
 	equal = equal && equalPullPolicies(depDeployment, reqDeployment)
+
+	if !equal {
+		logger.GetLogger("deployment_manager").Info("Resources are not equal", "deployed", deployed, "requested", requested)
+	}
+
 	return equal
 }
 

diff --git a/pkg/controller/nexus/resource/deployment/manager_test.go b/pkg/controller/nexus/resource/deployment/manager_test.go
@@ -20,14 +20,16 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/validation"
-	"github.com/m88i/nexus-operator/pkg/test"
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/validation"
+	"github.com/m88i/nexus-operator/pkg/logger"
+	"github.com/m88i/nexus-operator/pkg/test"
 )
 
 var (
@@ -54,9 +56,8 @@ func TestNewManager(t *testing.T) {
 		client: client,
 	}
 	got := NewManager(nexus, client)
-	if !reflect.DeepEqual(want, got) {
-		t.Errorf("TestNewManager()\nWant: %+v\tGot: %+v", want, got)
-	}
+	assert.Equal(t, want.nexus, got.nexus)
+	assert.Equal(t, want.client, got.client)
 }
 
 func TestManager_GetRequiredResources(t *testing.T) {
@@ -65,6 +66,7 @@ func TestManager_GetRequiredResources(t *testing.T) {
 	mgr := &Manager{
 		nexus:  allDefaultsCommunityNexus,
 		client: test.NewFakeClientBuilder().Build(),
+		log:    logger.GetLoggerWithResource("test", allDefaultsCommunityNexus),
 	}
 	resources, err := mgr.GetRequiredResources()
 	assert.Nil(t, err)

diff --git a/pkg/controller/nexus/resource/deployment/service.go b/pkg/controller/nexus/resource/deployment/service.go
@@ -15,10 +15,11 @@
 package deployment
 
 import (
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
 )
 
 const (

diff --git a/pkg/controller/nexus/resource/deployment/service_test.go b/pkg/controller/nexus/resource/deployment/service_test.go
@@ -17,10 +17,11 @@ package deployment
 import (
 	"testing"
 
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
 )
 
 func Test_newService(t *testing.T) {

diff --git a/pkg/controller/nexus/resource/interfaces.go b/pkg/controller/nexus/resource/interfaces.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/RHsyseng/operator-utils/pkg/resource"
 	"github.com/RHsyseng/operator-utils/pkg/resource/compare"
+
 	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
 )
 

diff --git a/pkg/controller/nexus/resource/meta/object.go b/pkg/controller/nexus/resource/meta/object.go
@@ -15,8 +15,9 @@
 package meta
 
 import (
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
 )
 
 const AppLabel = "app"

diff --git a/pkg/controller/nexus/resource/networking/ingress.go b/pkg/controller/nexus/resource/networking/ingress.go
@@ -15,11 +15,12 @@
 package networking
 
 import (
+	"k8s.io/api/networking/v1beta1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+
 	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
 	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/deployment"
 	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/meta"
-	"k8s.io/api/networking/v1beta1"
-	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 const (

diff --git a/pkg/controller/nexus/resource/networking/ingress_test.go b/pkg/controller/nexus/resource/networking/ingress_test.go
@@ -17,13 +17,14 @@ package networking
 import (
 	"testing"
 
-	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
-	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/deployment"
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/api/networking/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+
+	"github.com/m88i/nexus-operator/pkg/apis/apps/v1alpha1"
+	"github.com/m88i/nexus-operator/pkg/controller/nexus/resource/deployment"
 )
 
 var (