diff --git a/config/tests/jobs/README.md b/config/tests/jobs/README.md index 3cd192905cde2..f9e36fd783052 100644 --- a/config/tests/jobs/README.md +++ b/config/tests/jobs/README.md @@ -8,4 +8,7 @@ To run via bazel: `bazel test //config/tests/jobs/...` To run via go: `go test .` +If tests fail, re-run with the `UPDATE_FIXTURE_DATA=true` env variable +and include the modified files in the PR which updates the jobs. + [prow.k8s.io]: https://prow.k8s.io diff --git a/config/tests/jobs/jobs_test.go b/config/tests/jobs/jobs_test.go index 21bc60b2ee724..b4ff0a971ff1e 100644 --- a/config/tests/jobs/jobs_test.go +++ b/config/tests/jobs/jobs_test.go @@ -35,10 +35,13 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" coreapi "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + yaml "sigs.k8s.io/yaml/goyaml.v3" prowapi "sigs.k8s.io/prow/pkg/apis/prowjobs/v1" cfg "sigs.k8s.io/prow/pkg/config" @@ -1393,3 +1396,180 @@ func TestKubernetesProwJobsShouldNotUseDeprecatedScenarios(t *testing.T) { t.Logf("summary: %v/%v jobs using deprecated scenarios", jobsToFix, len(jobs)) } } + +func TestKubernetesPresubmitJobs(t *testing.T) { + jobs := c.AllStaticPresubmits([]string{"kubernetes/kubernetes"}) + var expected presubmitJobs + + for _, job := range jobs { + if !job.AlwaysRun && job.RunIfChanged == "" { + // Manually triggered, no additional review needed. + continue + } + + // Mirror those attributes of the job which must trigger additional reviews + // or are needed to identify the job. + j := presubmitJob{ + Name: job.Name, + SkipBranches: job.SkipBranches, + Branches: job.Branches, + + Optional: job.Optional, + RunIfChanged: job.RunIfChanged, + SkipIfOnlyChanged: job.SkipIfOnlyChanged, + } + + // This uses separate top-level fields instead of job attributes to + // make it more obvious when run_if_changed is used. + if job.AlwaysRun { + expected.AlwaysRun = append(expected.AlwaysRun, j) + } else { + expected.RunIfChanged = append(expected.RunIfChanged, j) + } + + } + expected.Normalize() + + // Encode the expected content. + var expectedData bytes.Buffer + if _, err := expectedData.Write([]byte(`# AUTOGENERATED by "UPDATE_FIXTURE_DATA=true go test ./config/tests/jobs". DO NOT EDIT! + +`)); err != nil { + t.Fatalf("unexpected error writing into buffer: %v", err) + } + + encoder := yaml.NewEncoder(&expectedData) + encoder.SetIndent(4) + if err := encoder.Encode(expected); err != nil { + t.Fatalf("unexpected error encoding %s: %v", presubmitsFile, err) + } + + // Compare. This proceeds on read or decoding errors because + // the file might get re-generated below. + var actual presubmitJobs + actualData, err := os.ReadFile(presubmitsFile) + if err != nil && !os.IsNotExist(err) { + t.Errorf("unexpected error: %v", err) + } + if err := yaml.Unmarshal(actualData, &actual); err != nil { + t.Errorf("unexpected error decoding %s: %v", presubmitsFile, err) + } + + // First check the in-memory structs. The diff is nicer for them (more context). + diff := cmp.Diff(actual, expected) + if diff == "" { + // Next check the encoded data. This should only be different on test updates. + diff = cmp.Diff(string(actualData), expectedData.String(), cmpopts.AcyclicTransformer("SplitLines", func(s string) []string { + return strings.Split(s, "\n") + })) + } + + if diff != "" { + t.Errorf(` +%s is out-dated. Detected differences (- actual, + expected): +%s + +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 https://github.com/kubernetes/community/pull/8196. + +To ensure that this is considered when defining pre-submit jobs, they +need to be listed in %s. If the pre-submit job is really needed, +re-run the test with UPDATE_FIXTURE_DATA=true and include the modified +file. + + +`, presubmitsFile, diff, presubmitsFile) + if value, _ := os.LookupEnv("UPDATE_FIXTURE_DATA"); value == "true" { + if err := os.WriteFile(presubmitsFile, expectedData.Bytes(), 0644); err != nil { + t.Fatalf("unexpected error: %v", err) + } + } + } +} + +// presubmitsFile contains the following struct. +const presubmitsFile = "presubmit-jobs.yaml" + +type presubmitJobs struct { + AlwaysRun []presubmitJob `yaml:"always_run"` + RunIfChanged []presubmitJob `yaml:"run_if_changed"` +} +type presubmitJob struct { + Name string `yaml:"name"` + SkipBranches []string `yaml:"skip_branches,omitempty"` + Branches []string `yaml:"branches,omitempty"` + Optional bool `yaml:"optional,omitempty"` + RunIfChanged string `yaml:"run_if_changed,omitempty"` + SkipIfOnlyChanged string `yaml:"skip_if_only_changed,omitempty"` +} + +func (p *presubmitJobs) Normalize() { + sortJobs(&p.AlwaysRun) + sortJobs(&p.RunIfChanged) +} + +func sortJobs(jobs *[]presubmitJob) { + for _, job := range *jobs { + sort.Strings(job.SkipBranches) + sort.Strings(job.Branches) + } + sort.Slice(*jobs, func(i, j int) bool { + switch strings.Compare((*jobs)[i].Name, (*jobs)[j].Name) { + case -1: + return true + case 1: + return false + } + switch slices.Compare((*jobs)[i].SkipBranches, (*jobs)[j].SkipBranches) { + case -1: + return true + case 1: + return false + } + switch slices.Compare((*jobs)[i].Branches, (*jobs)[j].Branches) { + case -1: + return true + case 1: + return false + } + return false + }) + + // If a job has the same settings regardless of the branch, then + // we can reduce to a single entry without the branch info. + shorterJobs := make([]presubmitJob, 0, len(*jobs)) + for i := 0; i < len(*jobs); { + job := (*jobs)[i] + job.Branches = nil + job.SkipBranches = nil + + if sameSettings(*jobs, job) { + shorterJobs = append(shorterJobs, job) + // Fast-forward to next job. + for i < len(*jobs) && (*jobs)[i].Name == job.Name { + i++ + } + } else { + // Keep all of the different entries. + for i < len(*jobs) && (*jobs)[i].Name == job.Name { + shorterJobs = append(shorterJobs, (*jobs)[i]) + } + } + } + *jobs = shorterJobs +} + +func sameSettings(jobs []presubmitJob, ref presubmitJob) bool { + for _, job := range jobs { + if job.Name != ref.Name { + continue + } + if job.Optional != ref.Optional || + job.RunIfChanged != ref.RunIfChanged || + job.SkipIfOnlyChanged != ref.SkipIfOnlyChanged { + return false + } + } + return true +} diff --git a/config/tests/jobs/presubmit-jobs.yaml b/config/tests/jobs/presubmit-jobs.yaml new file mode 100644 index 0000000000000..9900e4ffa2c16 --- /dev/null +++ b/config/tests/jobs/presubmit-jobs.yaml @@ -0,0 +1,158 @@ +# AUTOGENERATED by "UPDATE_FIXTURE_DATA=true go test ./config/tests/jobs". DO NOT EDIT! + +always_run: + - name: pull-kubernetes-conformance-kind-ga-only-parallel + - name: pull-kubernetes-dependencies + - name: pull-kubernetes-e2e-ec2 + optional: true + - name: pull-kubernetes-e2e-ec2-conformance + optional: true + - name: pull-kubernetes-e2e-gce + - name: pull-kubernetes-e2e-gce-canary + optional: true + - name: pull-kubernetes-e2e-kind + - name: pull-kubernetes-e2e-kind-ipv6 + - name: pull-kubernetes-integration + - name: pull-kubernetes-integration-go-compatibility + - name: pull-kubernetes-linter-hints + optional: true + - name: pull-kubernetes-node-e2e-containerd + - name: pull-kubernetes-typecheck + - name: pull-kubernetes-unit + - name: pull-kubernetes-unit-go-compatibility + - name: pull-kubernetes-verify +run_if_changed: + - name: check-dependency-stats + optional: true + run_if_changed: ^(go.mod|go.sum|vendor) + - name: pull-kubernetes-apidiff-client-go + optional: true + run_if_changed: ((^staging\/src\/k8s.io\/client-go)|(^staging\/src\/k8s.io\/code-generator\/examples))/.*\.go + - name: pull-kubernetes-conformance-image-test + optional: true + run_if_changed: conformance + - name: pull-kubernetes-conformance-kind-ipv6-parallel + optional: true + run_if_changed: ^test/ + - name: pull-kubernetes-cross + optional: true + run_if_changed: (^.go-version)|(^build/build-image/)|(^hack/lib/golang.sh)|(^build/common.sh) + - name: pull-kubernetes-e2e-autoscaling-hpa-cm + optional: true + run_if_changed: ^(pkg\/controller\/podautoscaler\/|test\/e2e\/autoscaling\/custom_metrics_stackdriver_autoscaling.go$) + - name: pull-kubernetes-e2e-autoscaling-hpa-cpu + optional: true + run_if_changed: ^(pkg\/controller\/podautoscaler\/|test\/e2e\/autoscaling\/horizontal_pod_autoscaling|test\/e2e\/framework\/autoscaling\/) + - name: pull-kubernetes-e2e-capz-azure-disk + optional: true + run_if_changed: azure.*\.go + - name: pull-kubernetes-e2e-capz-azure-disk-vmss + optional: true + run_if_changed: azure.*\.go + - name: pull-kubernetes-e2e-capz-azure-disk-windows + optional: true + run_if_changed: azure.*\.go + - name: pull-kubernetes-e2e-capz-azure-file + optional: true + run_if_changed: azure.*\.go + - name: pull-kubernetes-e2e-capz-azure-file-vmss + optional: true + run_if_changed: azure.*\.go + - name: pull-kubernetes-e2e-capz-azure-file-windows + optional: true + run_if_changed: azure.*\.go + - name: pull-kubernetes-e2e-capz-conformance + optional: true + run_if_changed: azure.*\.go + - name: pull-kubernetes-e2e-capz-windows-1-29 + optional: true + run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.* + - name: pull-kubernetes-e2e-capz-windows-1-30 + optional: true + run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.* + - name: pull-kubernetes-e2e-capz-windows-1-31 + optional: true + run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.* + - name: pull-kubernetes-e2e-capz-windows-1-32 + optional: true + run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.* + - name: pull-kubernetes-e2e-capz-windows-master + optional: true + run_if_changed: azure.*\.go$|.*windows\.go$|test/e2e/windows/.* + - name: pull-kubernetes-e2e-gce-cos-alpha-features + optional: true + run_if_changed: ^.*feature.*\.go + - name: pull-kubernetes-e2e-gce-csi-serial + optional: true + run_if_changed: ^(pkg\/controller\/volume|pkg\/kubelet\/volumemanager|pkg\/volume|pkg\/util\/mount|test\/e2e\/storage|test\/e2e\/testing-manifests\/storage-csi) + - name: pull-kubernetes-e2e-gce-kubelet-credential-provider + optional: true + run_if_changed: ^(pkg\/credentialprovider\/|test\/e2e_node\/plugins\/gcp-credential-provider) + - name: pull-kubernetes-e2e-gce-network-policies + optional: true + run_if_changed: ^(test/e2e/network/|pkg/apis/networking) + - name: pull-kubernetes-e2e-gce-network-proxy-grpc + optional: true + run_if_changed: ^(cluster/gce/manifests/konnectivity-server.yaml$|cluster/gce/addons/konnectivity-agent) + - name: pull-kubernetes-e2e-gce-network-proxy-http-connect + run_if_changed: ^(cluster/gce/manifests/konnectivity-server.yaml$|cluster/gce/addons/konnectivity-agent) + - name: pull-kubernetes-e2e-gce-providerless-1-30 + optional: true + 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) + - name: pull-kubernetes-e2e-gce-storage-slow + optional: true + run_if_changed: ^(pkg\/controller\/volume|pkg\/kubelet\/volumemanager|pkg\/volume|pkg\/util\/mount|test\/e2e\/storage|test\/e2e\/testing-manifests\/storage-csi) + - name: pull-kubernetes-e2e-gce-storage-snapshot + optional: true + run_if_changed: ^(pkg\/controller\/volume|pkg\/kubelet\/volumemanager|pkg\/volume|pkg\/util\/mount|test\/e2e\/storage|test\/e2e\/testing-manifests\/storage-csi) + - name: pull-kubernetes-e2e-gci-gce-autoscaling + optional: true + run_if_changed: ^(cluster/gce/manifests/cluster-autoscaler.manifest$|cluster/addons/rbac/cluster-autoscaler) + - name: pull-kubernetes-e2e-gci-gce-ingress + optional: true + run_if_changed: ^(test/e2e/network/|pkg/apis/networking) + - name: pull-kubernetes-e2e-gci-gce-ipvs + optional: true + run_if_changed: ^(test/e2e/network/|pkg/apis/networking|pkg/.*/ipvs/) + - name: pull-kubernetes-e2e-inplace-pod-resize-containerd-main-v2 + optional: true + run_if_changed: test/e2e/node/pod_resize.go|pkg/kubelet/kubelet.go|pkg/kubelet/kubelet_pods.go|pkg/kubelet/kuberuntime/kuberuntime_manager.go + - name: pull-kubernetes-e2e-kind-alpha-beta-features + optional: true + run_if_changed: ^pkg/features/ + - name: pull-kubernetes-e2e-kind-ipvs + optional: true + run_if_changed: ^(test/e2e/network/|pkg/apis/networking|pkg/proxy/ipvs/) + - name: pull-kubernetes-e2e-kind-kms + optional: true + 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/ + - name: pull-kubernetes-e2e-kind-nftables + optional: true + run_if_changed: ^(test/e2e/network/|pkg/apis/networking|pkg/proxy/nftables/) + - name: pull-kubernetes-e2e-storage-kind-disruptive + optional: true + run_if_changed: ^(pkg\/controller\/volume|pkg\/kubelet\/volumemanager|pkg\/volume|pkg\/util\/mount|test\/e2e\/storage|test\/e2e\/testing-manifests\/storage-csi) + - name: pull-kubernetes-kind-dra + optional: true + run_if_changed: /(dra|dynamicresources|resourceclaim|deviceclass|resourceslice|resourceclaimtemplate|dynamic-resource-allocation|pkg/apis/resource|api/resource)/.*.go + - name: pull-kubernetes-kind-dra-all + optional: true + run_if_changed: /(dra|dynamicresources|resourceclaim|deviceclass|resourceslice|resourceclaimtemplate|dynamic-resource-allocation|pkg/apis/resource|api/resource)/.*.go + - name: pull-kubernetes-kind-json-logging + optional: true + run_if_changed: /staging/src/k8s.io/component-base/logs/ + - name: pull-kubernetes-kind-text-logging + optional: true + run_if_changed: /staging/src/k8s.io/component-base/logs/ + - name: pull-kubernetes-local-e2e + optional: true + run_if_changed: local-up-cluster + - name: pull-kubernetes-node-e2e-crio-cgrpv1-dra + optional: true + run_if_changed: (/dra/|/dynamicresources/|/resourceclaim/|/deviceclass/|/resourceslice/|/resourceclaimtemplate/|/dynamic-resource-allocation/|/pkg/apis/resource/|/api/resource/|/test/e2e_node/dra_).*\.(go|yaml) + - name: pull-kubernetes-node-kubelet-credential-provider + optional: true + run_if_changed: ^(pkg\/credentialprovider\/|test\/e2e_node\/plugins\/gcp-credential-provider) + - name: pull-publishing-bot-validate + optional: true + run_if_changed: ^staging/publishing.*$ diff --git a/go.mod b/go.mod index b1fc721b5b882..6e4f5b839b718 100644 --- a/go.mod +++ b/go.mod @@ -87,7 +87,7 @@ require ( sigs.k8s.io/controller-runtime v0.12.3 sigs.k8s.io/controller-tools v0.9.2 sigs.k8s.io/prow v0.0.0-20240419142743-3cb2506c2ff3 - sigs.k8s.io/yaml v1.3.0 + sigs.k8s.io/yaml v1.4.0 ) require ( diff --git a/go.sum b/go.sum index 12931547e24ea..2baf43fbb04a8 100644 --- a/go.sum +++ b/go.sum @@ -1158,5 +1158,5 @@ sigs.k8s.io/structured-merge-diff/v4 v4.2.3 h1:PRbqxJClWWYMNV1dhaG4NsibJbArud9kF sigs.k8s.io/structured-merge-diff/v4 v4.2.3/go.mod h1:qjx8mGObPmV2aSZepjQjbmb2ihdVs8cGKBraizNC69E= sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o= sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc= -sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo= -sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= +sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E= +sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY=