Skip to content

Commit

Permalink
Add support for alternate default_decoration_configs format that allo…
Browse files Browse the repository at this point in the history
…ws defaulting by cluster. (#20656)

* Add support for alternate default_decoration_configs format that allows defaulting by cluster.

* Add unit tests for new DefaultDecorationConfigs format.

* Update documentation about default_decoration_configs.

* Expose fields for both default_decoration formats rather than using json.RawMessage.

* Respond to review. Namely replace regexp filters.
  • Loading branch information
cjwagner authored Mar 17, 2021
1 parent 015e36c commit 1acac76
Show file tree
Hide file tree
Showing 22 changed files with 1,601 additions and 603 deletions.
15 changes: 7 additions & 8 deletions config/tests/jobs/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,23 +741,23 @@ func TestValidPresets(t *testing.T) {
}

for _, presubmit := range c.AllStaticPresubmits(nil) {
if presubmit.Spec != nil && !cfg.ShouldDecorate(&c.JobConfig, presubmit.JobBase.UtilityConfig) {
if presubmit.Spec != nil && !*presubmit.Decorate {
if err := checkKubekinsPresets(presubmit.Name, presubmit.Spec, presubmit.Labels, validLabels); err != nil {
t.Errorf("Error in presubmit %q: %v", presubmit.Name, err)
}
}
}

for _, postsubmit := range c.AllStaticPostsubmits(nil) {
if postsubmit.Spec != nil && !cfg.ShouldDecorate(&c.JobConfig, postsubmit.JobBase.UtilityConfig) {
if postsubmit.Spec != nil && !*postsubmit.Decorate {
if err := checkKubekinsPresets(postsubmit.Name, postsubmit.Spec, postsubmit.Labels, validLabels); err != nil {
t.Errorf("Error in postsubmit %q: %v", postsubmit.Name, err)
}
}
}

for _, periodic := range c.AllPeriodics() {
if periodic.Spec != nil && !cfg.ShouldDecorate(&c.JobConfig, periodic.JobBase.UtilityConfig) {
if periodic.Spec != nil && !*periodic.Decorate {
if err := checkKubekinsPresets(periodic.Name, periodic.Spec, periodic.Labels, validLabels); err != nil {
t.Errorf("Error in periodic %q: %v", periodic.Name, err)
}
Expand Down Expand Up @@ -943,23 +943,23 @@ func checkScenarioArgs(jobName, imageName string, args []string) error {
// TestValidScenarioArgs makes sure all scenario args in job configs are valid
func TestValidScenarioArgs(t *testing.T) {
for _, job := range c.AllStaticPresubmits(nil) {
if job.Spec != nil && !cfg.ShouldDecorate(&c.JobConfig, job.JobBase.UtilityConfig) {
if job.Spec != nil && !*job.Decorate {
if err := checkScenarioArgs(job.Name, job.Spec.Containers[0].Image, job.Spec.Containers[0].Args); err != nil {
t.Errorf("Invalid Scenario Args : %s", err)
}
}
}

for _, job := range c.AllStaticPostsubmits(nil) {
if job.Spec != nil && !cfg.ShouldDecorate(&c.JobConfig, job.JobBase.UtilityConfig) {
if job.Spec != nil && !*job.Decorate {
if err := checkScenarioArgs(job.Name, job.Spec.Containers[0].Image, job.Spec.Containers[0].Args); err != nil {
t.Errorf("Invalid Scenario Args : %s", err)
}
}
}

for _, job := range c.AllPeriodics() {
if job.Spec != nil && !cfg.ShouldDecorate(&c.JobConfig, job.JobBase.UtilityConfig) {
if job.Spec != nil && !*job.Decorate {
if err := checkScenarioArgs(job.Name, job.Spec.Containers[0].Image, job.Spec.Containers[0].Args); err != nil {
t.Errorf("Invalid Scenario Args : %s", err)
}
Expand Down Expand Up @@ -1232,8 +1232,7 @@ func TestKubernetesProwJobsShouldUsePodUtils(t *testing.T) {
if job.Spec == nil || strings.HasPrefix("kubeflow", job.Name) {
continue
}
usesPodUtils := cfg.ShouldDecorate(&c.JobConfig, job.UtilityConfig)
if !usesPodUtils {
if !*job.Decorate {
// bootstrap jobs don't use multiple containers
container := job.Spec.Containers[0]
repos := []string{}
Expand Down
5 changes: 5 additions & 0 deletions prow/ANNOUNCEMENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
## New features

New features added to each component:
- *March 3, 2021* `plank.default_decoration_configs` can optionally be replaced with
`plank.default_decoration_config_entries` which supports a new format
that is a slice of filters with associated decoration configs rather than a
map. Currently entries can filter by repo and/or cluster. The old field is still
supported and will not be deprecated.
- *February 23, 2021* New format introduced in `plugins.yaml`. Repos can be excluded from plugin definition
at org level using `excluded_repos` notation. The previous format will be deprecated in *July 2021*, see
https://github.com/kubernetes/test-infra/issues/20631.
Expand Down
6 changes: 3 additions & 3 deletions prow/cmd/checkconfig/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -806,19 +806,19 @@ func ensureValidConfiguration(plugin, label, verb string, tideSubSet, tideSuperS
func validateDecoratedJobs(cfg *config.Config) error {
var nonDecoratedJobs []string
for _, presubmit := range cfg.AllStaticPresubmits([]string{}) {
if presubmit.Agent == string(v1.KubernetesAgent) && !config.ShouldDecorate(&cfg.JobConfig, presubmit.JobBase.UtilityConfig) {
if presubmit.Agent == string(v1.KubernetesAgent) && !*presubmit.JobBase.UtilityConfig.Decorate {
nonDecoratedJobs = append(nonDecoratedJobs, presubmit.Name)
}
}

for _, postsubmit := range cfg.AllStaticPostsubmits([]string{}) {
if postsubmit.Agent == string(v1.KubernetesAgent) && !config.ShouldDecorate(&cfg.JobConfig, postsubmit.JobBase.UtilityConfig) {
if postsubmit.Agent == string(v1.KubernetesAgent) && !*postsubmit.JobBase.UtilityConfig.Decorate {
nonDecoratedJobs = append(nonDecoratedJobs, postsubmit.Name)
}
}

for _, periodic := range cfg.AllPeriodics() {
if periodic.Agent == string(v1.KubernetesAgent) && !config.ShouldDecorate(&cfg.JobConfig, periodic.JobBase.UtilityConfig) {
if periodic.Agent == string(v1.KubernetesAgent) && !*periodic.JobBase.UtilityConfig.Decorate {
nonDecoratedJobs = append(nonDecoratedJobs, periodic.Name)
}
}
Expand Down
6 changes: 5 additions & 1 deletion prow/cmd/deck/pr_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,11 @@ func getStorageDirsForPR(c *config.Config, gitHubClient deckGitHubClient, gitCli
gcsConfig = presubmit.DecorationConfig.GCSConfiguration
} else {
// for undecorated jobs assume the default
gcsConfig = c.Plank.GetDefaultDecorationConfigs(fullRepo).GCSConfiguration
def := c.Plank.GuessDefaultDecorationConfig(fullRepo, presubmit.Cluster)
if def == nil || def.GCSConfiguration == nil {
return toSearch, fmt.Errorf("failed to guess gcs config based on default decoration config: %w", err)
}
gcsConfig = def.GCSConfiguration
}

gcsPath, _, _ := gcsupload.PathsForJob(gcsConfig, &downwardapi.JobSpec{
Expand Down
36 changes: 19 additions & 17 deletions prow/cmd/deck/pr_history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,16 +435,17 @@ func TestGetGCSDirsForPR(t *testing.T) {
config: &config.Config{
ProwConfig: config.ProwConfig{
Plank: config.Plank{
DefaultDecorationConfigs: map[string]*prowapi.DecorationConfig{
"*": {
GCSConfiguration: &prowapi.GCSConfiguration{
Bucket: "krusty-krab",
PathStrategy: "legacy",
DefaultOrg: "kubernetes",
DefaultRepo: "kubernetes",
DefaultDecorationConfigs: config.DefaultDecorationMapToSliceTesting(
map[string]*prowapi.DecorationConfig{
"*": {
GCSConfiguration: &prowapi.GCSConfiguration{
Bucket: "krusty-krab",
PathStrategy: "legacy",
DefaultOrg: "kubernetes",
DefaultRepo: "kubernetes",
},
},
},
},
}),
},
},
JobConfig: config.JobConfig{
Expand Down Expand Up @@ -532,15 +533,16 @@ func Test_getPRHistory(t *testing.T) {
},
ProwConfig: config.ProwConfig{
Plank: config.Plank{
DefaultDecorationConfigs: map[string]*prowapi.DecorationConfig{
"*": {
GCSConfiguration: &prowapi.GCSConfiguration{
Bucket: "gs://kubernetes-jenkins",
PathStrategy: prowapi.PathStrategyLegacy,
DefaultOrg: "kubernetes",
DefaultDecorationConfigs: config.DefaultDecorationMapToSliceTesting(
map[string]*prowapi.DecorationConfig{
"*": {
GCSConfiguration: &prowapi.GCSConfiguration{
Bucket: "gs://kubernetes-jenkins",
PathStrategy: prowapi.PathStrategyLegacy,
DefaultOrg: "kubernetes",
},
},
},
},
}),
},
Deck: config.Deck{
AllKnownStorageBuckets: sets.NewString("kubernetes-jenkins"),
Expand Down
13 changes: 12 additions & 1 deletion prow/cmd/plank/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ plank:
job_url_template: 'https://<domain>/view/<bucket-name>/pr-logs/pull/{{.Spec.Refs.Repo}}/{{with index .Spec.Refs.Pulls 0}}{{.Number}}{{end}}/{{.Spec.Job}}/{{.Status.BuildID}}'
report_template: '[Full PR test history](https://<domain>/pr-history?org={{.Spec.Refs.Org}}&repo={{.Spec.Refs.Repo}}&pr={{with index .Spec.Refs.Pulls 0}}{{.Number}}{{end}})'
default_decoration_configs:
'*':
# All entries that match a job are used, later entries override previous values.
# Omission of 'repo' and 'cluster' fields makes this entry match all jobs.
- config:
timeout: 4h
grace_period: 15s
utility_images: # pull specs for container images used to construct job pods
Expand All @@ -37,5 +39,14 @@ plank:
gcs_credentials_secret: <secret-name> # the name of the secret that stores cloud provider credentials
ssh_key_secrets:
- ssh-secret # name of the secret that stores the bot's ssh keys for GitHub, doesn't matter what the key of the map is and it will just uses the values
- repo: "^org/" # some regexp to match against <org/repo>
config:
timeout:2h
- cluster: "-trusted$" #some regexp to match against the cluster name
config:
# example override to use k8s SA with GCP workload identity rather than
# a GCP service account key file.
gcs_credentials_secret: ""
default_service_account_name: <k8s-sa-name>
```
1 change: 1 addition & 0 deletions prow/config/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ go_test(
"@io_k8s_apimachinery//pkg/util/diff:go_default_library",
"@io_k8s_apimachinery//pkg/util/errors:go_default_library",
"@io_k8s_apimachinery//pkg/util/sets:go_default_library",
"@io_k8s_sigs_yaml//:go_default_library",
"@io_k8s_utils//pointer:go_default_library",
],
)
Expand Down
Loading

0 comments on commit 1acac76

Please sign in to comment.