Skip to content

Commit 9eec8f8

Browse files
committed
Log out header information
Add header information as to existing logs, updated to INFO level from DEBUG, and update related Go tests to match changes. Issue: [sc-13581]
1 parent dc8c01a commit 9eec8f8

File tree

2 files changed

+28
-11
lines changed

2 files changed

+28
-11
lines changed

internal/upgradecheck/http.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func init() {
7575

7676
func checkForUpgrades(ctx context.Context, versionString string, backoff wait.Backoff,
7777
crclient crclient.Client, cfg *rest.Config,
78-
isOpenShift bool) (message string, err error) {
78+
isOpenShift bool) (message string, header string, err error) {
7979
var headerPayloadStruct *clientUpgradeData
8080

8181
// Guard against panics within the checkForUpgrades function to allow the
@@ -137,7 +137,7 @@ func checkForUpgrades(ctx context.Context, versionString string, backoff wait.Ba
137137
}
138138

139139
// TODO: Parse response and log info for user on potential upgrades
140-
return string(bodyBytes), err
140+
return string(bodyBytes), req.Header.Get(clientHeader), err
141141
}
142142

143143
// CheckForUpgradesScheduler invokes the check func when the operator starts
@@ -172,26 +172,26 @@ func CheckForUpgradesScheduler(ctx context.Context,
172172
return
173173
}
174174

175-
info, err := checkForUpgrades(ctx, versionString, backoff,
175+
info, header, err := checkForUpgrades(ctx, versionString, backoff,
176176
crclient, cfg, isOpenShift)
177177
if err != nil {
178178
log.V(1).Info("could not complete upgrade check",
179179
"response", err.Error())
180180
} else {
181-
log.V(1).Info(info)
181+
log.Info(info, clientHeader, header)
182182
}
183183

184184
ticker := time.NewTicker(upgradeCheckPeriod)
185185
for {
186186
select {
187187
case <-ticker.C:
188-
info, err = checkForUpgrades(ctx, versionString, backoff,
188+
info, header, err = checkForUpgrades(ctx, versionString, backoff,
189189
crclient, cfg, isOpenShift)
190190
if err != nil {
191191
log.V(1).Info("could not complete scheduled upgrade check",
192192
"response", err.Error())
193193
} else {
194-
log.V(1).Info(info)
194+
log.Info(info, clientHeader, header)
195195
}
196196
case <-ctx.Done():
197197
ticker.Stop()

internal/upgradecheck/http_test.go

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package upgradecheck
1717

1818
import (
1919
"context"
20+
"encoding/json"
2021
"errors"
2122
"fmt"
2223
"io"
@@ -67,6 +68,16 @@ func TestCheckForUpgrades(t *testing.T) {
6768
ctx := logging.NewContext(context.Background(), logging.Discard())
6869
cfg := &rest.Config{}
6970

71+
// Pass *testing.T to allows the correct messages from the assert package
72+
// in the event of certain failures.
73+
checkData := func(t *testing.T, header string) {
74+
data := clientUpgradeData{}
75+
err := json.Unmarshal([]byte(header), &data)
76+
assert.NilError(t, err)
77+
assert.Assert(t, data.DeploymentID != "")
78+
assert.Equal(t, data.PGOVersion, "4.7.3")
79+
}
80+
7081
t.Run("success", func(t *testing.T) {
7182
// A successful call
7283
funcFoo = func() (*http.Response, error) {
@@ -77,10 +88,11 @@ func TestCheckForUpgrades(t *testing.T) {
7788
}, nil
7889
}
7990

80-
res, err := checkForUpgrades(ctx, "4.7.3", backoff,
91+
res, header, err := checkForUpgrades(ctx, "4.7.3", backoff,
8192
fakeClient, cfg, false)
8293
assert.NilError(t, err)
8394
assert.Equal(t, res, `{"pgo_versions":[{"tag":"v5.0.4"},{"tag":"v5.0.3"},{"tag":"v5.0.2"},{"tag":"v5.0.1"},{"tag":"v5.0.0"}]}`)
95+
checkData(t, header)
8496
})
8597

8698
t.Run("total failure, err sending", func(t *testing.T) {
@@ -91,12 +103,13 @@ func TestCheckForUpgrades(t *testing.T) {
91103
return &http.Response{}, errors.New("whoops")
92104
}
93105

94-
res, err := checkForUpgrades(ctx, "4.7.3", backoff,
106+
res, header, err := checkForUpgrades(ctx, "4.7.3", backoff,
95107
fakeClient, cfg, false)
96108
// Two failed calls because of env var
97109
assert.Equal(t, counter, 2)
98110
assert.Equal(t, res, "")
99111
assert.Equal(t, err.Error(), `whoops`)
112+
checkData(t, header)
100113
})
101114

102115
t.Run("recovers from panic", func(t *testing.T) {
@@ -107,12 +120,14 @@ func TestCheckForUpgrades(t *testing.T) {
107120
panic(fmt.Errorf("oh no!"))
108121
}
109122

110-
res, err := checkForUpgrades(ctx, "4.7.3", backoff,
123+
res, header, err := checkForUpgrades(ctx, "4.7.3", backoff,
111124
fakeClient, cfg, false)
112125
// One call because of panic
113126
assert.Equal(t, counter, 1)
114127
assert.Equal(t, res, "")
115128
assert.Equal(t, err.Error(), `oh no!`)
129+
// no http response returned, so don't perform full check
130+
assert.Assert(t, header == "")
116131
})
117132

118133
t.Run("total failure, bad StatusCode", func(t *testing.T) {
@@ -126,12 +141,13 @@ func TestCheckForUpgrades(t *testing.T) {
126141
}, nil
127142
}
128143

129-
res, err := checkForUpgrades(ctx, "4.7.3", backoff,
144+
res, header, err := checkForUpgrades(ctx, "4.7.3", backoff,
130145
fakeClient, cfg, false)
131146
assert.Equal(t, res, "")
132147
// Two failed calls because of env var
133148
assert.Equal(t, counter, 2)
134149
assert.Equal(t, err.Error(), `received StatusCode 400`)
150+
checkData(t, header)
135151
})
136152

137153
t.Run("one failure, then success", func(t *testing.T) {
@@ -154,11 +170,12 @@ func TestCheckForUpgrades(t *testing.T) {
154170
}, nil
155171
}
156172

157-
res, err := checkForUpgrades(ctx, "4.7.3", backoff,
173+
res, header, err := checkForUpgrades(ctx, "4.7.3", backoff,
158174
fakeClient, cfg, false)
159175
assert.Equal(t, counter, 2)
160176
assert.NilError(t, err)
161177
assert.Equal(t, res, `{"pgo_versions":[{"tag":"v5.0.4"},{"tag":"v5.0.3"},{"tag":"v5.0.2"},{"tag":"v5.0.1"},{"tag":"v5.0.0"}]}`)
178+
checkData(t, header)
162179
})
163180
}
164181

0 commit comments

Comments
 (0)