Skip to content

Commit

Permalink
fix: evaluate ignoreChanges before checking if pre-submit should alwa…
Browse files Browse the repository at this point in the history 
…ys run

Signed-off-by: ankitm123 <[email protected]>
  • Loading branch information
@ankitm123
ankitm123 committed Sep 25, 2022
1 parent c12ac7d commit de62502
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 50 deletions.
14 changes: 7 additions & 7 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,31 +36,31 @@ build: build-webhooks build-poller build-keeper build-foghorn build-tekton-contr

.PHONY: build-webhooks
build-webhooks: ## Build the webhooks controller binary for the native OS
$(GO) build -i -ldflags "$(GO_LDFLAGS)" -o bin/$(WEBHOOKS_EXECUTABLE) $(WEBHOOKS_MAIN_SRC_FILE)
$(GO) build -ldflags "$(GO_LDFLAGS)" -o bin/$(WEBHOOKS_EXECUTABLE) $(WEBHOOKS_MAIN_SRC_FILE)

.PHONY: build-poller
build-poller: ## Build the poller controller binary for the native OS
$(GO) build -i -ldflags "$(GO_LDFLAGS)" -o bin/$(POLLER_EXECUTABLE) $(POLLER_MAIN_SRC_FILE)
$(GO) build -ldflags "$(GO_LDFLAGS)" -o bin/$(POLLER_EXECUTABLE) $(POLLER_MAIN_SRC_FILE)

.PHONY: build-keeper
build-keeper: ## Build the keeper controller binary for the native OS
$(GO) build -i -ldflags "$(GO_LDFLAGS)" -o bin/$(KEEPER_EXECUTABLE) $(KEEPER_MAIN_SRC_FILE)
$(GO) build -ldflags "$(GO_LDFLAGS)" -o bin/$(KEEPER_EXECUTABLE) $(KEEPER_MAIN_SRC_FILE)

.PHONY: build-foghorn
build-foghorn: ## Build the foghorn controller binary for the native OS
$(GO) build -i -ldflags "$(GO_LDFLAGS)" -o bin/$(FOGHORN_EXECUTABLE) $(FOGHORN_MAIN_SRC_FILE)
$(GO) build -ldflags "$(GO_LDFLAGS)" -o bin/$(FOGHORN_EXECUTABLE) $(FOGHORN_MAIN_SRC_FILE)

.PHONY: build-gc-jobs
build-gc-jobs: ## Build the GC jobs binary for the native OS
$(GO) build -i -ldflags "$(GO_LDFLAGS)" -o bin/$(GC_JOBS_EXECUTABLE) $(GC_JOBS_MAIN_SRC_FILE)
$(GO) build -ldflags "$(GO_LDFLAGS)" -o bin/$(GC_JOBS_EXECUTABLE) $(GC_JOBS_MAIN_SRC_FILE)

.PHONY: build-tekton-controller
build-tekton-controller: ## Build the Tekton controller binary for the native OS
$(GO) build -i -ldflags "$(GO_LDFLAGS)" -o bin/$(TEKTON_CONTROLLER_EXECUTABLE) $(TEKTON_CONTROLLER_MAIN_SRC_FILE)
$(GO) build -ldflags "$(GO_LDFLAGS)" -o bin/$(TEKTON_CONTROLLER_EXECUTABLE) $(TEKTON_CONTROLLER_MAIN_SRC_FILE)

.PHONY: build-jenkins-controller
build-jenkins-controller: ## Build the Jenkins controller binary for the native OS
$(GO) build -i -ldflags "$(GO_LDFLAGS)" -o bin/$(JENKINS_CONTROLLER_EXECUTABLE) $(JENKINS_CONTROLLER_MAIN_SRC_FILE)
$(GO) build -ldflags "$(GO_LDFLAGS)" -o bin/$(JENKINS_CONTROLLER_EXECUTABLE) $(JENKINS_CONTROLLER_MAIN_SRC_FILE)

.PHONY: release
release: linux
Expand Down
34 changes: 0 additions & 34 deletions pkg/config/job/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,40 +78,6 @@ func (c *Config) Merge(other Config) error {

// Init sets defaults and initializes Config
func (c *Config) Init(lh lighthouse.Config) error {
decoration := c.DecorationRequested()
if decoration {
// if c.Plank.DefaultDecorationConfig == nil {
// return errors.New("no default decoration config provided for plank")
// }
// if c.Plank.DefaultDecorationConfig.UtilityImages == nil {
// return errors.New("no default decoration image pull specs provided for plank")
// }
// if c.Plank.DefaultDecorationConfig.GCSConfiguration == nil {
// return errors.New("no default GCS decoration config provided for plank")
// }
// if c.Plank.DefaultDecorationConfig.GCSCredentialsSecret == "" {
// return errors.New("no default GCS credentials secret provided for plank")
// }
// for _, vs := range c.Presubmits {
// for i := range vs {
// // if ps.Decorate {
// // ps.DecorationConfig = ps.DecorationConfig.ApplyDefault(c.Plank.DefaultDecorationConfig)
// // }
// }
// }
// for _, js := range c.Postsubmits {
// for i := range js {
// // if ps.Decorate {
// // ps.DecorationConfig = ps.DecorationConfig.ApplyDefault(c.Plank.DefaultDecorationConfig)
// // }
// }
// }
// for i := range c.Periodics {
// // if ps.Decorate {
// // ps.DecorationConfig = ps.DecorationConfig.ApplyDefault(c.Plank.DefaultDecorationConfig)
// // }
// }
}
for _, ps := range c.Presubmits {
for i := range ps {
ps[i].SetDefaults(lh.PodNamespace)
Expand Down
18 changes: 11 additions & 7 deletions pkg/config/job/presubmit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type Presubmit struct {
RegexpChangeMatcher
Reporter
// AlwaysRun automatically for every PR, or only when a comment triggers it.
// However if A PR contains files that are included by the ignore changes regex, then a build wont be triggered
AlwaysRun bool `json:"always_run"`
// RequireRun if this value is true and AlwaysRun is false then we need to manually trigger this context for the PR to be allowed to auto merge.
RequireRun bool `json:"require_run,omitempty"`
Expand All @@ -47,7 +48,7 @@ type Presubmit struct {
JenkinsSpec *JenkinsSpec `json:"jenkins_spec,omitempty"`

// We'll set these when we load it.
//re *regexp.Regexp // from Trigger.
// re *regexp.Regexp // from Trigger.
}

// SetDefaults initializes default values
Expand Down Expand Up @@ -108,19 +109,22 @@ func (p Presubmit) ShouldRun(baseRef string, changes ChangedFilesProvider, force
if !p.CouldRun(baseRef) {
return false, nil
}

// Evaluate regex expressions before checking if pre-submit jobs are always supposed to run
if determined, shouldRun, err := p.RegexpChangeMatcher.ShouldRun(changes); err != nil {
return false, err
} else if determined {
return shouldRun, nil
}

// TODO temporary disable RequireRun
//if p.AlwaysRun || p.RequireRun {
// if p.AlwaysRun || p.RequireRun {
if p.AlwaysRun {
return true, nil
}
if forced {
return true, nil
}
if determined, shouldRun, err := p.RegexpChangeMatcher.ShouldRun(changes); err != nil {
return false, err
} else if determined {
return shouldRun, nil
}
return defaults, nil
}

Expand Down
1 change: 0 additions & 1 deletion pkg/jobutil/filter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ func AggregateFilter(filters []Filter) Filter {
// FilterPresubmits determines which presubmits should run and which should be skipped
// by evaluating the user-provided filter.
func FilterPresubmits(filter Filter, changes job.ChangedFilesProvider, branch string, presubmits []job.Presubmit, logger *logrus.Entry) ([]job.Presubmit, []job.Presubmit, error) {

var toTrigger []job.Presubmit
var namesToTrigger []string
var toSkipSuperset []job.Presubmit
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugins/trigger/generic-comment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func handleGenericComment(c Client, trigger *plugins.Trigger, gc scmprovider.Gen
return err
}
if !trusted {
resp := fmt.Sprintf("Cannot trigger testing until a trusted user reviews the PR and leaves an `/ok-to-test` message.")
resp := "Cannot trigger testing until a trusted user reviews the PR and leaves an `/ok-to-test` message."
c.Logger.Infof("Commenting \"%s\".", resp)
return c.SCMProviderClient.CreateComment(org, repo, number, true, plugins.FormatResponseRaw(gc.Body, gc.Link, c.SCMProviderClient.QuoteAuthorForComment(gc.Author.Login), resp))
}
Expand Down

0 comments on commit de62502

Please sign in to comment.