From 484bf0181a38bd95edc504165b09c81e96c37755 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Aug 08 2018 15:22:59 +0000
Subject: [PATCH 1/3] create parent dir for symlink if needed


---

diff --git a/hub/kojihub.py b/hub/kojihub.py
index c8d588f..c17f724 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -4966,6 +4966,9 @@ def ensure_volume_symlink(binfo):
         os.unlink(basedir)
     elif os.path.exists(basedir):
         raise koji.GenericError('Unexpected build content: %s', basedir)
+    else:
+        # parent dir might not exist
+        koji.ensuredir(os.path.dirname(basedir))
     os.symlink(relpath, basedir)
 
 

From d9c44735ccb9014ffe9c3552084045bd1548d8dc Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Aug 08 2018 15:22:59 +0000
Subject: [PATCH 2/3] actually add unit test


---

diff --git a/tests/test_hub/test_ensure_volume_symlink.py b/tests/test_hub/test_ensure_volume_symlink.py
new file mode 100644
index 0000000..9473656
--- /dev/null
+++ b/tests/test_hub/test_ensure_volume_symlink.py
@@ -0,0 +1,110 @@
+from __future__ import absolute_import
+import mock
+import os
+import os.path
+import shutil
+import tempfile
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+import koji
+import kojihub
+
+
+class TestEnsureVolumeSymlink(unittest.TestCase):
+
+    def setUp(self):
+        self.tempdir = tempfile.mkdtemp()
+        self.pathinfo = koji.PathInfo(self.tempdir)
+        mock.patch('koji.pathinfo', new=self.pathinfo).start()
+        mock.patch('kojihub.lookup_name', new=self.my_lookup_name).start()
+        self.check_volume_policy = mock.patch('kojihub.check_volume_policy',
+                return_value={'id':0, 'name': 'DEFAULT'}).start()
+        self.buildinfo = {
+                'id': 137,
+                'task_id': 'TASK_ID',
+                'name': 'some-image',
+                'version': '1.2.3.4',
+                'release': '3',
+                'epoch': None,
+                'source': None,
+                'state': koji.BUILD_STATES['BUILDING'],
+                'volume_id': 0,
+                'volume_name': 'DEFAULT',
+                }
+
+    def tearDown(self):
+        mock.patch.stopall()
+        shutil.rmtree(self.tempdir)
+
+    def my_lookup_name(self, table, info, **kw):
+        if table != 'volume':
+            raise Exception("Cannot fake call")
+        return {
+                'id': 'VOLUMEID:%s' % info,
+                'name': '%s' % info,
+                }
+
+    def test_volume_symlink_no_action(self):
+        kojihub.ensure_volume_symlink(self.buildinfo)
+        if os.listdir(self.tempdir):
+            raise Exception('call created unexpected files')
+
+        del self.buildinfo['volume_name']
+        with mock.patch('kojihub.logger') as logger:
+            kojihub.ensure_volume_symlink(self.buildinfo)
+            logger.warn.assert_called_once()
+
+    def test_volume_symlink_create(self):
+        self.buildinfo['volume_name'] = 'test'
+        self.buildinfo['volume_id'] = 1
+        kojihub.ensure_volume_symlink(self.buildinfo)
+        # TODO: check files
+
+    def test_volume_symlink_exists(self):
+        basedir = self.pathinfo.build(self.buildinfo)  # default volume
+        relpath = 'some/other/link'
+        os.makedirs(os.path.dirname(basedir))
+        os.symlink(relpath, basedir)
+        self.buildinfo['volume_name'] = 'test'
+        self.buildinfo['volume_id'] = 1
+        kojihub.ensure_volume_symlink(self.buildinfo)
+        # TODO: check files
+
+    def test_volume_symlink_exists_same(self):
+        basedir = self.pathinfo.build(self.buildinfo)  # default volume
+        relpath = ('../../../vol/test/packages/'
+                '%(name)s/%(version)s/%(release)s' % self.buildinfo)
+        os.makedirs(os.path.dirname(basedir))
+        os.symlink(relpath, basedir)
+        self.buildinfo['volume_name'] = 'test'
+        self.buildinfo['volume_id'] = 1
+        with mock.patch('os.unlink') as unlink:
+            kojihub.ensure_volume_symlink(self.buildinfo)
+            unlink.assert_not_called()
+        # TODO: check files
+
+    def test_volume_symlink_exists_error(self):
+        basedir = self.pathinfo.build(self.buildinfo)  # default volume
+        # create default volume dir, should trigger an error
+        os.makedirs(basedir)
+        self.buildinfo['volume_name'] = 'test'
+        self.buildinfo['volume_id'] = 1
+        with self.assertRaises(koji.GenericError):
+            kojihub.ensure_volume_symlink(self.buildinfo)
+        files = list(find_files(self.tempdir))
+        expected = [
+                'packages',
+                'packages/some-image',
+                'packages/some-image/1.2.3.4',
+                'packages/some-image/1.2.3.4/3',
+                ]
+        self.assertEqual(files, expected)
+
+
+def find_files(dirpath):
+    '''Find all files under dir, report relative paths'''
+    for path, dirs, files in os.walk(dirpath):
+        for fn in sorted(dirs + files):
+            yield os.path.relpath(os.path.join(path, fn), dirpath)

From 549d9d7955c15875c3eb65d54e70718c32d36c76 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Aug 08 2018 15:22:59 +0000
Subject: [PATCH 3/3] more checks in unit test


---

diff --git a/tests/test_hub/test_ensure_volume_symlink.py b/tests/test_hub/test_ensure_volume_symlink.py
index 9473656..e0acdf5 100644
--- a/tests/test_hub/test_ensure_volume_symlink.py
+++ b/tests/test_hub/test_ensure_volume_symlink.py
@@ -57,20 +57,34 @@ class TestEnsureVolumeSymlink(unittest.TestCase):
             logger.warn.assert_called_once()
 
     def test_volume_symlink_create(self):
+        basedir = self.pathinfo.build(self.buildinfo)  # default volume
         self.buildinfo['volume_name'] = 'test'
         self.buildinfo['volume_id'] = 1
         kojihub.ensure_volume_symlink(self.buildinfo)
-        # TODO: check files
+        files = list(find_files(self.tempdir))
+        expected = [
+                'packages',
+                'packages/some-image',
+                'packages/some-image/1.2.3.4',
+                'packages/some-image/1.2.3.4/3',
+                ]
+        self.assertEqual(files, expected)
+        relpath = ('../../../vol/test/packages/'
+                '%(name)s/%(version)s/%(release)s' % self.buildinfo)
+        self.assertEqual(os.readlink(basedir), relpath)
+
 
     def test_volume_symlink_exists(self):
         basedir = self.pathinfo.build(self.buildinfo)  # default volume
-        relpath = 'some/other/link'
+        oldpath = 'some/other/link'
         os.makedirs(os.path.dirname(basedir))
-        os.symlink(relpath, basedir)
+        os.symlink(oldpath, basedir)
         self.buildinfo['volume_name'] = 'test'
         self.buildinfo['volume_id'] = 1
         kojihub.ensure_volume_symlink(self.buildinfo)
-        # TODO: check files
+        relpath = ('../../../vol/test/packages/'
+                '%(name)s/%(version)s/%(release)s' % self.buildinfo)
+        self.assertEqual(os.readlink(basedir), relpath)
 
     def test_volume_symlink_exists_same(self):
         basedir = self.pathinfo.build(self.buildinfo)  # default volume
@@ -83,7 +97,14 @@ class TestEnsureVolumeSymlink(unittest.TestCase):
         with mock.patch('os.unlink') as unlink:
             kojihub.ensure_volume_symlink(self.buildinfo)
             unlink.assert_not_called()
-        # TODO: check files
+        files = list(find_files(self.tempdir))
+        expected = [
+                'packages',
+                'packages/some-image',
+                'packages/some-image/1.2.3.4',
+                'packages/some-image/1.2.3.4/3',
+                ]
+        self.assertEqual(files, expected)
 
     def test_volume_symlink_exists_error(self):
         basedir = self.pathinfo.build(self.buildinfo)  # default volume

