Skip to content

prowgen: DRY out JobBase generation #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 1 commit into from
Jul 2, 2019
Merged

prowgen: DRY out JobBase generation #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 30 additions & 56 deletions cmd/ci-operator-prowgen/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ const (
sentryDsnMountPath = "/etc/sentry-dsn"
sentryDsnSecretPath = "/etc/sentry-dsn/ci-operator"

presubmitPrefix = "pull"
postsubmitPrefix = "branch"

openshiftInstallerRandomCmd = `set -eux
target=$(awk < /usr/local/e2e-targets \
--assign "r=$RANDOM" \
Expand Down Expand Up @@ -384,12 +387,11 @@ func generatePodSpecRandom(info *config.Info, test *cioperatorapi.TestStepConfig
return podSpec
}

func generatePresubmitForTest(name string, info *config.Info, podSpec *kubeapi.PodSpec) *prowconfig.Presubmit {
func generateJobBase(name, prefix string, info *config.Info, podSpec *kubeapi.PodSpec) prowconfig.JobBase {
labels := map[string]string{jc.ProwJobLabelGenerated: jc.Generated}

jobPrefix := fmt.Sprintf("pull-ci-%s-%s-%s-", info.Org, info.Repo, info.Branch)
jobPrefix := fmt.Sprintf("%s-ci-%s-%s-%s-", prefix, info.Org, info.Repo, info.Branch)
if len(info.Variant) > 0 {
name = fmt.Sprintf("%s-%s", info.Variant, name)
labels[prowJobLabelVariant] = info.Variant
}
jobName := fmt.Sprintf("%s%s", jobPrefix, name)
Expand All @@ -399,18 +401,25 @@ func generatePresubmitForTest(name string, info *config.Info, podSpec *kubeapi.P
}

newTrue := true
return prowconfig.JobBase{
Agent: "kubernetes",
Labels: labels,
Name: jobName,
Spec: podSpec,
UtilityConfig: prowconfig.UtilityConfig{
DecorationConfig: &v1.DecorationConfig{SkipCloning: &newTrue},
Decorate: true,
},
}
}

func generatePresubmitForTest(name string, info *config.Info, podSpec *kubeapi.PodSpec) *prowconfig.Presubmit {
if len(info.Variant) > 0 {
name = fmt.Sprintf("%s-%s", info.Variant, name)
Copy link
Contributor

@stevekuznetsov stevekuznetsov Jul 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this more appropriate outside of generateJobBase?

Copy link
Member Author

@petr-muller petr-muller Jul 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to have this string on this level to make the Reporter/RerunCommand/Trigger properties of presubmits (L425-L429).

}
base := generateJobBase(name, presubmitPrefix, info, podSpec)
return &prowconfig.Presubmit{
JobBase: prowconfig.JobBase{
Agent: "kubernetes",
Labels: labels,
Name: jobName,
Spec: podSpec,
UtilityConfig: prowconfig.UtilityConfig{
DecorationConfig: &v1.DecorationConfig{SkipCloning: &newTrue},
Decorate: true,
},
},
JobBase: base,
AlwaysRun: true,
Brancher: prowconfig.Brancher{Branches: []string{info.Branch}},
Reporter: prowconfig.Reporter{
Expand All @@ -421,46 +430,14 @@ func generatePresubmitForTest(name string, info *config.Info, podSpec *kubeapi.P
}
}

func generatePostsubmitForTest(
name string,
info *config.Info,
treatBranchesAsExplicit bool,
podSpec *kubeapi.PodSpec) *prowconfig.Postsubmit {

labels := make(map[string]string)
labels[jc.ProwJobLabelGenerated] = jc.Generated

branchName := jc.MakeRegexFilenameLabel(info.Branch)
jobPrefix := fmt.Sprintf("branch-ci-%s-%s-%s-", info.Org, info.Repo, branchName)
func generatePostsubmitForTest(name string, info *config.Info, podSpec *kubeapi.PodSpec) *prowconfig.Postsubmit {
if len(info.Variant) > 0 {
name = fmt.Sprintf("%s-%s", info.Variant, name)
labels[prowJobLabelVariant] = info.Variant
}
jobName := fmt.Sprintf("%s%s", jobPrefix, name)
if len(jobName) > 63 && len(jobPrefix) < 53 {
// warn if the prefix gives people enough space to choose names and they've chosen something long
logrus.WithField("name", jobName).Warn("Generated job name is longer than 63 characters. This may cause issues when Prow attempts to label resources with job name. Consider a shorter name.")
}

branch := info.Branch
if treatBranchesAsExplicit {
branch = makeBranchExplicit(branch)
}

newTrue := true

base := generateJobBase(name, postsubmitPrefix, info, podSpec)
return &prowconfig.Postsubmit{
JobBase: prowconfig.JobBase{
Agent: "kubernetes",
Name: jobName,
Spec: podSpec,
Labels: labels,
UtilityConfig: prowconfig.UtilityConfig{
DecorationConfig: &v1.DecorationConfig{SkipCloning: &newTrue},
Decorate: true,
},
},
Brancher: prowconfig.Brancher{Branches: []string{branch}},
JobBase: base,
Brancher: prowconfig.Brancher{Branches: []string{makeBranchExplicit(info.Branch)}},
}
}

Expand Down Expand Up @@ -494,6 +471,7 @@ func generateJobs(
podSpec = generatePodSpecTemplate(info, release, &element)
}
}

presubmits[orgrepo] = append(presubmits[orgrepo], *generatePresubmitForTest(element.As, info, podSpec))
}

Expand All @@ -503,18 +481,14 @@ func generateJobs(
if promotion.PromotesOfficialImages(configSpec) {
additionalPresubmitArgs = []string{"--target=[release:latest]"}
}
presubmits[orgrepo] = append(presubmits[orgrepo], *generatePresubmitForTest("images", info, generateCiOperatorPodSpec(info, "[images]", additionalPresubmitArgs...)))

additionalPostsubmitArgs := []string{"--promote"}
if configSpec.PromotionConfiguration != nil {
additionalPostsubmitArgs := []string{"--promote"}
for additionalImage := range configSpec.PromotionConfiguration.AdditionalImages {
additionalPostsubmitArgs = append(additionalPostsubmitArgs, fmt.Sprintf("--target=%s", configSpec.PromotionConfiguration.AdditionalImages[additionalImage]))
}
}

presubmits[orgrepo] = append(presubmits[orgrepo], *generatePresubmitForTest("images", info, generateCiOperatorPodSpec(info, "[images]", additionalPresubmitArgs...)))

if configSpec.PromotionConfiguration != nil {
postsubmits[orgrepo] = append(postsubmits[orgrepo], *generatePostsubmitForTest("images", info, true, generateCiOperatorPodSpec(info, "[images]", additionalPostsubmitArgs...)))
postsubmits[orgrepo] = append(postsubmits[orgrepo], *generatePostsubmitForTest("images", info, generateCiOperatorPodSpec(info, "[images]", additionalPostsubmitArgs...)))
}
}

Expand Down
33 changes: 3 additions & 30 deletions cmd/ci-operator-prowgen/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,6 @@ func TestGeneratePostSubmitForTest(t *testing.T) {
name string
repoInfo *config.Info

treatBranchesAsExplicit bool

expected *prowconfig.Postsubmit
}{
{
Expand All @@ -528,7 +526,7 @@ func TestGeneratePostSubmitForTest(t *testing.T) {
},
},

Brancher: prowconfig.Brancher{Branches: []string{"branch"}},
Brancher: prowconfig.Brancher{Branches: []string{"^branch$"}},
},
},
{
Expand All @@ -548,7 +546,7 @@ func TestGeneratePostSubmitForTest(t *testing.T) {
DecorationConfig: &v1.DecorationConfig{SkipCloning: &newTrue},
Decorate: true,
}},
Brancher: prowconfig.Brancher{Branches: []string{"Branch"}},
Brancher: prowconfig.Brancher{Branches: []string{"^Branch$"}},
},
},
{
Expand All @@ -559,8 +557,6 @@ func TestGeneratePostSubmitForTest(t *testing.T) {
Branch: "Branch",
},

treatBranchesAsExplicit: true,

expected: &prowconfig.Postsubmit{
JobBase: prowconfig.JobBase{
Agent: "kubernetes",
Expand All @@ -573,32 +569,9 @@ func TestGeneratePostSubmitForTest(t *testing.T) {
Brancher: prowconfig.Brancher{Branches: []string{"^Branch$"}},
},
},

{
name: "name",
repoInfo: &config.Info{
Org: "Organization",
Repo: "Repository",
Branch: "Branch-.*",
},

treatBranchesAsExplicit: true,

expected: &prowconfig.Postsubmit{
JobBase: prowconfig.JobBase{
Agent: "kubernetes",
Name: "branch-ci-Organization-Repository-Branch-name",
Labels: map[string]string{"ci-operator.openshift.io/prowgen-controlled": "true"},
UtilityConfig: prowconfig.UtilityConfig{
DecorationConfig: &v1.DecorationConfig{SkipCloning: &newTrue},
Decorate: true,
}},
Brancher: prowconfig.Brancher{Branches: []string{"Branch-.*"}},
},
},
}
for _, tc := range tests {
postsubmit := generatePostsubmitForTest(tc.name, tc.repoInfo, tc.treatBranchesAsExplicit, nil) // podSpec tested in TestGeneratePodSpec
postsubmit := generatePostsubmitForTest(tc.name, tc.repoInfo, nil) // podSpec tested in TestGeneratePodSpec
if !equality.Semantic.DeepEqual(postsubmit, tc.expected) {
t.Errorf("expected postsubmit diff:\n%s", diff.ObjectDiff(tc.expected, postsubmit))
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/jobconfig/files.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ func writeToFile(path string, jobConfig *prowconfig.JobConfig) error {
return nil
}

var regexParts = regexp.MustCompile(`[^\w\-\.]+`)
var regexParts = regexp.MustCompile(`[^\w\-.]+`)

func MakeRegexFilenameLabel(possibleRegex string) string {
label := regexParts.ReplaceAllString(possibleRegex, "")
Expand Down