Repository: jenkins-x/lighthouse
Pull request #1398 (2021-12-15 21:43:20)

feat: Support poller require release success
Adds a flag to keeping polling releases until the commit status is successful. This can be useful if you always expect releases to succeed.

The main issue with this implementation is that a more recent poll might overwrite an already successful status if the jobs overlap, causing unnecessary/continuous polling (since the go-scm library [only exposes the most recent status](https://github.com/jenkins-x/go-scm/blob/e6613ef378a644b1c464409a88d27f59484fffa8/scm/driver/github/repo.go#L462-L465) for a particular context) so the user needs to ensure the poll interval is long enough to reduce the chance of this happening (e.g. longer than the job timeout). I have created an issue and a WIP PR to allow go-scm to support fixing this issue (for GitHub at least): https://github.com/jenkins-x/go-scm/issues/314

jenkins-x-bot: Hi @dippynark. Thanks for your PR.

I'm waiting for a [jenkins-x](https://github.com/orgs/jenkins-x/people) or [todo](https://github.com/orgs/todo/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://github.com/orgs/todo/people) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://jenkins-x.io/v3/develop/reference/chatops/?repo=jenkins-x%2Flighthouse).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository.
</details>

jenkins-x-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **pmuir**
You can assign the PR to them by writing `/assign @pmuir` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://jenkins-x.io/v3/develop/reference/chatops/?repo=jenkins-x%2Flighthouse).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/jenkins-x/lighthouse/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["pmuir"]} -->
ankitm123: /ok-to-test

Patch:
diff --git a/charts/lighthouse/templates/poller-deployment.yaml b/charts/lighthouse/templates/poller-deployment.yaml
index f48b2e79..345dabd8 100644
--- a/charts/lighthouse/templates/poller-deployment.yaml
+++ b/charts/lighthouse/templates/poller-deployment.yaml
@@ -41,6 +41,9 @@ spec:
           {{- if .Values.poller.contextMatchPattern }}
           - "--context-match-pattern={{ .Values.poller.contextMatchPattern }}"
           {{- end }}
+          {{- if .Values.poller.requireReleaseSuccess }}
+          - --require-release-success
+          {{- end }}
         ports:
           - name: http
             containerPort: {{ .Values.poller.internalPort }}
diff --git a/charts/lighthouse/values.yaml b/charts/lighthouse/values.yaml
index 61133ba4..6d64d28d 100644
--- a/charts/lighthouse/values.yaml
+++ b/charts/lighthouse/values.yaml
@@ -442,6 +442,9 @@ poller:
   # poller.contextMatchPattern -- Regex pattern to use to match commit status context
   contextMatchPattern: ""
 
+  # poller.requireReleaseSuccess -- Keep polling releases until the most recent commit status is successful
+  requireReleaseSuccess: false
+
   resources:
     # poller.resources.limits -- Resource limits applied to the poller pods
     limits:
diff --git a/cmd/poller/main.go b/cmd/poller/main.go
index 8cf22df5..ba02e93e 100644
--- a/cmd/poller/main.go
+++ b/cmd/poller/main.go
@@ -44,6 +44,7 @@ type options struct {
 	contextMatchPattern    string
 	runOnce                bool
 	dryRun                 bool
+	requireReleaseSuccess  bool
 	disablePollRelease     bool
 	disablePollPullRequest bool
 	pollPeriod             time.Duration
@@ -71,6 +72,7 @@ func gatherOptions(fs *flag.FlagSet, args ...string) options {
 	fs.BoolVar(&o.dryRun, "dry-run", false, "Disable POSTing to the webhook service and just log the webhooks instead.")
 	fs.BoolVar(&o.disablePollRelease, "no-release", false, "Disable polling for new commits on the main branch (releases) - mostly used for easier testing/debugging.")
 	fs.BoolVar(&o.disablePollPullRequest, "no-pr", false, "Disable polling for Pull Request changes - mostly used for easier testing/debugging.")
+	fs.BoolVar(&o.requireReleaseSuccess, "require-release-success", false, "Keep polling releases until the most recent commit status is successful.")
 
 	fs.StringVar(&o.namespace, "namespace", "jx", "The namespace to listen in")
 	fs.StringVar(&o.repoNames, "repo", "", "The git repository names to poll. If not specified all the repositories are polled")
@@ -218,7 +220,7 @@ func main() {
 		logrus.WithError(err).Fatal("failed to create scm client")
 	}
 
-	c, err := poller.NewPollingController(repoNames, serverURL, scmClient, contextMatchPatternCompiled, fb, o.notifier)
+	c, err := poller.NewPollingController(repoNames, serverURL, scmClient, contextMatchPatternCompiled, o.requireReleaseSuccess, fb, o.notifier)
 	if err != nil {
 		logrus.WithError(err).Fatal("Error creating Poller controller.")
 	}
diff --git a/pkg/poller/poller.go b/pkg/poller/poller.go
index ebc6f602..1d088819 100644
--- a/pkg/poller/poller.go
+++ b/pkg/poller/poller.go
@@ -29,6 +29,7 @@ type pollingController struct {
 	pollstate                   pollstate.Interface
 	logger                      *logrus.Entry
 	contextMatchPatternCompiled *regexp.Regexp
+	requireReleaseSuccess       bool
 	notifier                    func(webhook *scm.WebhookWrapper) error
 }
 
@@ -36,7 +37,7 @@ func (c *pollingController) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte("hello from lighthouse poller\n"))
 }
 
-func NewPollingController(repositoryNames []string, gitServer string, scmClient *scm.Client, contextMatchPatternCompiled *regexp.Regexp, fb filebrowser.Interface, notifier func(webhook *scm.WebhookWrapper) error) (*pollingController, error) {
+func NewPollingController(repositoryNames []string, gitServer string, scmClient *scm.Client, contextMatchPatternCompiled *regexp.Regexp, requireReleaseSuccess bool, fb filebrowser.Interface, notifier func(webhook *scm.WebhookWrapper) error) (*pollingController, error) {
 	logger := logrus.NewEntry(logrus.StandardLogger())
 	if gitServer == "" {
 		gitServer = "https://github.com"
@@ -47,6 +48,7 @@ func NewPollingController(repositoryNames []string, gitServer string, scmClient
 		logger:                      logger,
 		scmClient:                   scmClient,
 		contextMatchPatternCompiled: contextMatchPatternCompiled,
+		requireReleaseSuccess:       requireReleaseSuccess,
 		fb:                          fb,
 		notifier:                    notifier,
 		pollstate:                   pollstate.NewMemoryPollState(),
@@ -113,7 +115,7 @@ func (c *pollingController) PollReleases() {
 			}
 
 			// lets check we have not triggered this before
-			hasStatus, err := c.hasStatusForSHA(ctx, l, fullName, sha)
+			hasStatus, err := c.hasStatusForSHA(ctx, l, fullName, sha, true)
 			if err != nil {
 				return errors.Wrapf(err, "failed to check for status ")
 			}
@@ -121,6 +123,11 @@ func (c *pollingController) PollReleases() {
 				return nil
 			}
 
+			if c.requireReleaseSuccess {
+				// We have not been able to find a successful release status so invalidate cache to retry
+				c.pollstate.Invalidate(fullName, "release", sha)
+			}
+
 			l.Infof("triggering release webhook")
 
 			before := ""
@@ -206,7 +213,7 @@ func (c *pollingController) pollPullRequest(ctx context.Context, l *logrus.Entry
 		return nil
 	}
 
-	hasStatus, err := c.hasStatusForSHA(ctx, l, fullName, sha)
+	hasStatus, err := c.hasStatusForSHA(ctx, l, fullName, sha, false)
 	if err != nil {
 		return errors.Wrapf(err, "failed to check for status ")
 	}
@@ -241,7 +248,7 @@ func (c *pollingController) pollPullRequestPushHook(ctx context.Context, l *logr
 		return nil
 	}
 
-	hasStatus, err := c.hasStatusForSHA(ctx, l, fullName, sha)
+	hasStatus, err := c.hasStatusForSHA(ctx, l, fullName, sha, false)
 	if err != nil {
 		return errors.Wrapf(err, "failed to check for status ")
 	}
@@ -275,7 +282,7 @@ func (c *pollingController) pollPullRequestPushHook(ctx context.Context, l *logr
 	return nil
 }
 
-func (c *pollingController) hasStatusForSHA(ctx context.Context, l *logrus.Entry, fullName string, sha string) (bool, error) {
+func (c *pollingController) hasStatusForSHA(ctx context.Context, l *logrus.Entry, fullName string, sha string, isRelease bool) (bool, error) {
 	opts := scm.ListOptions{
 		Page: 1,
 	}
@@ -284,7 +291,7 @@ func (c *pollingController) hasStatusForSHA(ctx context.Context, l *logrus.Entry
 		return false, errors.Wrapf(err, "failed to list status")
 	}
 	for _, s := range statuses {
-		if c.isMatchingStatus(s) {
+		if c.isMatchingStatus(s, isRelease) {
 			l.WithField("Statuses", statuses).Info("the SHA has CI statuses so not triggering")
 			return true, nil
 		}
@@ -292,7 +299,13 @@ func (c *pollingController) hasStatusForSHA(ctx context.Context, l *logrus.Entry
 	return false, nil
 }
 
-func (c *pollingController) isMatchingStatus(s *scm.Status) bool {
+func (c *pollingController) isMatchingStatus(s *scm.Status, isRelease bool) bool {
+	if isRelease && c.requireReleaseSuccess {
+		if s.State != scm.StateSuccess {
+			return false
+		}
+	}
+
 	if c.contextMatchPatternCompiled != nil {
 		return c.contextMatchPatternCompiled.MatchString(s.Label)
 	}


Test patch:
diff --git a/pkg/poller/poller_test.go b/pkg/poller/poller_test.go
index 027eb973..e0b15fb6 100644
--- a/pkg/poller/poller_test.go
+++ b/pkg/poller/poller_test.go
@@ -17,11 +17,12 @@ import (
 )
 
 var (
-	repoNames           = []string{"myorg/myrepo"}
-	gitServer           = "https://github.com"
-	testDataDir         = "test_data"
-	contextMatchPattern = "^Lighthouse$"
-	statusLabel         = "Jenkins"
+	repoNames             = []string{"myorg/myrepo"}
+	gitServer             = "https://github.com"
+	testDataDir           = "test_data"
+	contextMatchPattern   = "^Lighthouse$"
+	statusLabel           = "Jenkins"
+	requireReleaseSuccess = true
 )
 
 func TestPollerReleases(t *testing.T) {
@@ -49,7 +50,7 @@ func TestPollerReleases(t *testing.T) {
 	sha := strings.TrimSpace(string(out))
 	fakeData.Statuses = map[string][]*scm.Status{sha: {{Label: statusLabel}}}
 
-	p, err := poller.NewPollingController(repoNames, gitServer, scmClient, contextMatchPatternCompiled, fb, fakeNotifier)
+	p, err := poller.NewPollingController(repoNames, gitServer, scmClient, contextMatchPatternCompiled, requireReleaseSuccess, fb, fakeNotifier)
 	require.NoError(t, err, "failed to create PollingController")
 
 	p.PollReleases()
@@ -107,7 +108,7 @@ func TestPollerPullRequests(t *testing.T) {
 	// Load fake status with label that doesn't match our context match pattern
 	fakeData.Statuses = map[string][]*scm.Status{sha: {{Label: statusLabel}}}
 
-	p, err := poller.NewPollingController(repoNames, gitServer, scmClient, contextMatchPatternCompiled, fb, fakeNotifier)
+	p, err := poller.NewPollingController(repoNames, gitServer, scmClient, contextMatchPatternCompiled, requireReleaseSuccess, fb, fakeNotifier)
 	require.NoError(t, err, "failed to create PollingController")
 
 	p.PollPullRequests()