fm-orchestrator

Clone
Source Code
GIT

#835 Add an incrementing prefix to the dist_hash.
Merged 7 years ago by mprahl. Opened 7 years ago by ralph.
increment  into  master

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

      dist_str = '.'.join([module_build.name, module_build.stream, str(module_build.version),
 

                           str(module_build.context)])
 

      dist_hash = hashlib.sha1(dist_str).hexdigest()[:8]
 

-     return conf.default_dist_tag_prefix + dist_hash
 

+     return "{prefix}{index}+{dist_hash}".format(
 

+         prefix=conf.default_dist_tag_prefix,
 

+         index=module_build.id or 0,
 

+         dist_hash=dist_hash,
 

+     )
 

  

  

  def create_dogpile_key_generator_func(skip_first_n_args=0):

file modified
+18 -11 
@@ -58,6 +58,11 @@ 
 

  

  def init_data():
 

      clean_database()
 

+     with make_session(conf) as session:
 

+         _populate_data(session)
 

+ 

+ 

+ def _populate_data(session):
 

      for index in range(10):
 

          build_one = ModuleBuild()
 

          build_one.name = 'nginx'
 
@@ -79,6 +84,8 @@ 
 

          build_one.time_completed = \
 

              datetime(2016, 9, 3, 11, 25, 32) + timedelta(minutes=(index * 10))
 

          build_one.rebuild_strategy = 'changed-and-after'
 

+         session.add(build_one)
 

+         session.commit()
 

          build_one_component_release = get_rpm_release(build_one)
 

  

          component_one_build_one = ComponentBuild()
 
@@ -128,6 +135,8 @@ 
 

          build_two.time_completed = \
 

              datetime(2016, 9, 3, 11, 27, 19) + timedelta(minutes=(index * 10))
 

          build_two.rebuild_strategy = 'changed-and-after'
 

+         session.add(build_two)
 

+         session.commit()
 

          build_two_component_release = get_rpm_release(build_two)
 

  

          component_one_build_two = ComponentBuild()
 
@@ -176,6 +185,8 @@ 
 

              datetime(2016, 9, 3, 12, 28, 40) + timedelta(minutes=(index * 10))
 

          build_three.time_completed = None
 

          build_three.rebuild_strategy = 'changed-and-after'
 

+         session.add(build_three)
 

+         session.commit()
 

          build_three_component_release = get_rpm_release(build_three)
 

  

          component_one_build_three = ComponentBuild()
 
@@ -206,17 +217,13 @@ 
 

          component_two_build_three.tagged = True
 

          component_two_build_three.build_time_only = True
 

  

-         with make_session(conf) as session:
 

-             session.add(build_one)
 

-             session.add(component_one_build_one)
 

-             session.add(component_two_build_one)
 

-             session.add(component_one_build_two)
 

-             session.add(component_two_build_two)
 

-             session.add(component_one_build_three)
 

-             session.add(component_two_build_three)
 

-             session.add(build_two)
 

-             session.add(build_three)
 

-             session.commit()
 

+         session.add(component_one_build_one)
 

+         session.add(component_two_build_one)
 

+         session.add(component_one_build_two)
 

+         session.add(component_two_build_two)
 

+         session.add(component_one_build_three)
 

+         session.add(component_two_build_three)
 

+         session.commit()
 

  

  

  def scheduler_init_data(communicator_state=None):

file modified
+12 -12 
@@ -780,9 +780,9 @@ 
 

          # Check that components are tagged after the batch is built.
 

          tag_groups = []
 

          tag_groups.append(set(
 

-             ['perl-Tangerine-0.23-1.module+814cfa39',
 

-              'perl-List-Compare-0.53-5.module+814cfa39',
 

-              'tangerine-0.22-3.module+814cfa39']))
 

+             ['perl-Tangerine-0.23-1.module+0+814cfa39',
 

+              'perl-List-Compare-0.53-5.module+0+814cfa39',
 

+              'tangerine-0.22-3.module+0+814cfa39']))
 

  

          def on_tag_artifacts_cb(cls, artifacts, dest_tag=True):
 

              if dest_tag is True:
 
@@ -791,9 +791,9 @@ 
 

  

          buildtag_groups = []
 

          buildtag_groups.append(set(
 

-             ['perl-Tangerine-0.23-1.module+814cfa39',
 

-              'perl-List-Compare-0.53-5.module+814cfa39',
 

-              'tangerine-0.22-3.module+814cfa39']))
 

+             ['perl-Tangerine-0.23-1.module+0+814cfa39',
 

+              'perl-List-Compare-0.53-5.module+0+814cfa39',
 

+              'tangerine-0.22-3.module+0+814cfa39']))
 

  

          def on_buildroot_add_artifacts_cb(cls, artifacts, install):
 

              self.assertEqual(buildtag_groups.pop(0), set(artifacts))
 
@@ -840,9 +840,9 @@ 
 

          # Check that components are tagged after the batch is built.
 

          tag_groups = []
 

          tag_groups.append(set(
 

-             ['perl-Tangerine-0.23-1.module+814cfa39',
 

-              'perl-List-Compare-0.53-5.module+814cfa39',
 

-              'tangerine-0.22-3.module+814cfa39']))
 

+             ['perl-Tangerine-0.23-1.module+0+814cfa39',
 

+              'perl-List-Compare-0.53-5.module+0+814cfa39',
 

+              'tangerine-0.22-3.module+0+814cfa39']))
 

  

          def on_tag_artifacts_cb(cls, artifacts, dest_tag=True):
 

              if dest_tag is True:
 
@@ -851,9 +851,9 @@ 
 

  

          buildtag_groups = []
 

          buildtag_groups.append(set(
 

-             ['perl-Tangerine-0.23-1.module+814cfa39',
 

-              'perl-List-Compare-0.53-5.module+814cfa39',
 

-              'tangerine-0.22-3.module+814cfa39']))
 

+             ['perl-Tangerine-0.23-1.module+0+814cfa39',
 

+              'perl-List-Compare-0.53-5.module+0+814cfa39',
 

+              'tangerine-0.22-3.module+0+814cfa39']))
 

  

          def on_buildroot_add_artifacts_cb(cls, artifacts, install):
 

              self.assertEqual(buildtag_groups.pop(0), set(artifacts))

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

  

          builder.module_tag = {"name": "module-foo", "id": 1}
 

          builder.module_build_tag = {"name": "module-foo-build", "id": 2}
 

-         dist_tag = 'module+b8661ee4'
 

+         dist_tag = 'module+1+b8661ee4'
 

          # Set listTagged to return test data
 

          builder.koji_session.listTagged.side_effect = [[], [], []]
 

          untagged = [{

file modified
+4 -4 
@@ -385,10 +385,10 @@ 
 

          self.assertEqual(module_build_two.state, models.BUILD_STATES['failed'])
 

          # Make sure the builds were untagged
 

          builder.untag_artifacts.assert_called_once_with([
 

-             'perl-Tangerine-0.23-1.module+814cfa39',
 

-             'perl-List-Compare-0.53-5.module+814cfa39',
 

-             'tangerine-0.22-3.module+814cfa39',
 

-             'module-build-macros-0.1-1.module+814cfa39'
 

+             'perl-Tangerine-0.23-1.module+0+814cfa39',
 

+             'perl-List-Compare-0.53-5.module+0+814cfa39',
 

+             'tangerine-0.22-3.module+0+814cfa39',
 

+             'module-build-macros-0.1-1.module+0+814cfa39'
 

          ])
 

  

      def test_cleanup_stale_failed_builds_no_components(self, create_builder, koji_get_session,

file modified
+1 
@@ -80,6 +80,7 @@ 
 

  

  

  class TestUtils(unittest.TestCase):
 

+     maxDiff = None
 

  

      def setUp(self):
 

          self.filtered_rpms = [

file modified
+10 -10 
@@ -142,13 +142,13 @@ 
 

                      'task_id': 12312321,
 

                      'state': 1,
 

                      'state_reason': None,
 

-                     'nvr': 'module-build-macros-01-1.module+b8661ee4',
 

+                     'nvr': 'module-build-macros-01-1.module+1+b8661ee4',
 

                  },
 

                  'nginx': {
 

                      'task_id': 12312345,
 

                      'state': 1,
 

                      'state_reason': None,
 

-                     'nvr': 'nginx-1.10.1-2.module+b8661ee4',
 

+                     'nvr': 'nginx-1.10.1-2.module+1+b8661ee4',
 

                  },
 

              },
 

          })
 
@@ -199,13 +199,13 @@ 
 

                      'task_id': 12312321,
 

                      'state': 1,
 

                      'state_reason': None,
 

-                     'nvr': 'module-build-macros-01-1.module+b8661ee4',
 

+                     'nvr': 'module-build-macros-01-1.module+1+b8661ee4',
 

                  },
 

                  'nginx': {
 

                      'task_id': 12312345,
 

                      'state': 1,
 

                      'state_reason': None,
 

-                     'nvr': 'nginx-1.10.1-2.module+b8661ee4',
 

+                     'nvr': 'nginx-1.10.1-2.module+1+b8661ee4',
 

                  },
 

              },
 

          })
 
@@ -262,13 +262,13 @@ 
 

                  'tasks': {
 

                      'rpms': {
 

                          'module-build-macros': {
 

-                             'nvr': 'module-build-macros-01-1.module+8d3cee59',
 

+                             'nvr': 'module-build-macros-01-1.module+30+8d3cee59',
 

                              'state': 1,
 

                              'state_reason': None,
 

                              'task_id': 47384002
 

                          },
 

                          'rubygem-rails': {
 

-                             'nvr': 'postgresql-9.5.3-4.module+8d3cee59',
 

+                             'nvr': 'postgresql-9.5.3-4.module+30+8d3cee59',
 

                              'state': 3,
 

                              'state_reason': None,
 

                              'task_id': 2433442
 
@@ -296,13 +296,13 @@ 
 

                  'tasks': {
 

                      'rpms': {
 

                          'module-build-macros': {
 

-                             'nvr': 'module-build-macros-01-1.module+0557c87d',
 

+                             'nvr': 'module-build-macros-01-1.module+29+0557c87d',
 

                              'state': 1,
 

                              'state_reason': None,
 

                              'task_id': 47384002
 

                          },
 

                          'postgresql': {
 

-                             'nvr': 'postgresql-9.5.3-4.module+0557c87d',
 

+                             'nvr': 'postgresql-9.5.3-4.module+29+0557c87d',
 

                              'state': 1,
 

                              'state_reason': None,
 

                              'task_id': 2433442
 
@@ -391,9 +391,9 @@ 
 

  

      def test_query_component_builds_filter_nvr(self):
 

          rv = self.client.get('/module-build-service/1/component-builds/?nvr=nginx-1.10.1-2.'
 

-                              'module%2Bb8661ee4')
 

+                              'module%2B1%2Bb8661ee4')
 

          data = json.loads(rv.data)
 

-         self.assertEquals(data['meta']['total'], 10)
 

+         self.assertEquals(data['meta']['total'], 1)
 

  

      def test_query_component_builds_filter_task_id(self):
 

          rv = self.client.get('/module-build-service/1/component-builds/?task_id=12312346')

For #823.

Can we merge this after #833? It touches a lot of the same code.

Yes, certainly!

One thought - this PR currently only changes the release field of the NVR for modular rpms, but not for modules themselves (in koji, via the content-generator). We should probably go general and include this increasing value in the module NVR as well. (noticed this while reviewing #833).

@ralph can you rebase this please?

What do you think about using the last character of the prefix instead of using a "+" to separate the index and dist_hash? This would be more aesthetically pleasing as one deployment uses "_" as the last character in the prefix and the other deployment of MBS uses "+".

We should consider filtering out module builds with the same NSV in this query because with module stream expansion, we will likely want those module builds to have the same index.

When you rebase, this function's parameters will have changed to just accepting a models.ModuleBuild object.

rebased onto 9593f14039c55cd95da418b6459b832b9c2d4a3c
7 years ago

rebased onto 01c8295
7 years ago

OK - this is ready for another review, @mprahl.

@ralph in the unit tests all the prefixes are coming up as 0, but the IDs of those module builds are not 0. Is get_rpm_release being called in MBS before the module build is committed to the database?

Edit:
It should be solved by committing the module builds before running lines like these:
https://pagure.io/fm-orchestrator/blob/master/f/tests/__init__.py#_82
Edited 7 years ago by mprahl

1 new commit added

  • Address modules in the test suite with id=0.
7 years ago

@mprahl - second commit should address that. Thanks!

Pull-Request has been merged by mprahl
7 years ago
Metadata
None
No Tags
Changes Summary 7
+5 -1
file changed
module_build_service/utils.py
+18 -11
file changed
tests/__init__.py
+12 -12
file changed
tests/test_build/test_build.py
+1 -1
file changed
tests/test_builder/test_koji.py
+4 -4
file changed
tests/test_scheduler/test_poller.py
+1 -0
file changed
tests/test_utils/test_utils.py
+10 -10
file changed
tests/test_views/test_views.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.