Skip to content

Commit f474cb0

Browse files
cbandycbandy
committed
Get logger from the passed context
1 parent 981e620 commit f474cb0

File tree

4 files changed

+85
-91
lines changed

4 files changed

+85
-91
lines changed

‎internal/upgradecheck/header.go‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"encoding/json"
2121
"net/http"
2222

23-
"github.com/go-logr/logr"
2423
googleuuid "github.com/google/uuid"
2524
corev1 "k8s.io/api/core/v1"
2625
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -29,6 +28,7 @@ import (
2928
"k8s.io/client-go/rest"
3029
crclient "sigs.k8s.io/controller-runtime/pkg/client"
3130

31+
"github.com/crunchydata/postgres-operator/internal/logging"
3232
"github.com/crunchydata/postgres-operator/internal/naming"
3333
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
3434
)
@@ -54,27 +54,27 @@ type clientUpgradeData struct {
5454
// generateHeader aggregates data and returns a struct of that data
5555
// If any errors are encountered, it logs those errors and uses the default values
5656
func generateHeader(ctx context.Context, cfg *rest.Config, crClient crclient.Client,
57-
log logr.Logger, pgoVersion string, isOpenShift bool) *clientUpgradeData {
57+
pgoVersion string, isOpenShift bool) *clientUpgradeData {
5858

5959
return &clientUpgradeData{
6060
PGOVersion: pgoVersion,
6161
IsOpenShift: isOpenShift,
62-
DeploymentID: ensureDeploymentID(ctx, crClient, log),
63-
PGOClustersTotal: getManagedClusters(ctx, crClient, log),
64-
KubernetesEnv: getServerVersion(cfg, log),
62+
DeploymentID: ensureDeploymentID(ctx, crClient),
63+
PGOClustersTotal: getManagedClusters(ctx, crClient),
64+
KubernetesEnv: getServerVersion(ctx, cfg),
6565
}
6666
}
6767

6868
// ensureDeploymentID checks if the UUID exists in memory or in a ConfigMap
6969
// If no UUID exists, ensureDeploymentID creates one and saves it in memory/as a ConfigMap
7070
// Any errors encountered will be logged and the ID result will be what is in memory
71-
func ensureDeploymentID(ctx context.Context, crClient crclient.Client, log logr.Logger) string {
71+
func ensureDeploymentID(ctx context.Context, crClient crclient.Client) string {
7272
// If there is no deploymentID in memory, generate one for possible use
7373
if deploymentID == "" {
7474
deploymentID = string(uuid.NewUUID())
7575
}
7676

77-
cm := manageUpgradeCheckConfigMap(ctx, crClient, log, deploymentID)
77+
cm := manageUpgradeCheckConfigMap(ctx, crClient, deploymentID)
7878

7979
if cm != nil && cm.Data["deployment_id"] != "" {
8080
deploymentID = cm.Data["deployment_id"]
@@ -88,8 +88,9 @@ func ensureDeploymentID(ctx context.Context, crClient crclient.Client, log logr.
8888
// If it exists and it has a valid UUID, use that to replace the in-memory ID
8989
// If it exists but the field is blank or mangled, we update the ConfigMap with the in-memory ID
9090
func manageUpgradeCheckConfigMap(ctx context.Context, crClient crclient.Client,
91-
log logr.Logger, currentID string) *corev1.ConfigMap {
91+
currentID string) *corev1.ConfigMap {
9292

93+
log := logging.FromContext(ctx)
9394
upgradeCheckConfigMapMetadata := naming.UpgradeCheckConfigMap()
9495

9596
cm := &corev1.ConfigMap{
@@ -154,11 +155,12 @@ func applyConfigMap(ctx context.Context, crClient crclient.Client,
154155

155156
// getManagedClusters returns a count of postgres clusters managed by this PGO instance
156157
// Any errors encountered will be logged and the count result will be 0
157-
func getManagedClusters(ctx context.Context, crClient crclient.Client, log logr.Logger) int {
158+
func getManagedClusters(ctx context.Context, crClient crclient.Client) int {
158159
var count int
159160
clusters := &v1beta1.PostgresClusterList{}
160161
err := crClient.List(ctx, clusters)
161162
if err != nil {
163+
log := logging.FromContext(ctx)
162164
log.V(1).Info("upgrade check issue: could not count postgres clusters",
163165
"response", err.Error())
164166
} else {
@@ -170,7 +172,8 @@ func getManagedClusters(ctx context.Context, crClient crclient.Client, log logr.
170172
// getServerVersion returns the stringified server version (i.e., the same info `kubectl version`
171173
// returns for the server)
172174
// Any errors encountered will be logged and will return an empty string
173-
func getServerVersion(cfg *rest.Config, log logr.Logger) string {
175+
func getServerVersion(ctx context.Context, cfg *rest.Config) string {
176+
log := logging.FromContext(ctx)
174177
discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg)
175178
if err != nil {
176179
log.V(1).Info("upgrade check issue: could not retrieve discovery client",

‎internal/upgradecheck/header_test.go‎

Lines changed: 54 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"strings"
2727
"testing"
2828

29-
"github.com/go-logr/logr"
3029
"github.com/wojas/genericr"
3130
"gotest.tools/v3/assert"
3231
corev1 "k8s.io/api/core/v1"
@@ -45,6 +44,7 @@ import (
4544

4645
"github.com/crunchydata/postgres-operator/internal/controller/postgrescluster"
4746
"github.com/crunchydata/postgres-operator/internal/controller/runtime"
47+
"github.com/crunchydata/postgres-operator/internal/logging"
4848
"github.com/crunchydata/postgres-operator/internal/naming"
4949
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
5050
)
@@ -125,7 +125,7 @@ func setupVersionServer(t *testing.T, works bool) (version.Info, *httptest.Serve
125125
Minor: "22",
126126
GitCommit: "v1.22.2",
127127
}
128-
return expect, httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter,
128+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter,
129129
req *http.Request) {
130130
if works {
131131
output, _ := json.Marshal(expect)
@@ -137,15 +137,17 @@ func setupVersionServer(t *testing.T, works bool) (version.Info, *httptest.Serve
137137
w.WriteHeader(http.StatusBadRequest)
138138
}
139139
}))
140+
t.Cleanup(server.Close)
141+
142+
return expect, server
140143
}
141144

142-
func setupLogCapture(t *testing.T) (*[]string, logr.Logger) {
143-
t.Helper()
145+
func setupLogCapture(ctx context.Context) (context.Context, *[]string) {
144146
calls := []string{}
145147
testlog := genericr.New(func(input genericr.Entry) {
146148
calls = append(calls, input.Message)
147149
})
148-
return &calls, testlog
150+
return logging.NewContext(ctx, testlog), &calls
149151
}
150152

151153
func TestGenerateHeader(t *testing.T) {
@@ -177,10 +179,10 @@ func TestGenerateHeader(t *testing.T) {
177179
fakeClientWithOptionalError := &fakeClientWithError{
178180
cc, "patch error",
179181
}
180-
calls, testlog := setupLogCapture(t)
182+
ctx, calls := setupLogCapture(ctx)
181183

182184
res := generateHeader(ctx, cfg, fakeClientWithOptionalError,
183-
testlog, "1.2.3", reconciler.IsOpenShift)
185+
"1.2.3", reconciler.IsOpenShift)
184186
assert.Equal(t, len(*calls), 1)
185187
assert.Equal(t, (*calls)[0], `upgrade check issue: could not apply configmap`)
186188
assert.Equal(t, res.IsOpenShift, reconciler.IsOpenShift)
@@ -197,10 +199,10 @@ func TestGenerateHeader(t *testing.T) {
197199
fakeClientWithOptionalError := &fakeClientWithError{
198200
cc, "list error",
199201
}
200-
calls, testlog := setupLogCapture(t)
202+
ctx, calls := setupLogCapture(ctx)
201203

202204
res := generateHeader(ctx, cfg, fakeClientWithOptionalError,
203-
testlog, "1.2.3", reconciler.IsOpenShift)
205+
"1.2.3", reconciler.IsOpenShift)
204206
assert.Equal(t, len(*calls), 1)
205207
assert.Equal(t, (*calls)[0], `upgrade check issue: could not count postgres clusters`)
206208
assert.Equal(t, res.IsOpenShift, reconciler.IsOpenShift)
@@ -211,11 +213,11 @@ func TestGenerateHeader(t *testing.T) {
211213
})
212214

213215
t.Run("error getting server version info", func(t *testing.T) {
214-
calls, testlog := setupLogCapture(t)
216+
ctx, calls := setupLogCapture(ctx)
215217
badcfg := &rest.Config{}
216218

217219
res := generateHeader(ctx, badcfg, cc,
218-
testlog, "1.2.3", reconciler.IsOpenShift)
220+
"1.2.3", reconciler.IsOpenShift)
219221
assert.Equal(t, len(*calls), 1)
220222
assert.Equal(t, (*calls)[0], `upgrade check issue: could not retrieve server version`)
221223
assert.Equal(t, res.IsOpenShift, reconciler.IsOpenShift)
@@ -229,10 +231,10 @@ func TestGenerateHeader(t *testing.T) {
229231
})
230232

231233
t.Run("success", func(t *testing.T) {
232-
calls, testlog := setupLogCapture(t)
234+
ctx, calls := setupLogCapture(ctx)
233235

234236
res := generateHeader(ctx, cfg, cc,
235-
testlog, "1.2.3", reconciler.IsOpenShift)
237+
"1.2.3", reconciler.IsOpenShift)
236238
assert.Equal(t, len(*calls), 0)
237239
assert.Equal(t, res.IsOpenShift, reconciler.IsOpenShift)
238240
assert.Equal(t, deploymentID, res.DeploymentID)
@@ -261,9 +263,9 @@ func TestEnsureID(t *testing.T) {
261263
t.Run("success, no id set in mem or configmap", func(t *testing.T) {
262264
deploymentID = ""
263265
oldID := deploymentID
264-
calls, testlog := setupLogCapture(t)
266+
ctx, calls := setupLogCapture(ctx)
265267

266-
newID := ensureDeploymentID(ctx, cc, testlog)
268+
newID := ensureDeploymentID(ctx, cc)
267269
assert.Equal(t, len(*calls), 0)
268270
assert.Assert(t, newID != oldID)
269271
assert.Assert(t, newID == deploymentID)
@@ -283,9 +285,9 @@ func TestEnsureID(t *testing.T) {
283285
err := cc.Get(ctx, naming.AsObjectKey(
284286
naming.UpgradeCheckConfigMap()), cm)
285287
assert.Error(t, err, `configmaps "pgo-upgrade-check" not found`)
286-
calls, testlog := setupLogCapture(t)
288+
ctx, calls := setupLogCapture(ctx)
287289

288-
newID := ensureDeploymentID(ctx, cc, testlog)
290+
newID := ensureDeploymentID(ctx, cc)
289291
assert.Equal(t, len(*calls), 0)
290292
assert.Assert(t, newID == oldID)
291293
assert.Assert(t, newID == deploymentID)
@@ -315,8 +317,8 @@ func TestEnsureID(t *testing.T) {
315317
assert.NilError(t, err)
316318

317319
oldID := setupDeploymentID(t)
318-
calls, testlog := setupLogCapture(t)
319-
newID := ensureDeploymentID(ctx, cc, testlog)
320+
ctx, calls := setupLogCapture(ctx)
321+
newID := ensureDeploymentID(ctx, cc)
320322
assert.Equal(t, len(*calls), 0)
321323
assert.Assert(t, newID != oldID)
322324
assert.Assert(t, newID == deploymentID)
@@ -342,13 +344,10 @@ func TestEnsureID(t *testing.T) {
342344
assert.NilError(t, err)
343345

344346
oldID := setupDeploymentID(t)
345-
calls, testlog := setupLogCapture(t)
346-
oldEnvVar := os.Getenv("PGO_NAMESPACE")
347-
os.Setenv("PGO_NAMESPACE", "")
347+
ctx, calls := setupLogCapture(ctx)
348+
t.Setenv("PGO_NAMESPACE", "")
348349

349-
newID := ensureDeploymentID(ctx, cc, testlog)
350-
// reset the var before testing so that errors here do not interfere with subsequent tests
351-
os.Setenv("PGO_NAMESPACE", oldEnvVar)
350+
newID := ensureDeploymentID(ctx, cc)
352351
assert.Equal(t, len(*calls), 1)
353352
assert.Equal(t, (*calls)[0], `upgrade check issue: namespace not set`)
354353
assert.Assert(t, newID == oldID)
@@ -363,9 +362,9 @@ func TestEnsureID(t *testing.T) {
363362
cc, "get error",
364363
}
365364
oldID := setupDeploymentID(t)
366-
calls, testlog := setupLogCapture(t)
365+
ctx, calls := setupLogCapture(ctx)
367366

368-
newID := ensureDeploymentID(ctx, fakeClientWithOptionalError, testlog)
367+
newID := ensureDeploymentID(ctx, fakeClientWithOptionalError)
369368
assert.Equal(t, len(*calls), 1)
370369
assert.Equal(t, (*calls)[0], `upgrade check issue: error retrieving configmap`)
371370
assert.Assert(t, newID == oldID)
@@ -383,8 +382,8 @@ func TestEnsureID(t *testing.T) {
383382
}
384383
oldID := setupDeploymentID(t)
385384

386-
calls, testlog := setupLogCapture(t)
387-
newID := ensureDeploymentID(ctx, fakeClientWithOptionalError, testlog)
385+
ctx, calls := setupLogCapture(ctx)
386+
newID := ensureDeploymentID(ctx, fakeClientWithOptionalError)
388387
assert.Equal(t, len(*calls), 1)
389388
assert.Equal(t, (*calls)[0], `upgrade check issue: could not apply configmap`)
390389
assert.Assert(t, newID == oldID)
@@ -406,13 +405,10 @@ func TestManageUpgradeCheckConfigMap(t *testing.T) {
406405
assert.NilError(t, err)
407406

408407
t.Run("no namespace given", func(t *testing.T) {
409-
calls, testlog := setupLogCapture(t)
410-
oldEnvVar := os.Getenv("PGO_NAMESPACE")
411-
os.Setenv("PGO_NAMESPACE", "")
408+
ctx, calls := setupLogCapture(ctx)
409+
t.Setenv("PGO_NAMESPACE", "")
412410

413-
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, testlog, "current-id")
414-
// reset the var before testing so that errors here do not interfere with subsequent tests
415-
os.Setenv("PGO_NAMESPACE", oldEnvVar)
411+
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, "current-id")
416412
assert.Equal(t, len(*calls), 1)
417413
assert.Equal(t, (*calls)[0], `upgrade check issue: namespace not set`)
418414
assert.Assert(t, returnedCM.Data["deployment_id"] == "current-id")
@@ -424,8 +420,8 @@ func TestManageUpgradeCheckConfigMap(t *testing.T) {
424420
naming.UpgradeCheckConfigMap()), cmRetrieved)
425421
assert.Error(t, err, `configmaps "pgo-upgrade-check" not found`)
426422

427-
calls, testlog := setupLogCapture(t)
428-
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, testlog, "current-id")
423+
ctx, calls := setupLogCapture(ctx)
424+
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, "current-id")
429425

430426
assert.Equal(t, len(*calls), 0)
431427
assert.Assert(t, returnedCM.Data["deployment_id"] == "current-id")
@@ -437,10 +433,10 @@ func TestManageUpgradeCheckConfigMap(t *testing.T) {
437433
fakeClientWithOptionalError := &fakeClientWithError{
438434
cc, "get error",
439435
}
440-
calls, testlog := setupLogCapture(t)
436+
ctx, calls := setupLogCapture(ctx)
441437

442438
returnedCM := manageUpgradeCheckConfigMap(ctx, fakeClientWithOptionalError,
443-
testlog, "current-id")
439+
"current-id")
444440
assert.Equal(t, len(*calls), 1)
445441
assert.Equal(t, (*calls)[0], `upgrade check issue: error retrieving configmap`)
446442
assert.Assert(t, returnedCM.Data["deployment_id"] == "current-id")
@@ -461,8 +457,8 @@ func TestManageUpgradeCheckConfigMap(t *testing.T) {
461457
naming.UpgradeCheckConfigMap()), cmRetrieved)
462458
assert.NilError(t, err)
463459

464-
calls, testlog := setupLogCapture(t)
465-
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, testlog, "current-id")
460+
ctx, calls := setupLogCapture(ctx)
461+
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, "current-id")
466462
assert.Equal(t, len(*calls), 0)
467463
assert.Assert(t, returnedCM.Data["deployment_id"] == "current-id")
468464
err = cc.Delete(ctx, cm)
@@ -484,8 +480,8 @@ func TestManageUpgradeCheckConfigMap(t *testing.T) {
484480
naming.UpgradeCheckConfigMap()), cmRetrieved)
485481
assert.NilError(t, err)
486482

487-
calls, testlog := setupLogCapture(t)
488-
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, testlog, "current-id")
483+
ctx, calls := setupLogCapture(ctx)
484+
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, "current-id")
489485
assert.Equal(t, len(*calls), 0)
490486
assert.Assert(t, returnedCM.Data["deployment_id"] == "current-id")
491487
err = cc.Delete(ctx, cm)
@@ -507,8 +503,8 @@ func TestManageUpgradeCheckConfigMap(t *testing.T) {
507503
naming.UpgradeCheckConfigMap()), cmRetrieved)
508504
assert.NilError(t, err)
509505

510-
calls, testlog := setupLogCapture(t)
511-
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, testlog, "current-id")
506+
ctx, calls := setupLogCapture(ctx)
507+
returnedCM := manageUpgradeCheckConfigMap(ctx, cc, "current-id")
512508
assert.Equal(t, len(*calls), 0)
513509
assert.Assert(t, returnedCM.Data["deployment-id"] != "current-id")
514510
err = cc.Delete(ctx, cm)
@@ -520,9 +516,9 @@ func TestManageUpgradeCheckConfigMap(t *testing.T) {
520516
cc, "patch error",
521517
}
522518

523-
calls, testlog := setupLogCapture(t)
519+
ctx, calls := setupLogCapture(ctx)
524520
returnedCM := manageUpgradeCheckConfigMap(ctx, fakeClientWithOptionalError,
525-
testlog, "current-id")
521+
"current-id")
526522
assert.Equal(t, len(*calls), 1)
527523
assert.Equal(t, (*calls)[0], `upgrade check issue: could not apply configmap`)
528524
assert.Assert(t, returnedCM.Data["deployment_id"] == "current-id")
@@ -652,8 +648,8 @@ func TestGetManagedClusters(t *testing.T) {
652648

653649
t.Run("success", func(t *testing.T) {
654650
fakeClient := setupFakeClientWithPGOScheme(t, true)
655-
calls, testlog := setupLogCapture(t)
656-
count := getManagedClusters(ctx, fakeClient, testlog)
651+
ctx, calls := setupLogCapture(ctx)
652+
count := getManagedClusters(ctx, fakeClient)
657653
assert.Equal(t, len(*calls), 0)
658654
assert.Assert(t, count == 2)
659655
})
@@ -662,8 +658,8 @@ func TestGetManagedClusters(t *testing.T) {
662658
fakeClientWithOptionalError := &fakeClientWithError{
663659
setupFakeClientWithPGOScheme(t, true), "list error",
664660
}
665-
calls, testlog := setupLogCapture(t)
666-
count := getManagedClusters(ctx, fakeClientWithOptionalError, testlog)
661+
ctx, calls := setupLogCapture(ctx)
662+
count := getManagedClusters(ctx, fakeClientWithOptionalError)
667663
assert.Equal(t, len(*calls), 1)
668664
assert.Equal(t, (*calls)[0], `upgrade check issue: could not count postgres clusters`)
669665
assert.Assert(t, count == 0)
@@ -673,23 +669,22 @@ func TestGetManagedClusters(t *testing.T) {
673669
func TestGetServerVersion(t *testing.T) {
674670
t.Run("success", func(t *testing.T) {
675671
expect, server := setupVersionServer(t, true)
676-
defer server.Close()
677-
calls, testlog := setupLogCapture(t)
678-
got := getServerVersion(&rest.Config{
672+
ctx, calls := setupLogCapture(context.Background())
673+
674+
got := getServerVersion(ctx, &rest.Config{
679675
Host: server.URL,
680-
}, testlog)
676+
})
681677
assert.Equal(t, len(*calls), 0)
682678
assert.Equal(t, got, expect.String())
683679
})
684680

685681
t.Run("failure", func(t *testing.T) {
686682
_, server := setupVersionServer(t, false)
687-
defer server.Close()
683+
ctx, calls := setupLogCapture(context.Background())
688684

689-
calls, testlog := setupLogCapture(t)
690-
got := getServerVersion(&rest.Config{
685+
got := getServerVersion(ctx, &rest.Config{
691686
Host: server.URL,
692-
}, testlog)
687+
})
693688
assert.Equal(t, len(*calls), 1)
694689
assert.Equal(t, (*calls)[0], `upgrade check issue: could not retrieve server version`)
695690
assert.Equal(t, got, "")

0 commit comments

Comments
 (0)