diff --git a/pkg/controller/istiocsr/certificates_test.go b/pkg/controller/istiocsr/certificates_test.go index e9bb417eb..46c4ba5f1 100644 --- a/pkg/controller/istiocsr/certificates_test.go +++ b/pkg/controller/istiocsr/certificates_test.go @@ -210,7 +210,7 @@ func TestCreateOrApplyCertificates(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/istiocsr/controller.go b/pkg/controller/istiocsr/controller.go index c25a6e985..8c9612a46 100644 --- a/pkg/controller/istiocsr/controller.go +++ b/pkg/controller/istiocsr/controller.go @@ -14,7 +14,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" - + "github.com/go-logr/logr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -22,8 +22,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/go-logr/logr" - certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" v1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" @@ -90,7 +88,10 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { } key := strings.Split(value, "_") if len(key) != 2 { - r.log.Error(fmt.Errorf("invalid label format"), "%s label value(%s) not in expected format on %s resource", IstiocsrResourceWatchLabelName, value, obj.GetName()) + r.log.V(2).Info("label value not in expected format", + "labelName", IstiocsrResourceWatchLabelName, + "labelValue", value, + "resource", obj.GetName()) return false } namespace = key[0] diff --git a/pkg/controller/istiocsr/controller_test.go b/pkg/controller/istiocsr/controller_test.go index 9d29a2d86..f16cadff8 100644 --- a/pkg/controller/istiocsr/controller_test.go +++ b/pkg/controller/istiocsr/controller_test.go @@ -410,7 +410,7 @@ func TestReconcile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) @@ -752,7 +752,7 @@ func TestProcessReconcileRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/istiocsr/deployments_test.go b/pkg/controller/istiocsr/deployments_test.go index fad4d8162..eb9333b6e 100644 --- a/pkg/controller/istiocsr/deployments_test.go +++ b/pkg/controller/istiocsr/deployments_test.go @@ -1069,7 +1069,7 @@ func TestCreateOrApplyDeployments(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/istiocsr/install_instiocsr_test.go b/pkg/controller/istiocsr/install_instiocsr_test.go index d128b1f00..653755bc5 100644 --- a/pkg/controller/istiocsr/install_instiocsr_test.go +++ b/pkg/controller/istiocsr/install_instiocsr_test.go @@ -115,7 +115,7 @@ func TestReconcileIstioCSRDeployment(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/istiocsr/networkpolicies.go b/pkg/controller/istiocsr/networkpolicies.go index 03bbca008..cafc00506 100644 --- a/pkg/controller/istiocsr/networkpolicies.go +++ b/pkg/controller/istiocsr/networkpolicies.go @@ -8,7 +8,6 @@ import ( networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common" "github.com/openshift/cert-manager-operator/pkg/operator/assets" diff --git a/pkg/controller/istiocsr/rbacs.go b/pkg/controller/istiocsr/rbacs.go index 22564f5fa..4f14c31dc 100644 --- a/pkg/controller/istiocsr/rbacs.go +++ b/pkg/controller/istiocsr/rbacs.go @@ -162,7 +162,7 @@ func (r *Reconciler) updateClusterRoleNameInStatus(istiocsr *v1alpha1.IstioCSR, if existing != nil && existing.GetName() != "" { name = existing.GetName() } else { - r.log.Error(fmt.Errorf("error updating clusterrole name in status"), "istiocsr", istiocsr.GetNamespace()) + r.log.V(1).Info("unable to determine clusterrole name for status update", "istiocsr", istiocsr.GetNamespace()) } } istiocsr.Status.ClusterRole = name @@ -261,7 +261,7 @@ func (r *Reconciler) updateClusterRoleBindingNameInStatus(istiocsr *v1alpha1.Ist if existing != nil && existing.GetName() != "" { name = existing.GetName() } else { - r.log.Error(fmt.Errorf("error updating clusterrolebinding name in status"), "istiocsr", istiocsr.GetNamespace()) + r.log.V(1).Info("unable to determine clusterrolebinding name for status update", "istiocsr", istiocsr.GetNamespace()) } } istiocsr.Status.ClusterRoleBinding = name diff --git a/pkg/controller/istiocsr/rbacs_test.go b/pkg/controller/istiocsr/rbacs_test.go index 8b9793bcb..1b875ea9f 100644 --- a/pkg/controller/istiocsr/rbacs_test.go +++ b/pkg/controller/istiocsr/rbacs_test.go @@ -683,7 +683,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/istiocsr/serviceaccounts_test.go b/pkg/controller/istiocsr/serviceaccounts_test.go index 82e0f9c4a..efeec2665 100644 --- a/pkg/controller/istiocsr/serviceaccounts_test.go +++ b/pkg/controller/istiocsr/serviceaccounts_test.go @@ -167,7 +167,7 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/istiocsr/services_test.go b/pkg/controller/istiocsr/services_test.go index 708a116a4..0187d70f3 100644 --- a/pkg/controller/istiocsr/services_test.go +++ b/pkg/controller/istiocsr/services_test.go @@ -101,7 +101,7 @@ func TestCreateOrApplyServices(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/istiocsr/test_utils.go b/pkg/controller/istiocsr/test_utils.go index f2276b9f2..6289069d9 100644 --- a/pkg/controller/istiocsr/test_utils.go +++ b/pkg/controller/istiocsr/test_utils.go @@ -9,7 +9,6 @@ import ( "encoding/pem" "fmt" "maps" - "testing" "time" appsv1 "k8s.io/api/apps/v1" @@ -17,8 +16,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" - - "github.com/go-logr/logr/testr" + "k8s.io/klog/v2" cmacme "github.com/cert-manager/cert-manager/pkg/apis/acme/v1" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" @@ -42,11 +40,11 @@ var ( type CertificateTweak func(*x509.Certificate) -func testReconciler(t *testing.T) *Reconciler { +func testReconciler() *Reconciler { return &Reconciler{ ctx: context.Background(), eventRecorder: record.NewFakeRecorder(100), - log: testr.New(t), + log: klog.NewKlogr().WithName(ControllerName), scheme: testutil.Scheme, } } diff --git a/pkg/controller/istiocsr/utils.go b/pkg/controller/istiocsr/utils.go index 896c8bbe6..676afc408 100644 --- a/pkg/controller/istiocsr/utils.go +++ b/pkg/controller/istiocsr/utils.go @@ -493,7 +493,7 @@ func (r *Reconciler) disallowMultipleIstioCSRInstances(istiocsr *v1alpha1.IstioC statusMessage := fmt.Sprintf("multiple instances of istiocsr exists, %s/%s will not be processed", istiocsr.GetNamespace(), istiocsr.GetName()) if containsProcessingRejectedAnnotation(istiocsr) { - r.log.V(4).Info("%s/%s istiocsr resource contains processing rejected annotation", istiocsr.Namespace, istiocsr.Name) + r.log.V(4).Info("istiocsr resource contains processing rejected annotation", "namespace", istiocsr.Namespace, "name", istiocsr.Name) // ensure status is updated. var updateErr error if istiocsr.Status.SetCondition(v1alpha1.Ready, metav1.ConditionFalse, v1alpha1.ReasonFailed, statusMessage) { diff --git a/pkg/controller/trustmanager/certificates_test.go b/pkg/controller/trustmanager/certificates_test.go index 122773ed8..d9511a3f6 100644 --- a/pkg/controller/trustmanager/certificates_test.go +++ b/pkg/controller/trustmanager/certificates_test.go @@ -255,7 +255,7 @@ func TestIssuerReconciliation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) @@ -398,7 +398,7 @@ func TestCertificateReconciliation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/trustmanager/configmaps_test.go b/pkg/controller/trustmanager/configmaps_test.go index 4629949b0..b51c3151e 100644 --- a/pkg/controller/trustmanager/configmaps_test.go +++ b/pkg/controller/trustmanager/configmaps_test.go @@ -311,7 +311,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} r.CtrlClient = mock diff --git a/pkg/controller/trustmanager/controller_test.go b/pkg/controller/trustmanager/controller_test.go index 3efd817a0..ee2b36c3e 100644 --- a/pkg/controller/trustmanager/controller_test.go +++ b/pkg/controller/trustmanager/controller_test.go @@ -119,7 +119,7 @@ func TestReconcile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) @@ -337,7 +337,7 @@ func TestProcessReconcileRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/trustmanager/deployments_test.go b/pkg/controller/trustmanager/deployments_test.go index 8e86c456d..5e473725b 100644 --- a/pkg/controller/trustmanager/deployments_test.go +++ b/pkg/controller/trustmanager/deployments_test.go @@ -57,7 +57,7 @@ func TestDeploymentObject(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) - r := testReconciler(t) + r := testReconciler() tm := tt.tm.Build() dep, err := r.getDeploymentObject(tm, getResourceLabels(tm), getResourceAnnotations(tm), "") if err != nil { @@ -86,7 +86,7 @@ func TestDeploymentObject(t *testing.T) { func TestDeploymentSpec(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) - r := testReconciler(t) + r := testReconciler() tm := testTrustManager().Build() dep, err := r.getDeploymentObject(tm, getResourceLabels(tm), getResourceAnnotations(tm), "") if err != nil { @@ -215,7 +215,7 @@ func TestDeploymentContainerArgs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) - r := testReconciler(t) + r := testReconciler() tmBuilder := tt.tmBuilder if tmBuilder == nil { @@ -247,7 +247,7 @@ func TestDeploymentDefaultCAPackage(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) t.Run("adds arg, volume, mount, and hash annotation when enabled", func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { @@ -294,7 +294,7 @@ func TestDeploymentDefaultCAPackage(t *testing.T) { }) t.Run("no arg, volume, or annotation when disabled", func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() tm := testTrustManager().Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "") if err != nil { @@ -384,7 +384,7 @@ func TestDeploymentOverrides(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) - r := testReconciler(t) + r := testReconciler() tm := testTrustManager().Build() tt.configure(tm) @@ -690,7 +690,7 @@ func TestDeploymentReconciliation(t *testing.T) { } else { t.Setenv(trustManagerImageNameEnvVarName, "") } - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go index 437022391..f3d0b416e 100644 --- a/pkg/controller/trustmanager/install_trustmanager_test.go +++ b/pkg/controller/trustmanager/install_trustmanager_test.go @@ -74,7 +74,7 @@ func TestUpdateStatusObservedState(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} tm := tt.trustManager() diff --git a/pkg/controller/trustmanager/rbacs_test.go b/pkg/controller/trustmanager/rbacs_test.go index 634bffc30..77e7cedf0 100644 --- a/pkg/controller/trustmanager/rbacs_test.go +++ b/pkg/controller/trustmanager/rbacs_test.go @@ -562,7 +562,7 @@ func TestRBACReconciliation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) @@ -775,7 +775,7 @@ func TestRBACReconciliationWithSecretTargets(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} tm := tt.tmBuilder.Build() if tt.preReq != nil { diff --git a/pkg/controller/trustmanager/serviceaccounts_test.go b/pkg/controller/trustmanager/serviceaccounts_test.go index 70c119582..d557204c1 100644 --- a/pkg/controller/trustmanager/serviceaccounts_test.go +++ b/pkg/controller/trustmanager/serviceaccounts_test.go @@ -46,7 +46,7 @@ func TestServiceAccountObject(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() tm := tt.tm.Build() sa := r.getServiceAccountObject(getResourceLabels(tm), getResourceAnnotations(tm)) @@ -171,7 +171,7 @@ func TestServiceAccountReconciliation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/trustmanager/services_test.go b/pkg/controller/trustmanager/services_test.go index 4c7432370..6e79b2320 100644 --- a/pkg/controller/trustmanager/services_test.go +++ b/pkg/controller/trustmanager/services_test.go @@ -250,7 +250,7 @@ func TestServiceReconciliation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock) diff --git a/pkg/controller/trustmanager/test_utils.go b/pkg/controller/trustmanager/test_utils.go index e0b96cc9d..b4c2ef45a 100644 --- a/pkg/controller/trustmanager/test_utils.go +++ b/pkg/controller/trustmanager/test_utils.go @@ -9,8 +9,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" - - "github.com/go-logr/logr/testr" + "k8s.io/klog/v2" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/testutil" @@ -106,11 +105,11 @@ func (b *trustManagerBuilder) Build() *v1alpha1.TrustManager { return b.TrustManager } -func testReconciler(t *testing.T) *Reconciler { +func testReconciler() *Reconciler { return &Reconciler{ ctx: context.Background(), eventRecorder: record.NewFakeRecorder(100), - log: testr.New(t), + log: klog.NewKlogr().WithName(ControllerName), scheme: testutil.Scheme, } } diff --git a/pkg/controller/trustmanager/webhooks_test.go b/pkg/controller/trustmanager/webhooks_test.go index 17841c685..b224c8788 100644 --- a/pkg/controller/trustmanager/webhooks_test.go +++ b/pkg/controller/trustmanager/webhooks_test.go @@ -226,7 +226,7 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) + r := testReconciler() mock := &fakes.FakeCtrlClient{} if tt.preReq != nil { tt.preReq(r, mock)