Differential D1043

move docker stuff to thread with waiting and retry
AbandonedPublic
Actions

Authored by lbrabec on Nov 1 2016, 11:53 AM.

Details

Reviewers
kparal
tflink
jskladan
Summary

Changes introduced in D1002 are not working properly in real environment.
This diff moves the related docker stuff to thread, where several retries
take place before raising.

Test Plan

py.test

Diff Detail

Repository
rTRGR taskotron-trigger
Branch
fix/thread (branched from develop)
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 900
Build 900: arc lint + arc unit
lbrabec retitled this revision from to move docker stuff to thread with waiting and retry.Nov 1 2016, 11:53 AM
lbrabec updated this object.
lbrabec edited the test plan for this revision. (Show Details)
lbrabec added reviewers: jskladan, tflink, kparal.
jskladan added a comment.Nov 1 2016, 12:57 PM

Note that this is a work-around for https://fedorahosted.org/koji/ticket/331 (probably), which blocks us from testing the docker images.

jskladan accepted this revision.Nov 1 2016, 12:57 PM
This revision is now accepted and ready to land.Nov 1 2016, 12:57 PM
tflink added a comment.Nov 1 2016, 3:48 PM

I don't see anything that worries me - has it been tested with real-time fedmsgs?

jobtriggers/koji_build_msg.py
79

I'd prefer to see this configurable

kparal added a comment.Nov 2 2016, 11:22 AM

Please note that T776 seems it should be resolved in a few days, which should also fix the docker issues. So I'd rather wait with this, maybe it will not be needed at all.

jobtriggers/koji_build_msg.py
44

This is missing an explanation why.

mkrizek added a subscriber: mkrizek.Feb 8 2017, 9:49 AM

This is probably not needed anymore and can be closed since the koji fix has been deployed.

kparal requested changes to this revision.Mar 13 2017, 3:54 PM

Koji has been fixed, so I assume this is no longer needed. @lbrabec, can you abandon this? Thanks.

This revision now requires changes to proceed.Mar 13 2017, 3:54 PM
lbrabec abandoned this revision.Mar 14 2017, 11:21 AM

Revision Contents

PathCoverage (All)Coverage (Touched)Packages
Mjobtriggers/koji_build_msg.py (21 lines)90%61%
Mtesting/test_koji_build_trigger.py (12 lines)100%100%

Diff 2664

View Options

jobtriggers/koji_build_msg.py

1import koji 1​import koji
2import fedmsg 2​import fedmsg
3import fedmsg.encoding 3​import fedmsg.encoding
4​import threading
5​import time
4 6​
5from . import config, utils 7​from . import config, utils
6from .jobtrigger import JobTrigger 8​from .jobtrigger import JobTrigger
7from . import exceptions as exc 9​from . import exceptions as exc
8 10​
9ITEM_TYPE_PACKAGE = 'koji_build' 11​ITEM_TYPE_PACKAGE = 'koji_build'
10ITEM_TYPE_DOCKER = 'docker_image' 12​ITEM_TYPE_DOCKER = 'docker_image'
11 13​
Show All 22 Lines35 raise exc.TriggerMsgError('rejecting message not from primary koji instance '
34 '(%s from %s)' % (nvr, instance)) 36​ '(%s from %s)' % (nvr, instance))
35 37​
36 if state != koji.BUILD_STATES['COMPLETE']: 38​ if state != koji.BUILD_STATES['COMPLETE']:
37 raise exc.TriggerMsgError('rejecting message for not being a build complete message ' 39​ raise exc.TriggerMsgError('rejecting message for not being a build complete message '
38 '(%s, status %s)' % (nvr, state)) 40​ '(%s, status %s)' % (nvr, state))
39 41​
40 # FIXME This is probably a temporary approach, could change in future, see D1002#19239 42​ # FIXME This is probably a temporary approach, could change in future, see D1002#19239
41 if owner == 'containerbuild': 43​ if owner == 'containerbuild':
42 return self._process_docker(msg) 44​ thread = threading.Thread(target=self._process_docker_retry, args=(msg,))
kparalUnsubmitted
Not Done

This is missing an explanation why.

45​ thread.daemon = True
46​ thread.start()
43 else: 47​ else:
44 if 'fc' not in release: 48​ if 'fc' not in release:
45 raise exc.TriggerMsgError('rejecting message for not containing "fc" in release ' 49​ raise exc.TriggerMsgError('rejecting message for not containing "fc" in release '
46 '(%s)' % nvr) 50​ '(%s)' % nvr)
47 return self._process_package(msg) 51​ return self._process_package(msg)
48 52​
49 def _process_package(self, msg): 53​ def _process_package(self, msg):
50 name, version, release = self.get_nvr(msg) 54​ name, version, release = self.get_nvr(msg)
Show All 9 Lines59 data = {
60 "name": name, 64​ "name": name,
61 "version": version, 65​ "version": version,
62 "release": release, 66​ "release": release,
63 "critpath_pkgs": critpath_pkgs, 67​ "critpath_pkgs": critpath_pkgs,
64 "distgit_branch": distgit_branch, 68​ "distgit_branch": distgit_branch,
65 } 69​ }
66 self.do_trigger(data) 70​ self.do_trigger(data)
67 71​
72​ def _process_docker_retry(self, msg):
73​ wait_time = 10
74​ waited = 0
75​ while True:
76​ try:
77​ return self._process_docker(msg)
78​ except exc.TriggerMsgError as e:
79​ if waited < 60*60*3:
tflinkUnsubmitted
Not Done

I'd prefer to see this configurable

80​ pass
81​ else:
82​ raise e
83​ time.sleep(wait_time)
84​ waited += wait_time
85​ wait_time *= 2
86​
68 def _process_docker(self, msg): 87​ def _process_docker(self, msg):
69 name, version, release = self.get_nvr(msg) 88​ name, version, release = self.get_nvr(msg)
70 build_id = msg['msg']['build_id'] 89​ build_id = msg['msg']['build_id']
71 item = utils.get_docker_registry_url(build_id) 90​ item = utils.get_docker_registry_url(build_id)
72 distgit_branch = utils.get_distgit_branch_docker(build_id) 91​ distgit_branch = utils.get_distgit_branch_docker(build_id)
73 critpath_pkgs = [] # FIXME do we need this for docker images? 92​ critpath_pkgs = [] # FIXME do we need this for docker images?
74 data = { 93​ data = {
75 "_msg": msg, 94​ "_msg": msg,
Show All 18 Lines
View Options

testing/test_koji_build_trigger.py

1import koji 1​import koji
2import pytest 2​import pytest
3import mock 3​import mock
4​import threading
4from dingus import Dingus 5​from dingus import Dingus
5from munch import Munch 6​from munch import Munch
6 7​
7from jobtriggers import koji_build_msg 8​from jobtriggers import koji_build_msg
8from jobtriggers import utils 9​from jobtriggers import utils
9 10​
10params = [('someowner', koji_build_msg.ITEM_TYPE_PACKAGE, "KojiBuildPackageCompleted"), 11​params = [('someowner', koji_build_msg.ITEM_TYPE_PACKAGE, "KojiBuildPackageCompleted"),
11 ('containerbuild', koji_build_msg.ITEM_TYPE_DOCKER, "KojiBuildDockerCompleted")] 12​ ('containerbuild', koji_build_msg.ITEM_TYPE_DOCKER, "KojiBuildDockerCompleted")]
12 13​
14​
15​class ThreadStub:
16​ def __init__(self, *args, **kwargs):
17​ self.kwargs = kwargs
18​
19​ def start(self):
20​ return self.kwargs['target'](*(self.kwargs['args']))
21​
22​
13@pytest.mark.usefixtures('prepare') 23​@pytest.mark.usefixtures('prepare')
14@pytest.mark.parametrize('buildtype', params) 24​@pytest.mark.parametrize('buildtype', params)
15class TestKojiBuildCompletedJobTrigger(): 25​class TestKojiBuildCompletedJobTrigger():
16 26​
17 @pytest.fixture 27​ @pytest.fixture
18 def prepare(self, monkeypatch, buildtype): 28​ def prepare(self, monkeypatch, buildtype):
19 self.ref_instance = 'primary' 29​ self.ref_instance = 'primary'
20 self.ref_new = koji.BUILD_STATES['COMPLETE'] 30​ self.ref_new = koji.BUILD_STATES['COMPLETE']
▲ Show 20 Lines • Show All 43 Lines • ▼ Show 20 Line(s)62""" % (self.ref_tasks, self.message_type, self.ref_tasks_docker, self.message_type)
64 stub_session = mock.MagicMock() 74​ stub_session = mock.MagicMock()
65 stub_session.listArchives.return_value = [ 75​ stub_session.listArchives.return_value = [
66 {'extra': {'docker': {'repositories': ['image@shaXXX', self.ref_nvr]}}}] 76​ {'extra': {'docker': {'repositories': ['image@shaXXX', self.ref_nvr]}}}]
67 stub_session.listTags.return_value = [{'name': 'ubuntu-docker'}, {'name': 'f24-docker'}] 77​ stub_session.listTags.return_value = [{'name': 'ubuntu-docker'}, {'name': 'f24-docker'}]
68 78​
69 stub_get_session = mock.MagicMock(return_value=stub_session) 79​ stub_get_session = mock.MagicMock(return_value=stub_session)
70 monkeypatch.setattr(utils, '_get_koji_session', stub_get_session) 80​ monkeypatch.setattr(utils, '_get_koji_session', stub_get_session)
71 81​
82​ monkeypatch.setattr(threading, 'Thread', ThreadStub)
83​
72 def _create_msg(self, ref_instance, ref_new, ref_name, ref_release, ref_version, ref_owner): 84​ def _create_msg(self, ref_instance, ref_new, ref_name, ref_release, ref_version, ref_owner):
73 msg = Munch(body='{"i": 1,\ 85​ msg = Munch(body='{"i": 1,\
74 "msg": {"build_id": 221146,\ 86​ "msg": {"build_id": 221146,\
75 "owner": "%s",\ 87​ "owner": "%s",\
76 "instance": "%s",\ 88​ "instance": "%s",\
77 "new": %d,\ 89​ "new": %d,\
78 "name": "%s",\ 90​ "name": "%s",\
79 "release": "%s",\ 91​ "release": "%s",\
▲ Show 20 Lines • Show All 110 Lines • Show Last 20 Lines