Skip to content

Commit dc898f4

Browse files
authored
Merge pull request #558 from saschagrunert/enhance-testfreeze-codefreeze
Enhance testfreeze plugin to distinguish Code Freeze and Test Freeze
2 parents c7d4b61 + be0f38b commit dc898f4

File tree

6 files changed

+459
-44
lines changed

6 files changed

+459
-44
lines changed

pkg/plugins/testfreeze/checker/checker.go

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,29 @@ import (
3131
"github.com/go-git/go-git/v5/plumbing"
3232
gitmemory "github.com/go-git/go-git/v5/storage/memory"
3333
"github.com/sirupsen/logrus"
34+
"gopkg.in/yaml.v3"
3435
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3536
v1 "sigs.k8s.io/prow/pkg/apis/prowjobs/v1"
3637
)
3738

3839
const (
39-
prowjobsURL = "https://prow.k8s.io/prowjobs.js?omit=annotations,labels,decoration_config,pod_spec"
40-
jobName = "ci-fast-forward"
41-
unknownTime = "unknown"
40+
prowjobsURL = "https://prow.k8s.io/prowjobs.js?omit=annotations,labels,decoration_config,pod_spec"
41+
prowConfigURL = "https://raw.githubusercontent.com/kubernetes/test-infra/master/config/prow/config.yaml"
42+
jobName = "ci-fast-forward"
43+
unknownTime = "unknown"
44+
kubernetesRepo = "kubernetes/kubernetes"
4245
)
4346

47+
// ProwConfig represents a simplified Prow configuration.
48+
type ProwConfig struct {
49+
Tide struct {
50+
Queries []struct {
51+
Repos []string `yaml:"repos"`
52+
ExcludedBranches []string `yaml:"excludedBranches"`
53+
} `yaml:"queries"`
54+
} `yaml:"tide"`
55+
}
56+
4457
// Checker is the main structure of checking if we're in Test Freeze.
4558
type Checker struct {
4659
checker checker
@@ -49,6 +62,9 @@ type Checker struct {
4962

5063
// Result is the result returned by `InTestFreeze`.
5164
type Result struct {
65+
// InCodeFreeze is true if we're in Code Freeze.
66+
InCodeFreeze bool
67+
5268
// InTestFreeze is true if we're in Test Freeze.
5369
InTestFreeze bool
5470

@@ -71,6 +87,7 @@ type checker interface {
7187
CloseBody(*http.Response) error
7288
ReadAllBody(*http.Response) ([]byte, error)
7389
UnmarshalProwJobs([]byte) (*v1.ProwJobList, error)
90+
UnmarshalProwConfig([]byte) (interface{}, error)
7491
}
7592

7693
type defaultChecker struct{}
@@ -83,8 +100,49 @@ func New(log *logrus.Entry) *Checker {
83100
}
84101
}
85102

103+
// inCodeFreeze checks if the given branch is excluded in the Prow Tide configuration,
104+
// which indicates Code Freeze is active.
105+
func (c *Checker) inCodeFreeze(branch string) (bool, error) {
106+
resp, err := c.checker.HttpGet(prowConfigURL)
107+
if err != nil {
108+
return false, fmt.Errorf("get prow config: %w", err)
109+
}
110+
defer c.checker.CloseBody(resp)
111+
112+
body, err := c.checker.ReadAllBody(resp)
113+
if err != nil {
114+
return false, fmt.Errorf("read response body: %w", err)
115+
}
116+
117+
configInterface, err := c.checker.UnmarshalProwConfig(body)
118+
if err != nil {
119+
return false, fmt.Errorf("unmarshal prow config: %w", err)
120+
}
121+
122+
prowConfig, ok := configInterface.(*ProwConfig)
123+
if !ok {
124+
return false, errors.New("failed to type assert prow config")
125+
}
126+
127+
// Check if the branch is excluded in any Tide query for kubernetes/kubernetes
128+
for _, query := range prowConfig.Tide.Queries {
129+
for _, repo := range query.Repos {
130+
if repo == kubernetesRepo {
131+
for _, excludedBranch := range query.ExcludedBranches {
132+
if excludedBranch == branch {
133+
return true, nil
134+
}
135+
}
136+
}
137+
}
138+
}
139+
140+
return false, nil
141+
}
142+
86143
// InTestFreeze returns if we're in Test Freeze:
87144
// https://github.com/kubernetes/sig-release/blob/2d8a1cc/releases/release_phases.md#test-freeze
145+
// It also checks for Code Freeze by examining the Prow Tide configuration.
88146
// It errors in case of any issue.
89147
func (c *Checker) InTestFreeze() (*Result, error) {
90148
remote := git.NewRemote(gitmemory.NewStorage(), &gitconfig.RemoteConfig{
@@ -146,7 +204,14 @@ func (c *Checker) InTestFreeze() (*Result, error) {
146204
// Found the latest minor version on the latest release branch,
147205
// which means we're not in Test Freeze.
148206
if latestSemver.EQ(parsed) {
207+
// Check if we're still in Code Freeze
208+
inCodeFreeze, err := c.inCodeFreeze(latestBranch)
209+
if err != nil {
210+
c.log.WithError(err).Error("Unable to check Code Freeze status.")
211+
}
212+
149213
return &Result{
214+
InCodeFreeze: inCodeFreeze,
150215
InTestFreeze: false,
151216
Branch: latestBranch,
152217
Tag: "v" + tag,
@@ -165,7 +230,14 @@ func (c *Checker) InTestFreeze() (*Result, error) {
165230

166231
// Latest minor version not found in latest release branch,
167232
// we're in Test Freeze.
233+
// Check if we're also in Code Freeze
234+
inCodeFreeze, err := c.inCodeFreeze(latestBranch)
235+
if err != nil {
236+
c.log.WithError(err).Error("Unable to check Code Freeze status.")
237+
}
238+
168239
return &Result{
240+
InCodeFreeze: inCodeFreeze,
169241
InTestFreeze: true,
170242
Branch: latestBranch,
171243
Tag: "v" + latestSemver.String(),
@@ -222,3 +294,11 @@ func (*defaultChecker) UnmarshalProwJobs(data []byte) (*v1.ProwJobList, error) {
222294
}
223295
return prowJobs, nil
224296
}
297+
298+
func (*defaultChecker) UnmarshalProwConfig(data []byte) (interface{}, error) {
299+
prowConfig := &ProwConfig{}
300+
if err := yaml.Unmarshal(data, prowConfig); err != nil {
301+
return nil, err
302+
}
303+
return prowConfig, nil
304+
}

pkg/plugins/testfreeze/checker/checker_test.go

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ package checker
1818

1919
import (
2020
"errors"
21+
"io"
22+
"net/http"
23+
"strings"
2124
"testing"
2225
"time"
2326

@@ -160,3 +163,213 @@ func TestInTestFreeze(t *testing.T) {
160163
})
161164
}
162165
}
166+
167+
func TestInCodeFreeze(t *testing.T) {
168+
t.Parallel()
169+
170+
errTest := errors.New("test error")
171+
172+
for _, tc := range []struct {
173+
name string
174+
branch string
175+
prepare func(*checkerfakes.FakeChecker)
176+
assert func(bool, error)
177+
}{
178+
{
179+
name: "success branch is excluded (code freeze active)",
180+
branch: "release-1.34",
181+
prepare: func(mock *checkerfakes.FakeChecker) {
182+
prowConfig := &ProwConfig{}
183+
prowConfig.Tide.Queries = []struct {
184+
Repos []string `yaml:"repos"`
185+
ExcludedBranches []string `yaml:"excludedBranches"`
186+
}{
187+
{
188+
Repos: []string{"kubernetes/kubernetes"},
189+
ExcludedBranches: []string{"release-1.34"},
190+
},
191+
}
192+
mock.HttpGetReturns(&http.Response{
193+
Body: io.NopCloser(strings.NewReader("")),
194+
}, nil)
195+
mock.ReadAllBodyReturns([]byte{}, nil)
196+
mock.UnmarshalProwConfigReturns(prowConfig, nil)
197+
},
198+
assert: func(inCodeFreeze bool, err error) {
199+
assert.True(t, inCodeFreeze)
200+
assert.Nil(t, err)
201+
},
202+
},
203+
{
204+
name: "success branch not excluded (code freeze not active)",
205+
branch: "release-1.34",
206+
prepare: func(mock *checkerfakes.FakeChecker) {
207+
prowConfig := &ProwConfig{}
208+
prowConfig.Tide.Queries = []struct {
209+
Repos []string `yaml:"repos"`
210+
ExcludedBranches []string `yaml:"excludedBranches"`
211+
}{
212+
{
213+
Repos: []string{"kubernetes/kubernetes"},
214+
ExcludedBranches: []string{"release-1.33"},
215+
},
216+
}
217+
mock.HttpGetReturns(&http.Response{
218+
Body: io.NopCloser(strings.NewReader("")),
219+
}, nil)
220+
mock.ReadAllBodyReturns([]byte{}, nil)
221+
mock.UnmarshalProwConfigReturns(prowConfig, nil)
222+
},
223+
assert: func(inCodeFreeze bool, err error) {
224+
assert.False(t, inCodeFreeze)
225+
assert.Nil(t, err)
226+
},
227+
},
228+
{
229+
name: "success multiple queries, branch excluded in one",
230+
branch: "release-1.34",
231+
prepare: func(mock *checkerfakes.FakeChecker) {
232+
prowConfig := &ProwConfig{}
233+
prowConfig.Tide.Queries = []struct {
234+
Repos []string `yaml:"repos"`
235+
ExcludedBranches []string `yaml:"excludedBranches"`
236+
}{
237+
{
238+
Repos: []string{"kubernetes/test-infra"},
239+
ExcludedBranches: []string{"release-1.34"},
240+
},
241+
{
242+
Repos: []string{"kubernetes/kubernetes"},
243+
ExcludedBranches: []string{"release-1.34"},
244+
},
245+
}
246+
mock.HttpGetReturns(&http.Response{
247+
Body: io.NopCloser(strings.NewReader("")),
248+
}, nil)
249+
mock.ReadAllBodyReturns([]byte{}, nil)
250+
mock.UnmarshalProwConfigReturns(prowConfig, nil)
251+
},
252+
assert: func(inCodeFreeze bool, err error) {
253+
assert.True(t, inCodeFreeze)
254+
assert.Nil(t, err)
255+
},
256+
},
257+
{
258+
name: "success branch excluded for different repo only",
259+
branch: "release-1.34",
260+
prepare: func(mock *checkerfakes.FakeChecker) {
261+
prowConfig := &ProwConfig{}
262+
prowConfig.Tide.Queries = []struct {
263+
Repos []string `yaml:"repos"`
264+
ExcludedBranches []string `yaml:"excludedBranches"`
265+
}{
266+
{
267+
Repos: []string{"kubernetes/test-infra"},
268+
ExcludedBranches: []string{"release-1.34"},
269+
},
270+
{
271+
Repos: []string{"kubernetes/kubernetes"},
272+
ExcludedBranches: []string{"release-1.33"},
273+
},
274+
}
275+
mock.HttpGetReturns(&http.Response{
276+
Body: io.NopCloser(strings.NewReader("")),
277+
}, nil)
278+
mock.ReadAllBodyReturns([]byte{}, nil)
279+
mock.UnmarshalProwConfigReturns(prowConfig, nil)
280+
},
281+
assert: func(inCodeFreeze bool, err error) {
282+
assert.False(t, inCodeFreeze)
283+
assert.Nil(t, err)
284+
},
285+
},
286+
{
287+
name: "success empty tide queries",
288+
branch: "release-1.34",
289+
prepare: func(mock *checkerfakes.FakeChecker) {
290+
prowConfig := &ProwConfig{}
291+
mock.HttpGetReturns(&http.Response{
292+
Body: io.NopCloser(strings.NewReader("")),
293+
}, nil)
294+
mock.ReadAllBodyReturns([]byte{}, nil)
295+
mock.UnmarshalProwConfigReturns(prowConfig, nil)
296+
},
297+
assert: func(inCodeFreeze bool, err error) {
298+
assert.False(t, inCodeFreeze)
299+
assert.Nil(t, err)
300+
},
301+
},
302+
{
303+
name: "error on HttpGet",
304+
branch: "release-1.34",
305+
prepare: func(mock *checkerfakes.FakeChecker) {
306+
mock.HttpGetReturns(nil, errTest)
307+
},
308+
assert: func(inCodeFreeze bool, err error) {
309+
assert.False(t, inCodeFreeze)
310+
assert.NotNil(t, err)
311+
assert.Contains(t, err.Error(), "get prow config")
312+
},
313+
},
314+
{
315+
name: "error on ReadAllBody",
316+
branch: "release-1.34",
317+
prepare: func(mock *checkerfakes.FakeChecker) {
318+
mock.HttpGetReturns(&http.Response{
319+
Body: io.NopCloser(strings.NewReader("")),
320+
}, nil)
321+
mock.ReadAllBodyReturns(nil, errTest)
322+
},
323+
assert: func(inCodeFreeze bool, err error) {
324+
assert.False(t, inCodeFreeze)
325+
assert.NotNil(t, err)
326+
assert.Contains(t, err.Error(), "read response body")
327+
},
328+
},
329+
{
330+
name: "error on UnmarshalProwConfig",
331+
branch: "release-1.34",
332+
prepare: func(mock *checkerfakes.FakeChecker) {
333+
mock.HttpGetReturns(&http.Response{
334+
Body: io.NopCloser(strings.NewReader("")),
335+
}, nil)
336+
mock.ReadAllBodyReturns([]byte{}, nil)
337+
mock.UnmarshalProwConfigReturns(nil, errTest)
338+
},
339+
assert: func(inCodeFreeze bool, err error) {
340+
assert.False(t, inCodeFreeze)
341+
assert.NotNil(t, err)
342+
assert.Contains(t, err.Error(), "unmarshal prow config")
343+
},
344+
},
345+
{
346+
name: "error on type assertion",
347+
branch: "release-1.34",
348+
prepare: func(mock *checkerfakes.FakeChecker) {
349+
mock.HttpGetReturns(&http.Response{
350+
Body: io.NopCloser(strings.NewReader("")),
351+
}, nil)
352+
mock.ReadAllBodyReturns([]byte{}, nil)
353+
// Return a string instead of *ProwConfig
354+
mock.UnmarshalProwConfigReturns("wrong type", nil)
355+
},
356+
assert: func(inCodeFreeze bool, err error) {
357+
assert.False(t, inCodeFreeze)
358+
assert.NotNil(t, err)
359+
assert.Contains(t, err.Error(), "failed to type assert prow config")
360+
},
361+
},
362+
} {
363+
t.Run(tc.name, func(t *testing.T) {
364+
mock := &checkerfakes.FakeChecker{}
365+
tc.prepare(mock)
366+
367+
sut := New(logrus.NewEntry(logrus.StandardLogger()))
368+
sut.checker = mock
369+
370+
inCodeFreeze, err := sut.inCodeFreeze(tc.branch)
371+
372+
tc.assert(inCodeFreeze, err)
373+
})
374+
}
375+
}

0 commit comments

Comments
 (0)