fedora-infra / loopabull-tasks

Clone
Source Code
GIT

#1 PR flags: adds support for running/pending state
Merged 5 years ago by pingou. Opened 5 years ago by mvadkert.
fedora-infra/ mvadkert/loopabull-tasks fedora-ci/general/3  into  master

file modified
+2 -2 
@@ -1,5 +1,5 @@ 
 

  ---
 

- - name: buildsys.build.state.change
 

+ - name: ci.pipeline.allpackages-pr.complete
 

    hosts: localhost
 

    gather_facts: false
 

  
@@ -10,5 +10,5 @@ 
 

      - debug: var=msg
 

  

    roles:
 

-     - {role: flag_ci_pr, msg: msg}
 

+     - {role: flag_ci_pr, msg: msg, state: "running", seed_flag_ci_pr: seed_flag_ci_pr}

file added
+14 
@@ -0,0 +1,14 @@ 
 

+ ---
 

+ - name: ci.pipeline.allpackages-pr.package.running
 

+   hosts: localhost
 

+   gather_facts: false
 

+ 

+   vars_files:
 

+    - "/srv/private/vars_loopabull.yml"
 

+ 

+   tasks:
 

+     - debug: var=msg
 

+ 

+   roles:
 

+     - {role: flag_ci_pr, msg: msg, state: "running", seed_flag_ci_pr: seed_flag_ci_pr}
 

+

file modified
+32 -15 
@@ -12,6 +12,7 @@ 
 

  """
 

  from __future__ import unicode_literals
 

  

+ import hashlib
 

  import json
 

  import os
 

  import sys
 
@@ -19,11 +20,13 @@ 
 

  import requests
 

  from requests.packages.urllib3.util import retry
 

  

- 

- 

- def main(msg):
 

+ def main(msg, pipeline_state='complete', seed='empty'):
 

      """ Check if the build was successful and if so, flag the commit in
 

      pagure.
 

+ 

+     :param str msg: the fedmsg message body
 

+     :param str pipeline_state: state of the testing, one of 'complete'
 

+                                or 'running'
 

      """
 

  

      timeout = (30, 30)
 
@@ -37,16 +40,30 @@ 
 

      requests_session.mount(
 

          'https://', requests.adapters.HTTPAdapter(max_retries=retry_conf))
 

  

-     done_states = {
 

-         'SUCCESS': {'api': 'success', 'human': 'passed'},
 

-         'UNSTABLE': {'api': 'failure', 'human': 'failed'},
 

-         'FAILURE': {'api': 'error', 'human': 'errored'},
 

-     }
 

-     state = msg['status']
 

-     if state not in done_states:
 

-         print('Build is not in one of the expected status, ignoring')
 

+     if pipeline_state not in ['complete', 'running']:
 

+         print("Pipeline state is not 'complete' or 'running'.")
 

          return
 

  

+     # test complete messages
 

+     if pipeline_state == 'complete':
 

+         done_states = {
 

+             'SUCCESS': {'api': 'success', 'human': 'passed'},
 

+             'UNSTABLE': {'api': 'failure', 'human': 'failed'},
 

+             'FAILURE': {'api': 'error', 'human': 'errored'},
 

+         }
 

+         state = msg['status']
 

+         if state not in done_states:
 

+             print('Build is not in one of the expected status, ignoring')
 

+             return
 

+ 

+         status = done_states[state]['api']
 

+         human_status = done_states[state]['human']
 

+ 

+     # test running messages
 

+     elif pipeline_state == 'running':
 

+         status = 'pending'
 

+         human_status = 'running'
 

+ 

      pr_id = msg['rev'].partition('PR-')[2]
 

      if not pr_id:
 

          print(
 
@@ -54,12 +71,12 @@ 
 

              msg['rev'])
 

          return
 

  

- 

      data = {
 

          'username': 'Fedora CI',
 

-         'status': done_states[state]['api'],
 

-         'comment': 'Package tests: %s' % (done_states[state]['human']),
 

+         'status': status,
 

+         'comment': 'Package tests: %s' % human_status,
 

          'url': msg['build_url'],
 

+         'uid': hashlib.md5(pr_id + seed).hexdigest()
 

      }
 

  

      pagure_url = 'https://src.fedoraproject.org'
 
@@ -107,4 +124,4 @@ 
 

  if __name__ == '__main__':
 

      msg = sys.argv[1]
 

      msg = json.loads(msg)
 

-     sys.exit(main(msg))
 

+     sys.exit(main(msg, sys.argv[2], sys.argv[3]))

file modified
+1 -1 
@@ -5,7 +5,7 @@ 
 

        mode: 0755
 

  

  - name: Run the script
 

-   command: /usr/local/bin/flag_ci_pr.py {{ msg | to_json | quote }}
 

+   command: /usr/local/bin/flag_ci_pr.py {{ msg | to_json | quote }} {{ state | quote }} {{ seed_flag_pr_ci | quote }}
 

    register: output
 

    environment:
 

        API_TOKEN: "{{ api_token_flag_pr_ci }}"

This patch adds support for runinng state for Fedora CI.

It keeps only the recent flag around, so running transitions nicely
to complete/error state and vice versa (for reruns).

Resolves https://pagure.io/fedora-ci/general/issue/3

Signed-off-by: Miroslav Vadkerti mvadkert@redhat.com

@pingou @psss @bookwar kindly asking for review

I tested the script and it seems to work on our staging pagure instance we have internally. I was not able to test loop-a-bull ...

I don't understand the implementation in detail but in general the proposed approach seems fine to me: Showing only one flag with either running or complete/error status is probably the best solution for now, until we get proper handling of pull requests versions/rebases.

@kevev @puiterwijk @smooge @pingou hi, would anybody have time to review this pls? We would like to show this feature on DevConf CZ. Thanks very much.

Let's add a default value to this variable so we're backward compatible

s/if/elif/

let's find a way to generate these, it shouldn't be a static and public string.

rebased onto f4fd4c6ba2315be8cbfaa54cb63e0eaf976545f3
5 years ago

@pingou all issues addressed, politely asking for another round of review

rebased onto 3ccd19eca60777f6ec6c6ae7e4ab73a8b7ebf4e0
5 years ago

We need to come up with a better way to generate the uid, this is still too weak :(

@pingou can you elaborate what is the problem, please? Is it that someone cannot fake easily the result or? Should I add more strings to the generation, so it is not just one value hashed? ... btw on downstream we just remove all old flags with "[citest]", is that something we do not want to do here?

The problem we were trying to solve with this was: https://pagure.io/fedora-ci/general/issue/1 (if we have just one result, no need to write which commit it relates to). Also with a rebase the reference to hash makes little sense.

rebased onto 342bdeed29febe892d51084a057cc5b8183d367a
5 years ago

rebased onto e803bad6487dedd00f76118d7977022091c58297
5 years ago

@pingou updated to use seed_flag_ci_pr variable for hash calculation. Please take a look. ty!

Looks fine to me :)

Pull-Request has been merged by pingou
5 years ago
Metadata
None
No Tags
Changes Summary 4
+2 -2
file changed
playbooks/org.centos.prod.ci.pipeline.allpackages-pr.complete.yml
+14
file added
playbooks/org.centos.prod.ci.pipeline.allpackages-pr.package.running.yml
+32 -15
file changed
playbooks/roles/flag_ci_pr/files/flag_ci_pr.py
+1 -1
file changed
playbooks/roles/flag_ci_pr/tasks/main.yml
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.