Skip to content

Commit 4f6979f

Browse files
committed
tests: ensure that pre-submits get additional reviews
Blocking pre-submit jobs must be for stable, important features. Non-blocking pre-submit jobs should only be run automatically if they meet the criteria outlined in kubernetes/community#8196. To ensure that this is considered when defining pre-submit jobs, they need to be listed in `config/tests/jobs/presubmit-jobs.yaml`. UPDATE_FIXTURE_DATA=true re-generates that file to the expected state for inclusion in a PR.
1 parent 229f05c commit 4f6979f

File tree

5 files changed

+336
-1
lines changed

5 files changed

+336
-1
lines changed

Diff for: config/tests/jobs/README.md

+3
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,7 @@ To run via bazel: `bazel test //config/tests/jobs/...`
88

99
To run via go: `go test .`
1010

11+
If tests fail, re-run with the `UPDATE_FIXTURE_DATA=true` env variable
12+
and include the modified files in the PR which updates the jobs.
13+
1114
[prow.k8s.io]: https://prow.k8s.io

Diff for: config/tests/jobs/jobs_test.go

+174
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,13 @@ import (
3535
"testing"
3636
"time"
3737

38+
"github.com/google/go-cmp/cmp"
39+
"github.com/google/go-cmp/cmp/cmpopts"
3840
coreapi "k8s.io/api/core/v1"
3941
"k8s.io/apimachinery/pkg/api/resource"
4042
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
4143
"k8s.io/apimachinery/pkg/util/sets"
44+
yaml "sigs.k8s.io/yaml/goyaml.v3"
4245

4346
prowapi "sigs.k8s.io/prow/pkg/apis/prowjobs/v1"
4447
cfg "sigs.k8s.io/prow/pkg/config"
@@ -1393,3 +1396,174 @@ func TestKubernetesProwJobsShouldNotUseDeprecatedScenarios(t *testing.T) {
13931396
t.Logf("summary: %v/%v jobs using deprecated scenarios", jobsToFix, len(jobs))
13941397
}
13951398
}
1399+
1400+
func TestKubernetesPresubmitJobs(t *testing.T) {
1401+
jobs := c.AllStaticPresubmits([]string{"kubernetes/kubernetes"})
1402+
var expected presubmitJobs
1403+
1404+
for _, job := range jobs {
1405+
if !job.AlwaysRun && job.RunIfChanged == "" {
1406+
// Manually triggered, no additional review needed.
1407+
continue
1408+
}
1409+
1410+
// Mirror those attributes of the job which must trigger additional reviews
1411+
// or are needed to identify the job.
1412+
j := presubmitJob{
1413+
Name: job.Name,
1414+
SkipBranches: job.SkipBranches,
1415+
Branches: job.Branches,
1416+
1417+
Optional: job.Optional,
1418+
RunIfChanged: job.RunIfChanged,
1419+
SkipIfOnlyChanged: job.SkipIfOnlyChanged,
1420+
}
1421+
1422+
// This uses separate top-level fields instead of job attributes to
1423+
// make it more obvious when run_if_changed is used.
1424+
if job.AlwaysRun {
1425+
expected.AlwaysRun = append(expected.AlwaysRun, j)
1426+
} else {
1427+
expected.RunIfChanged = append(expected.RunIfChanged, j)
1428+
}
1429+
1430+
}
1431+
expected.Normalize()
1432+
1433+
// Encode the expected content.
1434+
var expectedData bytes.Buffer
1435+
encoder := yaml.NewEncoder(&expectedData)
1436+
encoder.SetIndent(4)
1437+
if err := encoder.Encode(expected); err != nil {
1438+
t.Fatalf("unexpected error encoding %s: %v", presubmitsFile, err)
1439+
}
1440+
1441+
// Compare. This proceeds on read or decoding errors because
1442+
// the file might get re-generated below.
1443+
var actual presubmitJobs
1444+
actualData, err := os.ReadFile(presubmitsFile)
1445+
if err != nil && !os.IsNotExist(err) {
1446+
t.Errorf("unexpected error: %v", err)
1447+
}
1448+
if err := yaml.Unmarshal(actualData, &actual); err != nil {
1449+
t.Errorf("unexpected error decoding %s: %v", presubmitsFile, err)
1450+
}
1451+
1452+
// First check the in-memory structs. The diff is nicer for them (more context).
1453+
diff := cmp.Diff(actual, expected)
1454+
if diff == "" {
1455+
// Next check the encoded data. This should only be different on test updates.
1456+
diff = cmp.Diff(string(actualData), expectedData.String(), cmpopts.AcyclicTransformer("SplitLines", func(s string) []string {
1457+
return strings.Split(s, "\n")
1458+
}))
1459+
}
1460+
1461+
if diff != "" {
1462+
t.Errorf(`
1463+
%s is out-dated. Detected differences (- actual, + expected):
1464+
%s
1465+
1466+
Blocking pre-submit jobs must be for stable, important features.
1467+
Non-blocking pre-submit jobs should only be run automatically if they meet
1468+
the criteria outlined in https://github.com/kubernetes/community/pull/8196.
1469+
1470+
To ensure that this is considered when defining pre-submit jobs, they
1471+
need to be listed in %s. If the pre-submit job is really needed,
1472+
re-run the test with UPDATE_FIXTURE_DATA=true and include the modified
1473+
file.
1474+
1475+
1476+
`, presubmitsFile, diff, presubmitsFile)
1477+
if value, _ := os.LookupEnv("UPDATE_FIXTURE_DATA"); value == "true" {
1478+
if err := os.WriteFile(presubmitsFile, expectedData.Bytes(), 0644); err != nil {
1479+
t.Fatalf("unexpected error: %v", err)
1480+
}
1481+
}
1482+
}
1483+
}
1484+
1485+
// presubmitsFile contains the following struct.
1486+
const presubmitsFile = "presubmit-jobs.yaml"
1487+
1488+
type presubmitJobs struct {
1489+
AlwaysRun []presubmitJob `yaml:"always_run"`
1490+
RunIfChanged []presubmitJob `yaml:"run_if_changed"`
1491+
}
1492+
type presubmitJob struct {
1493+
Name string `yaml:"name"`
1494+
SkipBranches []string `yaml:"skip_branches,omitempty"`
1495+
Branches []string `yaml:"branches,omitempty"`
1496+
Optional bool `yaml:"optional,omitempty"`
1497+
RunIfChanged string `yaml:"run_if_changed,omitempty"`
1498+
SkipIfOnlyChanged string `yaml:"skip_if_only_changed,omitempty"`
1499+
}
1500+
1501+
func (p *presubmitJobs) Normalize() {
1502+
sortJobs(&p.AlwaysRun)
1503+
sortJobs(&p.RunIfChanged)
1504+
}
1505+
1506+
func sortJobs(jobs *[]presubmitJob) {
1507+
for _, job := range *jobs {
1508+
sort.Strings(job.SkipBranches)
1509+
sort.Strings(job.Branches)
1510+
}
1511+
sort.Slice(*jobs, func(i, j int) bool {
1512+
switch strings.Compare((*jobs)[i].Name, (*jobs)[j].Name) {
1513+
case -1:
1514+
return true
1515+
case 1:
1516+
return false
1517+
}
1518+
switch slices.Compare((*jobs)[i].SkipBranches, (*jobs)[j].SkipBranches) {
1519+
case -1:
1520+
return true
1521+
case 1:
1522+
return false
1523+
}
1524+
switch slices.Compare((*jobs)[i].Branches, (*jobs)[j].Branches) {
1525+
case -1:
1526+
return true
1527+
case 1:
1528+
return false
1529+
}
1530+
return false
1531+
})
1532+
1533+
// If a job has the same settings regardless of the branch, then
1534+
// we can reduce to a single entry without the branch info.
1535+
shorterJobs := make([]presubmitJob, 0, len(*jobs))
1536+
for i := 0; i < len(*jobs); {
1537+
job := (*jobs)[i]
1538+
job.Branches = nil
1539+
job.SkipBranches = nil
1540+
1541+
if sameSettings(*jobs, job) {
1542+
shorterJobs = append(shorterJobs, job)
1543+
// Fast-forward to next job.
1544+
for i < len(*jobs) && (*jobs)[i].Name == job.Name {
1545+
i++
1546+
}
1547+
} else {
1548+
// Keep all of the different entries.
1549+
for i < len(*jobs) && (*jobs)[i].Name == job.Name {
1550+
shorterJobs = append(shorterJobs, (*jobs)[i])
1551+
}
1552+
}
1553+
}
1554+
*jobs = shorterJobs
1555+
}
1556+
1557+
func sameSettings(jobs []presubmitJob, ref presubmitJob) bool {
1558+
for _, job := range jobs {
1559+
if job.Name != ref.Name {
1560+
continue
1561+
}
1562+
if job.Optional != ref.Optional ||
1563+
job.RunIfChanged != ref.RunIfChanged ||
1564+
job.SkipIfOnlyChanged != ref.SkipIfOnlyChanged {
1565+
return false
1566+
}
1567+
}
1568+
return true
1569+
}

Diff for: config/tests/jobs/presubmit-jobs.yaml

+156
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
always_run:
2+
- name: pull-kubernetes-conformance-kind-ga-only-parallel
3+
- name: pull-kubernetes-dependencies
4+
- name: pull-kubernetes-e2e-ec2
5+
optional: true
6+
- name: pull-kubernetes-e2e-ec2-conformance
7+
optional: true
8+
- name: pull-kubernetes-e2e-gce
9+
- name: pull-kubernetes-e2e-gce-canary
10+
optional: true
11+
- name: pull-kubernetes-e2e-kind
12+
- name: pull-kubernetes-e2e-kind-ipv6
13+
- name: pull-kubernetes-integration
14+
- name: pull-kubernetes-integration-go-compatibility
15+
- name: pull-kubernetes-linter-hints
16+
optional: true
17+
- name: pull-kubernetes-node-e2e-containerd
18+
- name: pull-kubernetes-typecheck
19+
- name: pull-kubernetes-unit
20+
- name: pull-kubernetes-unit-go-compatibility
21+
- name: pull-kubernetes-verify
22+
run_if_changed:
23+
- name: check-dependency-stats
24+
optional: true
25+
run_if_changed: ^(go.mod|go.sum|vendor)
26+
- name: pull-kubernetes-apidiff-client-go
27+
optional: true
28+
run_if_changed: (^staging\/src\/k8s.io\/client-go)|(^staging\/src\/k8s.io\/code-generator\/examples)
29+
- name: pull-kubernetes-conformance-image-test
30+
optional: true
31+
run_if_changed: conformance
32+
- name: pull-kubernetes-conformance-kind-ipv6-parallel
33+
optional: true
34+
run_if_changed: ^test/
35+
- name: pull-kubernetes-cross
36+
optional: true
37+
run_if_changed: (^.go-version)|(^build/build-image/)|(^hack/lib/golang.sh)|(^build/common.sh)
38+
- name: pull-kubernetes-e2e-autoscaling-hpa-cm
39+
optional: true
40+
run_if_changed: ^(pkg\/controller\/podautoscaler\/|test\/e2e\/autoscaling\/custom_metrics_stackdriver_autoscaling.go$)
41+
- name: pull-kubernetes-e2e-autoscaling-hpa-cpu
42+
optional: true
43+
run_if_changed: ^(pkg\/controller\/podautoscaler\/|test\/e2e\/autoscaling\/horizontal_pod_autoscaling|test\/e2e\/framework\/autoscaling\/)
44+
- name: pull-kubernetes-e2e-capz-azure-disk
45+
optional: true
46+
run_if_changed: azure.*\.go
47+
- name: pull-kubernetes-e2e-capz-azure-disk-vmss
48+
optional: true
49+
run_if_changed: azure.*\.go
50+
- name: pull-kubernetes-e2e-capz-azure-disk-windows
51+
optional: true
52+
run_if_changed: azure.*\.go
53+
- name: pull-kubernetes-e2e-capz-azure-file
54+
optional: true
55+
run_if_changed: azure.*\.go
56+
- name: pull-kubernetes-e2e-capz-azure-file-vmss
57+
optional: true
58+
run_if_changed: azure.*\.go
59+
- name: pull-kubernetes-e2e-capz-azure-file-windows
60+
optional: true
61+
run_if_changed: azure.*\.go
62+
- name: pull-kubernetes-e2e-capz-conformance
63+
optional: true
64+
run_if_changed: azure.*\.go
65+
- name: pull-kubernetes-e2e-capz-windows-1-29
66+
optional: true
67+
run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.*
68+
- name: pull-kubernetes-e2e-capz-windows-1-30
69+
optional: true
70+
run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.*
71+
- name: pull-kubernetes-e2e-capz-windows-1-31
72+
optional: true
73+
run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.*
74+
- name: pull-kubernetes-e2e-capz-windows-1-32
75+
optional: true
76+
run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.*
77+
- name: pull-kubernetes-e2e-capz-windows-master
78+
optional: true
79+
run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.*
80+
- name: pull-kubernetes-e2e-gce-cos-alpha-features
81+
optional: true
82+
run_if_changed: ^.*feature.*\.go
83+
- name: pull-kubernetes-e2e-gce-csi-serial
84+
optional: true
85+
run_if_changed: ^(pkg\/controller\/volume|pkg\/kubelet\/volumemanager|pkg\/volume|pkg\/util\/mount|test\/e2e\/storage|test\/e2e\/testing-manifests\/storage-csi)
86+
- name: pull-kubernetes-e2e-gce-kubelet-credential-provider
87+
optional: true
88+
run_if_changed: ^(pkg\/credentialprovider\/|test\/e2e_node\/plugins\/gcp-credential-provider)
89+
- name: pull-kubernetes-e2e-gce-network-policies
90+
optional: true
91+
run_if_changed: ^(test/e2e/network/|pkg/apis/networking)
92+
- name: pull-kubernetes-e2e-gce-network-proxy-grpc
93+
optional: true
94+
run_if_changed: ^(cluster/gce/manifests/konnectivity-server.yaml$|cluster/gce/addons/konnectivity-agent)
95+
- name: pull-kubernetes-e2e-gce-network-proxy-http-connect
96+
run_if_changed: ^(cluster/gce/manifests/konnectivity-server.yaml$|cluster/gce/addons/konnectivity-agent)
97+
- name: pull-kubernetes-e2e-gce-providerless-1-30
98+
optional: true
99+
run_if_changed: (provider|cloud-controller-manager|cloud|ipam|azure|legacy-cloud-providers|test\/e2e\/cloud\/gcp|test\/e2e\/framework\/provider|nvidia|accelerator|test\/e2e\/network|test\/e2e\/storage)
100+
- name: pull-kubernetes-e2e-gce-storage-slow
101+
optional: true
102+
run_if_changed: ^(pkg\/controller\/volume|pkg\/kubelet\/volumemanager|pkg\/volume|pkg\/util\/mount|test\/e2e\/storage|test\/e2e\/testing-manifests\/storage-csi)
103+
- name: pull-kubernetes-e2e-gce-storage-snapshot
104+
optional: true
105+
run_if_changed: ^(pkg\/controller\/volume|pkg\/kubelet\/volumemanager|pkg\/volume|pkg\/util\/mount|test\/e2e\/storage|test\/e2e\/testing-manifests\/storage-csi)
106+
- name: pull-kubernetes-e2e-gci-gce-autoscaling
107+
optional: true
108+
run_if_changed: ^(cluster/gce/manifests/cluster-autoscaler.manifest$|cluster/addons/rbac/cluster-autoscaler)
109+
- name: pull-kubernetes-e2e-gci-gce-ingress
110+
optional: true
111+
run_if_changed: ^(test/e2e/network/|pkg/apis/networking)
112+
- name: pull-kubernetes-e2e-gci-gce-ipvs
113+
optional: true
114+
run_if_changed: ^(test/e2e/network/|pkg/apis/networking|pkg/.*/ipvs/)
115+
- name: pull-kubernetes-e2e-inplace-pod-resize-containerd-main-v2
116+
optional: true
117+
run_if_changed: test/e2e/node/pod_resize.go|pkg/kubelet/kubelet.go|pkg/kubelet/kubelet_pods.go|pkg/kubelet/kuberuntime/kuberuntime_manager.go
118+
- name: pull-kubernetes-e2e-kind-alpha-beta-features
119+
optional: true
120+
run_if_changed: ^pkg/features/
121+
- name: pull-kubernetes-e2e-kind-ipvs
122+
optional: true
123+
run_if_changed: ^(test/e2e/network/|pkg/apis/networking|pkg/proxy/ipvs/)
124+
- name: pull-kubernetes-e2e-kind-kms
125+
optional: true
126+
run_if_changed: staging/src/k8s.io/apiserver/pkg/storage/value/|staging/src/k8s.io/kms/|staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/|test/e2e/testing-manifests/auth/encrypt/
127+
- name: pull-kubernetes-e2e-kind-nftables
128+
optional: true
129+
run_if_changed: ^(test/e2e/network/|pkg/apis/networking|pkg/proxy/nftables/)
130+
- name: pull-kubernetes-e2e-storage-kind-disruptive
131+
optional: true
132+
run_if_changed: ^(pkg\/controller\/volume|pkg\/kubelet\/volumemanager|pkg\/volume|pkg\/util\/mount|test\/e2e\/storage|test\/e2e\/testing-manifests\/storage-csi)
133+
- name: pull-kubernetes-kind-dra
134+
optional: true
135+
run_if_changed: /(dra|dynamicresources|resourceclaim|deviceclass|resourceslice|resourceclaimtemplate|dynamic-resource-allocation|pkg/apis/resource|api/resource)/.*.go
136+
- name: pull-kubernetes-kind-dra-all
137+
optional: true
138+
run_if_changed: /(dra|dynamicresources|resourceclaim|deviceclass|resourceslice|resourceclaimtemplate|dynamic-resource-allocation|pkg/apis/resource|api/resource)/.*.go
139+
- name: pull-kubernetes-kind-json-logging
140+
optional: true
141+
run_if_changed: /staging/src/k8s.io/component-base/logs/
142+
- name: pull-kubernetes-kind-text-logging
143+
optional: true
144+
run_if_changed: /staging/src/k8s.io/component-base/logs/
145+
- name: pull-kubernetes-local-e2e
146+
optional: true
147+
run_if_changed: local-up-cluster
148+
- name: pull-kubernetes-node-e2e-crio-cgrpv1-dra
149+
optional: true
150+
run_if_changed: (/dra/|/dynamicresources/|/resourceclaim/|/deviceclass/|/resourceslice/|/resourceclaimtemplate/|/dynamic-resource-allocation/|/pkg/apis/resource/|/api/resource/|/test/e2e_node/dra_).*\.(go|yaml)
151+
- name: pull-kubernetes-node-kubelet-credential-provider
152+
optional: true
153+
run_if_changed: ^(pkg\/credentialprovider\/|test\/e2e_node\/plugins\/gcp-credential-provider)
154+
- name: pull-publishing-bot-validate
155+
optional: true
156+
run_if_changed: ^staging/publishing.*$

Diff for: go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ require (
8787
sigs.k8s.io/controller-runtime v0.12.3
8888
sigs.k8s.io/controller-tools v0.9.2
8989
sigs.k8s.io/prow v0.0.0-20240419142743-3cb2506c2ff3
90-
sigs.k8s.io/yaml v1.3.0
90+
sigs.k8s.io/yaml v1.4.0
9191
)
9292

9393
require (

Diff for: go.sum

+2
Original file line numberDiff line numberDiff line change
@@ -1160,3 +1160,5 @@ sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
11601160
sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
11611161
sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo=
11621162
sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8=
1163+
sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
1164+
sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY=

0 commit comments

Comments
 (0)