Add cleanup_repos to revert build_only overrides

Repos marked build_only: true are backed up before handle_repos()
overwrites them and restored at the end of each image build, so CI
mirror URLs are not baked into the final image.

The cleanup RUN was injected into every Dockerfile, not just images
that called handle_repos() for those repos. Child images with no
backup fell through to "rm -f", deleting the inherited repo file
(e.g. debian.sources in bifrost-base, breaking cpio install).

Fix by adding a {repo}.enabled sentinel that handle_repos creates
when it processes a build_only repo, making the cleanup conditional
on that sentinel.

Change-Id: Ie5b9970a4575b6258dc238059d3f33ddfe155789
Signed-off-by: Michal Nasiadka <mnasiadka@gmail.com>

1 files changed, 373 insertions, 0 deletions

diff --git a/kolla/tests/test_methods.py b/kolla/tests/test_methods.py
index 8a577aead..1e30f1f9b 100644
--- a/kolla/tests/test_methods.py
+++ b/kolla/tests/test_methods.py
@@ -10,6 +10,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 import tempfile
 
 import yaml
@@ -398,6 +399,378 @@ class MethodsTest(base.TestCase):
             self.assertRaises(ValueError, methods.handle_repos,
                               template_vars, ['test-repo'], 'enable')
 
+    def test_handle_repos_deb_build_only_backup(self):
+        repos = {'debian': {'build-only-mirror': {
+            'build_only': True,
+            'url': 'http://mirror.example.com/debian',
+            'suite': 'trixie',
+            'component': 'main',
+            'gpg_key': '/usr/share/keyrings/debian-archive-keyring.gpg',
+        }}}
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(repos, f)
+            fname = f.name
+        try:
+            template_vars = {
+                'base_arch': 'x86_64',
+                'base_distro': 'debian',
+                'base_package_type': 'deb',
+                'openstack_release_codename': 'Gazpacho',
+                'repos_yaml': fname,
+            }
+            result = methods.handle_repos(
+                template_vars, ['build-only-mirror'], 'enable')
+            self.assertIn('mkdir -p /tmp/kolla-repos-backup', result)
+            self.assertIn(
+                'cp /etc/apt/sources.list.d/build-only-mirror.sources'
+                ' /tmp/kolla-repos-backup/build-only-mirror.sources',
+                result)
+            self.assertIn(
+                'touch /tmp/kolla-repos-backup/build-only-mirror.enabled',
+                result)
+            # backup must precede the actual repo write
+            self.assertLess(
+                result.index('cp /etc/apt/sources.list.d/build-only-mirror'),
+                result.index("echo 'Uris:"))
+        finally:
+            os.unlink(fname)
+
+    def test_handle_repos_rpm_build_only_backup(self):
+        repos = {'rpm': {'build-only-mirror': {
+            'build_only': True,
+            'name': 'build-only-mirror',
+            'baseurl': 'http://mirror.example.com/rpm/',
+            'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-Rocky-10',
+        }}}
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(repos, f)
+            fname = f.name
+        try:
+            template_vars = {
+                'base_arch': 'x86_64',
+                'base_distro': 'centos',
+                'base_package_type': 'rpm',
+                'repos_yaml': fname,
+            }
+            result = methods.handle_repos(
+                template_vars, ['build-only-mirror'], 'enable')
+            self.assertIn('mkdir -p /tmp/kolla-repos-backup', result)
+            self.assertIn(
+                'cp /etc/yum.repos.d/build-only-mirror.repo'
+                ' /tmp/kolla-repos-backup/build-only-mirror.repo',
+                result)
+            self.assertIn(
+                'touch /tmp/kolla-repos-backup/build-only-mirror.repo'
+                '.enabled',
+                result)
+            # backup must precede DNF_REMOVE_EXISTING
+            self.assertLess(
+                result.index(
+                    'cp /etc/yum.repos.d/build-only-mirror.repo'),
+                result.index('grep -rlF'))
+        finally:
+            os.unlink(fname)
+
+    def test_handle_repos_rpm_shared_file_group_backup(self):
+        """Repos sharing a file_group back up that file once, by its name.
+
+        e.g. Rocky's baseos/appstream/crb all live in rocky.repo.
+        """
+        repos = {'rpm': {
+            'shared-a': {
+                'build_only': True,
+                'file_group': 'shared.repo',
+                'name': 'shared-a',
+                'baseurl': 'http://mirror.example.com/a/',
+                'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-a',
+            },
+            'shared-b': {
+                'build_only': True,
+                'file_group': 'shared.repo',
+                'name': 'shared-b',
+                'baseurl': 'http://mirror.example.com/b/',
+                'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-b',
+            },
+        }}
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(repos, f)
+            fname = f.name
+        try:
+            template_vars = {
+                'base_arch': 'x86_64',
+                'base_distro': 'centos',
+                'base_package_type': 'rpm',
+                'repos_yaml': fname,
+            }
+            result = methods.handle_repos(
+                template_vars, ['shared-a', 'shared-b'], 'enable')
+            self.assertEqual(
+                1,
+                result.count(
+                    'cp /etc/yum.repos.d/shared.repo'
+                    ' /tmp/kolla-repos-backup/shared.repo'))
+            self.assertNotIn('shared-a.repo /tmp/kolla-repos-backup',
+                             result)
+            self.assertNotIn('shared-b.repo /tmp/kolla-repos-backup',
+                             result)
+            # each repo still gets its own override file written
+            self.assertIn('>/etc/yum.repos.d/shared-a.repo', result)
+            self.assertIn('>/etc/yum.repos.d/shared-b.repo', result)
+        finally:
+            os.unlink(fname)
+
+    def test_get_cleanup_commands_rpm_shared_file_group(self):
+        """The shared file is restored once, each own override always."""
+        repos = {'rpm': {
+            'shared-a': {
+                'build_only': True,
+                'file_group': 'shared.repo',
+                'name': 'shared-a',
+                'baseurl': 'http://mirror.example.com/a/',
+                'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-a',
+            },
+            'shared-b': {
+                'build_only': True,
+                'file_group': 'shared.repo',
+                'name': 'shared-b',
+                'baseurl': 'http://mirror.example.com/b/',
+                'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-b',
+            },
+        }}
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(repos, f)
+            fname = f.name
+        try:
+            result = methods.get_cleanup_commands(
+                fname, 'rpm', 'centos', 'x86_64')
+            self.assertEqual(
+                1,
+                result.count(
+                    'mv /tmp/kolla-repos-backup/shared.repo'
+                    ' /etc/yum.repos.d/shared.repo'))
+            self.assertIn('rm -f /etc/yum.repos.d/shared-a.repo', result)
+            self.assertIn('rm -f /etc/yum.repos.d/shared-b.repo', result)
+        finally:
+            os.unlink(fname)
+
+    def _rocky_mirror_override(self):
+        """The exact override shape documented for Rocky, minus file_group.
+
+        repos_yaml overrides replace a repo's whole entry, so this matches
+        what a real user would write per doc/source/admin/image-building
+        .rst - it must not restate 'file_group' for the fix to be tested.
+        """
+        return {'rocky': {
+            'baseos': {
+                'build_only': True,
+                'name': 'baseos',
+                'baseurl': 'http://mirror.example.com/rocky/10/BaseOS/',
+                'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-Rocky-10',
+            },
+            'appstream': {
+                'build_only': True,
+                'name': 'appstream',
+                'baseurl': 'http://mirror.example.com/rocky/10/AppStream/',
+                'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-Rocky-10',
+            },
+            'crb': {
+                'build_only': True,
+                'name': 'crb',
+                'baseurl': 'http://mirror.example.com/rocky/10/CRB/',
+                'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-Rocky-10',
+            },
+        }}
+
+    def test_handle_repos_rocky_baseos_backs_up_shared_file(self):
+        """file_group must be resolved even if the override omits it.
+
+        Rocky's baseos/appstream/crb live in rocky.repo, not their own
+        .repo files, and a repos_yaml override doesn't restate file_group.
+        """
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(self._rocky_mirror_override(), f)
+            fname = f.name
+        try:
+            template_vars = {
+                'base_arch': 'x86_64',
+                'base_distro': 'rocky',
+                'base_package_type': 'rpm',
+                'repos_yaml': fname,
+            }
+            result = methods.handle_repos(
+                template_vars, ['baseos', 'appstream'], 'enable')
+            self.assertEqual(
+                1,
+                result.count(
+                    'cp /etc/yum.repos.d/rocky.repo'
+                    ' /tmp/kolla-repos-backup/rocky.repo'))
+            self.assertNotIn('baseos.repo /tmp/kolla-repos-backup', result)
+            self.assertNotIn('appstream.repo /tmp/kolla-repos-backup',
+                             result)
+        finally:
+            os.unlink(fname)
+
+    def test_get_cleanup_commands_rocky_restores_shared_file(self):
+        """Restoring baseos/appstream/crb recovers the shared rocky.repo."""
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(self._rocky_mirror_override(), f)
+            fname = f.name
+        try:
+            result = methods.get_cleanup_commands(
+                fname, 'rpm', 'rocky', 'x86_64')
+            self.assertEqual(
+                1,
+                result.count(
+                    'mv /tmp/kolla-repos-backup/rocky.repo'
+                    ' /etc/yum.repos.d/rocky.repo'))
+            self.assertIn('rm -f /etc/yum.repos.d/baseos.repo', result)
+            self.assertIn('rm -f /etc/yum.repos.d/appstream.repo', result)
+            self.assertIn('rm -f /etc/yum.repos.d/crb.repo', result)
+        finally:
+            os.unlink(fname)
+
+    def test_handle_repos_build_only_no_double_backup(self):
+        """Each distinct file is backed up exactly once per handle_repos."""
+        repos = {'debian': {
+            'mirror-a': {
+                'build_only': True,
+                'url': 'http://mirror.example.com/a',
+                'suite': 'trixie',
+                'component': 'main',
+                'gpg_key': '/usr/share/keyrings/debian-archive-keyring.gpg',
+            },
+            'mirror-b': {
+                'build_only': True,
+                'url': 'http://mirror.example.com/b',
+                'suite': 'trixie',
+                'component': 'main',
+                'gpg_key': '/usr/share/keyrings/debian-archive-keyring.gpg',
+            },
+        }}
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(repos, f)
+            fname = f.name
+        try:
+            template_vars = {
+                'base_arch': 'x86_64',
+                'base_distro': 'debian',
+                'base_package_type': 'deb',
+                'openstack_release_codename': 'Gazpacho',
+                'repos_yaml': fname,
+            }
+            result = methods.handle_repos(
+                template_vars, ['mirror-a', 'mirror-b'], 'enable')
+            self.assertEqual(
+                1,
+                result.count(
+                    'cp /etc/apt/sources.list.d/mirror-a.sources'
+                    ' /tmp/kolla-repos-backup/mirror-a.sources'))
+            self.assertEqual(
+                1,
+                result.count(
+                    'cp /etc/apt/sources.list.d/mirror-b.sources'
+                    ' /tmp/kolla-repos-backup/mirror-b.sources'))
+        finally:
+            os.unlink(fname)
+
+    def test_handle_repos_without_build_only_no_backup(self):
+        template_vars = {
+            'base_arch': 'x86_64',
+            'base_distro': 'debian',
+            'base_package_type': 'deb',
+            'openstack_release_codename': 'Gazpacho',
+        }
+        result = methods.handle_repos(template_vars, ['grafana'], 'enable')
+        self.assertNotIn('kolla-repos-backup', result)
+
+    def test_get_cleanup_commands_deb(self):
+        repos = {'debian': {'build-only-mirror': {
+            'build_only': True,
+            'url': 'http://mirror.example.com/debian',
+            'suite': 'trixie',
+            'component': 'main',
+            'gpg_key': '/usr/share/keyrings/debian-archive-keyring.gpg',
+        }}}
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(repos, f)
+            fname = f.name
+        try:
+            result = methods.get_cleanup_commands(
+                fname, 'deb', 'debian', 'x86_64')
+            self.assertTrue(result.startswith('RUN '))
+            self.assertIn(
+                '[ -f /tmp/kolla-repos-backup/build-only-mirror.enabled ]',
+                result)
+            self.assertIn(
+                'mv /tmp/kolla-repos-backup/build-only-mirror.sources'
+                ' /etc/apt/sources.list.d/build-only-mirror.sources',
+                result)
+            self.assertIn(
+                'rm -f /etc/apt/sources.list.d/build-only-mirror.sources',
+                result)
+            self.assertIn('rm -rf /tmp/kolla-repos-backup', result)
+        finally:
+            os.unlink(fname)
+
+    def test_get_cleanup_commands_rpm(self):
+        repos = {'rpm': {'build-only-mirror': {
+            'build_only': True,
+            'name': 'build-only-mirror',
+            'baseurl': 'http://mirror.example.com/',
+            'gpgkey': 'file:///etc/pki/rpm-gpg/RPM-GPG-KEY-Rocky-10',
+        }}}
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(repos, f)
+            fname = f.name
+        try:
+            result = methods.get_cleanup_commands(
+                fname, 'rpm', 'centos', 'x86_64')
+            self.assertTrue(result.startswith('RUN '))
+            self.assertIn(
+                '[ -f /tmp/kolla-repos-backup/build-only-mirror.repo'
+                '.enabled ]',
+                result)
+            self.assertIn(
+                'mv /tmp/kolla-repos-backup/build-only-mirror.repo'
+                ' /etc/yum.repos.d/build-only-mirror.repo',
+                result)
+            self.assertIn(
+                'rm -f /etc/yum.repos.d/build-only-mirror.repo',
+                result)
+            self.assertIn('rm -rf /tmp/kolla-repos-backup', result)
+        finally:
+            os.unlink(fname)
+
+    def test_get_cleanup_commands_empty_without_build_only(self):
+        result = methods.get_cleanup_commands(None, 'deb', 'debian', 'x86_64')
+        self.assertEqual('', result)
+
+    def test_get_cleanup_commands_skips_distro_repos(self):
+        """distro: True repos are a no-op in handle_repos."""
+        repos = {'debian': {'debian': {
+            'build_only': True,
+            'distro': True,
+        }}}
+        with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml',
+                                         delete=False) as f:
+            yaml.dump(repos, f)
+            fname = f.name
+        try:
+            result = methods.get_cleanup_commands(
+                fname, 'deb', 'debian', 'x86_64')
+            self.assertEqual('', result)
+        finally:
+            os.unlink(fname)
+
     def test_repos_yaml_rpm_section_override_not_blocked_by_distro_section(
             self):
         """Overriding distro repos in 'rpm' section must not be undone.