Skip to content

Commit be0f38b

Browse files
committed
Enhance testfreeze plugin to distinguish Code Freeze and Test Freeze
This change improves the testfreeze plugin to provide distinct messaging for Code Freeze and Test Freeze periods, making it clearer what actions are required during each phase. The plugin now: - Detects whether we are in Code Freeze, Test Freeze, or both - Shows appropriate instructions for each phase - For Code Freeze: explains milestone restrictions and approval process - For Test Freeze: provides information about fast-forward merges Signed-off-by: Sascha Grunert <[email protected]>
1 parent e3ae8cf commit be0f38b

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)