#828 ostree/utils: Drop timestamps from generated repo names - continuation
Closed 6 years ago by lsedlar. Opened 6 years ago by onosek.

file modified
+20 -23
@@ -14,7 +14,6 @@ 

  # along with this program; if not, see <https://gnu.org/licenses/>.

  

  

- import datetime

  import json

  import logging

  import os
@@ -66,19 +65,6 @@ 

      return commitid

  

  

- def _write_repofile(path, name, repo):

-     """Write a .repo file with given data."""

-     with open(path, 'w') as f:

-         f.write("[%s]\n" % name)

-         f.write("name=%s\n" % name)

-         f.write("baseurl=%s\n" % repo['baseurl'])

-         exclude = repo.get('exclude', None)

-         if exclude:

-             f.write("exclude=%s\n" % exclude)

-         gpgcheck = '1' if repo.get('gpgcheck', False) else '0'

-         f.write("gpgcheck=%s\n" % gpgcheck)

- 

- 

  def tweak_treeconf(treeconf, source_repos=None, keep_original_sources=False, update_dict=None):

      """

      Update tree config file by adding new repos, and remove existing repos
@@ -86,23 +72,34 @@ 

      Additionally, other values can be passed to method by 'update_dict' parameter to

      update treefile content.

      """

-     # add this timestamp to repo name to get unique repo filename and repo name

-     # should be safe enough

-     time = datetime.datetime.now().strftime("%Y%m%d%H%M%S")

- 

      treeconf_dir = os.path.dirname(treeconf)

      with open(treeconf, 'r') as f:

          treeconf_content = json.load(f)

  

      # backup the old tree config

-     os.rename(treeconf, '%s.%s.bak' % (treeconf, time))

+     os.rename(treeconf, '{}.bak'.format(treeconf))

  

      repos = []

      if source_repos:

-         for repo in source_repos:

-             name = "%s-%s" % (repo['name'], time)

-             _write_repofile("%s/%s.repo" % (treeconf_dir, name), name, repo)

-             repos.append(name)

+         # Sort to ensure reliable ordering

+         source_repos = sorted(source_repos, key=lambda x: x['name'])

+         # Now, since pungi includes timestamps in the repo names which

+         # currently defeats rpm-ostree's change detection, let's just

+         # use repos named 'repo-<number>'.

+         # https://pagure.io/pungi/issue/811

+         with open("{}/pungi.repo".format(treeconf_dir), 'w') as f:

+             for i, repo in enumerate(source_repos):

+                 name = 'repo-{}'.format(i)

+                 f.write("[%s]\n" % name)

+                 f.write("name=%s\n" % name)

+                 f.write("baseurl=%s\n" % repo['baseurl'])

+                 exclude = repo.get('exclude', None)

+                 if exclude:

+                     f.write("exclude=%s\n" % exclude)

+                 gpgcheck = '1' if repo.get('gpgcheck', False) else '0'

+                 f.write("gpgcheck=%s\n" % gpgcheck)

+ 

+                 repos.append(name)

  

      original_repos = treeconf_content.get('repos', [])

      if keep_original_sources:

file modified
+25 -44
@@ -7,7 +7,6 @@ 

  import os

  import json

  import sys

- import datetime

  

  sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))

  sys.path.insert(0, os.path.join(os.path.dirname(__file__), 'bin'))
@@ -187,11 +186,8 @@ 

                          self.topdir + '/fedora-atomic-docker-host.json'],

                         logfile=self.topdir + '/logs/Atomic/create-ostree-repo.log', show_cmd=True, stdout=True)])

  

-     @mock.patch('pungi.ostree.utils.datetime')

      @mock.patch('kobo.shortcuts.run')

-     def test_extra_config_with_extra_repos(self, run, time):

-         time.datetime.now.return_value = datetime.datetime(2016, 1, 1, 1, 1)

-         timestamp = time.datetime.now().strftime("%Y%m%d%H%M%S")

+     def test_extra_config_with_extra_repos(self, run):

  

          configdir = os.path.join(self.topdir, 'config')

          self._make_dummy_config_dir(configdir)
@@ -228,47 +224,36 @@ 

              '--extra-config=%s' % extra_config_file,

          ])

  

-         server_repo_name = "server-%s" % timestamp

-         server_repo = os.path.join(configdir, "%s.repo" % server_repo_name)

-         self.assertTrue(os.path.isfile(server_repo))

-         with open(server_repo, 'r') as f:

-             content = f.read()

-             self.assertIn("[%s]" % server_repo_name, content)

-             self.assertIn("name=%s" % server_repo_name, content)

-             self.assertIn("baseurl=http://www.example.com/Server/repo", content)

-             self.assertIn("gpgcheck=0", content)

- 

-         optional_repo_name = "optional-%s" % timestamp

-         optional_repo = os.path.join(configdir, "%s.repo" % optional_repo_name)

-         self.assertTrue(os.path.isfile(optional_repo))

-         with open(optional_repo, 'r') as f:

-             content = f.read()

-             self.assertIn("[%s]" % optional_repo_name, content)

-             self.assertIn("name=%s" % optional_repo_name, content)

-             self.assertIn("baseurl=http://example.com/repo/x86_64/optional", content)

-             self.assertIn("gpgcheck=0", content)

- 

-         extra_repo_name = "extra-%s" % timestamp

-         extra_repo = os.path.join(configdir, "%s.repo" % extra_repo_name)

-         self.assertTrue(os.path.isfile(extra_repo))

-         with open(extra_repo, 'r') as f:

-             content = f.read()

-             self.assertIn("[%s]" % extra_repo_name, content)

-             self.assertIn("name=%s" % extra_repo_name, content)

-             self.assertIn("baseurl=http://example.com/repo/x86_64/extra", content)

-             self.assertIn("gpgcheck=0", content)

+         pungi_repo = os.path.join(configdir, "pungi.repo")

+         self.assertTrue(os.path.isfile(pungi_repo))

+         with open(pungi_repo, 'r') as f:

+             content = f.read().strip()

+             result_template = (

+                 "[repo-0]",

+                 "name=repo-0",

+                 "baseurl=http://example.com/repo/x86_64/extra",

+                 "gpgcheck=0",

+                 "[repo-1]",

+                 "name=repo-1",

+                 "baseurl=http://example.com/repo/x86_64/optional",

+                 "exclude=systemd-container",

+                 "gpgcheck=0",

+                 "[repo-2]",

+                 "name=repo-2",

+                 "baseurl=http://www.example.com/Server/repo",

+                 "gpgcheck=0",

+             )

+             result = '\n'.join(result_template).strip()

+             self.assertEqual(content, result)

  

          treeconf = json.load(open(treefile, 'r'))

          repos = treeconf['repos']

          self.assertEqual(len(repos), 3)

-         for name in [server_repo_name, optional_repo_name, extra_repo_name]:

+         for name in ("repo-0", "repo-1", "repo-2"):

              self.assertIn(name, repos)

  

-     @mock.patch('pungi.ostree.utils.datetime')

      @mock.patch('kobo.shortcuts.run')

-     def test_extra_config_with_keep_original_sources(self, run, time):

-         time.datetime.now.return_value = datetime.datetime(2016, 1, 1, 1, 1)

-         timestamp = time.datetime.now().strftime("%Y%m%d%H%M%S")

+     def test_extra_config_with_keep_original_sources(self, run):

  

          configdir = os.path.join(self.topdir, 'config')

          self._make_dummy_config_dir(configdir)
@@ -306,15 +291,11 @@ 

              '--extra-config=%s' % extra_config_file,

          ])

  

-         server_repo_name = "server-%s" % timestamp

-         optional_repo_name = "optional-%s" % timestamp

-         extra_repo_name = "extra-%s" % timestamp

- 

          treeconf = json.load(open(treefile, 'r'))

          repos = treeconf['repos']

          self.assertEqual(len(repos), 6)

          for name in ['fedora-rawhide', 'fedora-24', 'fedora-23',

-                      server_repo_name, optional_repo_name, extra_repo_name]:

+                      'repo-0', 'repo-1', 'repo-2']:

              self.assertIn(name, repos)

  

  

Just to be sure ... should repos names in pungi.repo really be like "name=repo-1" instead of "name=additional"?

The name should not matter. The only important thing is that it has to match what is inserted into the treefile.

Code change looks good to me. I started a test compose in stage with this patch, will report results when it finishes.

Thank you very much for picking up the torch on this!

As far as repo naming, let's circle back to that later; it's not a critical issue now but it will definitely be important later.

rebased onto c41af8ee5df9124628b6ecb4b6a9c3df65624322

6 years ago

rebased onto 75f8d41

6 years ago

The test compose failed due to disk I/O errors, which are unrelated to the change. :broken_heart:
I restarted it.

I finally managed to finish testing this and it works similarly to what happened before. In my test there were changes in packages, so the commit was created, however the files are now named in a way that will be identical every time.

I'll rebase on current master and merge.

Pull-Request has been closed by lsedlar

6 years ago