Allow the repository clone path to be specified in the CLI
As it currently stands, Pegleg clones site repositories into the /tmp directory. Even if the site repository already exists in the /tmp directory it is still cloned there which results in wasted disk space. This commit allows users to pass in a `clone_path` (-p) option to Pegleg CLI commands that specify where to clone a site repository. If the clone path matches the path of an existing repository, then a error message is logged stating so. If the repository already exists in the clone path, the user can either specify to use that local repo by passing it as the site repository or they proceed by passing in a different clone path. This commit also updates the logic that deletes the copy of the repo that is created in the temporary folder to also delete the parent folder that contains the copied repo. This scenario happens when using a local repository as the site repository. Addionally, this commit adds a cleanup fixture that removes files and directories created in the temporary folder by the unit tests. Change-Id: I1b2943493b8f201f337ea60006c009973dd941b3
This commit is contained in:
Rick Bartra
parent
c20d714c61
commit
e3d37db45e
|
|
@ -103,6 +103,20 @@ The revision can also be specified via (for example):
|
|||
|
||||
-r /opt/aic-site-clcp-manifests@revision
|
||||
|
||||
**-p / --clone-path** (Optional).
|
||||
|
||||
The path where the repo will be cloned. By default the repo will be cloned to
|
||||
the /tmp path. If this option is included and the repo already exists, then the
|
||||
repo will not be cloned again and the user must specify a new clone path or
|
||||
pass in the local copy of the repository as the site repository. Suppose the
|
||||
repo name is airship-treasuremap and the clone path is /tmp/mypath then the
|
||||
following directory is created /tmp/mypath/airship-treasuremap which will
|
||||
contain the contents of the repo. Example of using clone path:
|
||||
|
||||
::
|
||||
|
||||
-p /tmp/mypath
|
||||
|
||||
.. _cli-repo-lint:
|
||||
|
||||
Lint
|
||||
|
|
@ -148,6 +162,20 @@ These should be named per the site-definition file, e.g.:
|
|||
|
||||
-e global=/opt/global -e secrets=/opt/secrets
|
||||
|
||||
**-p / --clone-path** (Optional).
|
||||
|
||||
The path where the repo will be cloned. By default the repo will be cloned to
|
||||
the /tmp path. If this option is included and the repo already exists, then the
|
||||
repo will not be cloned again and the user must specify a new clone path or
|
||||
pass in the local copy of the repository as the site repository. Suppose the
|
||||
repo name is airship-treasuremap and the clone path is /tmp/mypath then the
|
||||
following directory is created /tmp/mypath/airship-treasuremap which will
|
||||
contain the contents of the repo. Example of using clone path:
|
||||
|
||||
::
|
||||
|
||||
-p /tmp/mypath
|
||||
|
||||
Repository Overrides
|
||||
^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
|
|
|
|||
|
|
@ -65,6 +65,20 @@ REPOSITORY_USERNAME_OPTION = click.option(
|
|||
'specified in the site-definition file. Any occurrences of REPO_USERNAME '
|
||||
'will be replaced with this value.')
|
||||
|
||||
REPOSITORY_CLONE_PATH_OPTION = click.option(
|
||||
'-p',
|
||||
'--clone-path',
|
||||
'clone_path',
|
||||
help=
|
||||
'The path where the repo will be cloned. By default the repo will be '
|
||||
'cloned to the /tmp path. If this option is included and the repo already '
|
||||
'exists, then the repo will not be cloned again and the user must specify '
|
||||
'a new clone path or pass in the local copy of the repository as the site '
|
||||
'repository. Suppose the repo name is airship-treasuremap and the clone '
|
||||
'path is /tmp/mypath then the following directory is created '
|
||||
'/tmp/mypath/airship-treasuremap which will contain the contents of the '
|
||||
'repo')
|
||||
|
||||
ALLOW_MISSING_SUBSTITUTIONS_OPTION = click.option(
|
||||
'-f',
|
||||
'--fail-on-missing-sub-src',
|
||||
|
|
@ -116,11 +130,12 @@ def main(*, verbose):
|
|||
|
||||
@main.group(help='Commands related to repositories')
|
||||
@MAIN_REPOSITORY_OPTION
|
||||
@REPOSITORY_CLONE_PATH_OPTION
|
||||
# TODO(felipemonteiro): Support EXTRA_REPOSITORY_OPTION as well to be
|
||||
# able to lint multiple repos together.
|
||||
@REPOSITORY_USERNAME_OPTION
|
||||
@REPOSITORY_KEY_OPTION
|
||||
def repo(*, site_repository, repo_key, repo_username):
|
||||
def repo(*, site_repository, clone_path, repo_key, repo_username):
|
||||
"""Group for repo-level actions, which include:
|
||||
|
||||
* lint: lint all sites across the repository
|
||||
|
|
@ -128,6 +143,7 @@ def repo(*, site_repository, repo_key, repo_username):
|
|||
"""
|
||||
|
||||
config.set_site_repo(site_repository)
|
||||
config.set_clone_path(clone_path)
|
||||
config.set_repo_key(repo_key)
|
||||
config.set_repo_username(repo_username)
|
||||
|
||||
|
|
@ -168,10 +184,12 @@ def lint_repo(*, fail_on_missing_sub_src, exclude_lint, warn_lint):
|
|||
|
||||
@main.group(help='Commands related to sites')
|
||||
@MAIN_REPOSITORY_OPTION
|
||||
@REPOSITORY_CLONE_PATH_OPTION
|
||||
@EXTRA_REPOSITORY_OPTION
|
||||
@REPOSITORY_USERNAME_OPTION
|
||||
@REPOSITORY_KEY_OPTION
|
||||
def site(*, site_repository, extra_repositories, repo_key, repo_username):
|
||||
def site(*, site_repository, clone_path, extra_repositories,
|
||||
repo_key, repo_username):
|
||||
"""Group for site-level actions, which include:
|
||||
|
||||
* list: list available sites in a manifests repo
|
||||
|
|
@ -182,6 +200,7 @@ def site(*, site_repository, extra_repositories, repo_key, repo_username):
|
|||
"""
|
||||
|
||||
config.set_site_repo(site_repository)
|
||||
config.set_clone_path(clone_path)
|
||||
config.set_extra_repo_store(extra_repositories or [])
|
||||
config.set_repo_key(repo_key)
|
||||
config.set_repo_username(repo_username)
|
||||
|
|
@ -301,16 +320,19 @@ def lint_site(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name):
|
|||
|
||||
@main.group(help='Commands related to types')
|
||||
@MAIN_REPOSITORY_OPTION
|
||||
@REPOSITORY_CLONE_PATH_OPTION
|
||||
@EXTRA_REPOSITORY_OPTION
|
||||
@REPOSITORY_USERNAME_OPTION
|
||||
@REPOSITORY_KEY_OPTION
|
||||
def type(*, site_repository, extra_repositories, repo_key, repo_username):
|
||||
def type(*, site_repository, clone_path, extra_repositories,
|
||||
repo_key, repo_username):
|
||||
"""Group for repo-level actions, which include:
|
||||
|
||||
* list: list all types across the repository
|
||||
|
||||
"""
|
||||
config.set_site_repo(site_repository)
|
||||
config.set_clone_path(clone_path)
|
||||
config.set_extra_repo_store(extra_repositories or [])
|
||||
config.set_repo_key(repo_key)
|
||||
config.set_repo_username(repo_username)
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ except NameError:
|
|||
GLOBAL_CONTEXT = {
|
||||
'site_repo': './',
|
||||
'extra_repos': [],
|
||||
'clone_path': None,
|
||||
'site_path': 'site',
|
||||
'type_path': 'type'
|
||||
}
|
||||
|
|
@ -32,6 +33,14 @@ def set_site_repo(r):
|
|||
GLOBAL_CONTEXT['site_repo'] = r.rstrip('/') + '/'
|
||||
|
||||
|
||||
def get_clone_path():
|
||||
return GLOBAL_CONTEXT['clone_path']
|
||||
|
||||
|
||||
def set_clone_path(p):
|
||||
GLOBAL_CONTEXT['clone_path'] = p
|
||||
|
||||
|
||||
def get_extra_repo_store():
|
||||
return GLOBAL_CONTEXT.get('extra_repo_store', [])
|
||||
|
||||
|
|
|
|||
|
|
@ -152,7 +152,9 @@ def _process_repository(repo_url_or_path, repo_revision):
|
|||
|
||||
if os.path.exists(repo_url_or_path):
|
||||
repo_name = util.git.repo_name(repo_url_or_path)
|
||||
new_temp_path = os.path.join(tempfile.mkdtemp(), repo_name)
|
||||
parent_temp_path = tempfile.mkdtemp()
|
||||
__REPO_FOLDERS.setdefault(repo_name, parent_temp_path)
|
||||
new_temp_path = os.path.join(parent_temp_path, repo_name)
|
||||
norm_path, sub_path = util.git.normalize_repo_path(repo_url_or_path)
|
||||
shutil.copytree(src=norm_path, dst=new_temp_path, symlinks=True)
|
||||
__REPO_FOLDERS.setdefault(repo_name, new_temp_path)
|
||||
|
|
@ -169,7 +171,7 @@ def _process_site_repository(repo_url_or_path, repo_revision):
|
|||
|
||||
Also validate that the provided ``repo_url_or_path`` is a valid Git
|
||||
repository. If ``repo_url_or_path`` doesn't already exist, clone it.
|
||||
If it does, extra the appropriate revision and check it out.
|
||||
If it does, extract the appropriate revision and check it out.
|
||||
|
||||
:param repo_url_or_path: Repo path or URL and associated auth information.
|
||||
If URL, examples include:
|
||||
|
|
@ -306,9 +308,12 @@ def _handle_repository(repo_url_or_path, *args, **kwargs):
|
|||
checkout specified reference .
|
||||
|
||||
"""
|
||||
# Retrieve the clone path where the repo will be cloned
|
||||
clone_path = config.get_clone_path()
|
||||
|
||||
try:
|
||||
return util.git.git_handler(repo_url_or_path, *args, **kwargs)
|
||||
return util.git.git_handler(repo_url_or_path, clone_path=clone_path,
|
||||
*args, **kwargs)
|
||||
except exceptions.GitException as e:
|
||||
raise click.ClickException(e)
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -29,7 +29,8 @@ LOG = logging.getLogger(__name__)
|
|||
__all__ = ('git_handler', )
|
||||
|
||||
|
||||
def git_handler(repo_url, ref=None, proxy_server=None, auth_key=None):
|
||||
def git_handler(repo_url, ref=None, proxy_server=None,
|
||||
auth_key=None, clone_path=None):
|
||||
"""Handle directories that are Git repositories.
|
||||
|
||||
If ``repo_url`` is a valid URL for which a local repository doesn't
|
||||
|
|
@ -50,6 +51,8 @@ def git_handler(repo_url, ref=None, proxy_server=None, auth_key=None):
|
|||
:param proxy_server: optional, HTTP proxy to use while cloning the repo.
|
||||
:param auth_key: If supplied results in using SSH to clone the repository
|
||||
with the specified key. If the value is None, SSH is not used.
|
||||
:param clone_path: The path where the repo will be cloned. By default the
|
||||
repo will be cloned to the /tmp path.
|
||||
:returns: Path to the cloned repo if a repo was cloned, else absolute
|
||||
path to ``repo_url``.
|
||||
:raises ValueError: If ``repo_url`` isn't a valid URL or doesn't begin
|
||||
|
|
@ -72,7 +75,8 @@ def git_handler(repo_url, ref=None, proxy_server=None, auth_key=None):
|
|||
# we need to clone the repo_url first since it doesn't exist and then
|
||||
# checkout the appropriate reference - and return the tmpdir
|
||||
if parsed_url.scheme in supported_clone_protocols:
|
||||
return _try_git_clone(repo_url, ref, proxy_server, auth_key)
|
||||
return _try_git_clone(repo_url, ref, proxy_server,
|
||||
auth_key, clone_path)
|
||||
else:
|
||||
raise ValueError('repo_url=%s must use one of the following '
|
||||
'protocols: %s' %
|
||||
|
|
@ -138,7 +142,8 @@ def _get_current_ref(repo_url):
|
|||
return None
|
||||
|
||||
|
||||
def _try_git_clone(repo_url, ref=None, proxy_server=None, auth_key=None):
|
||||
def _try_git_clone(repo_url, ref=None, proxy_server=None,
|
||||
auth_key=None, clone_path=None):
|
||||
"""Try cloning Git repo from ``repo_url`` using the reference ``ref``.
|
||||
|
||||
:param repo_url: URL of remote Git repo or path to local Git repo.
|
||||
|
|
@ -146,6 +151,8 @@ def _try_git_clone(repo_url, ref=None, proxy_server=None, auth_key=None):
|
|||
:param proxy_server: optional, HTTP proxy to use while cloning the repo.
|
||||
:param auth_key: If supplied results in using SSH to clone the repository
|
||||
with the specified key. If the value is None, SSH is not used.
|
||||
:param clone_path: The path where the repo will be cloned. By default the
|
||||
repo will be cloned to the /tmp path.
|
||||
:returns: Path to the cloned repo.
|
||||
:rtype: str
|
||||
:raises GitException: If ``repo_url`` is invalid or could not be found.
|
||||
|
|
@ -155,12 +162,20 @@ def _try_git_clone(repo_url, ref=None, proxy_server=None, auth_key=None):
|
|||
|
||||
"""
|
||||
|
||||
if clone_path is None:
|
||||
clone_path = tempfile.mkdtemp()
|
||||
# the name here is important as it bubbles back up to the output filename
|
||||
# and ensure we handle url/foo.git/ cases. prefix is 'tmp' by default.
|
||||
root_temp_dir = tempfile.mkdtemp()
|
||||
repo_name = repo_url.rstrip('/').split('/')[-1]
|
||||
temp_dir = os.path.join(root_temp_dir, repo_name)
|
||||
os.makedirs(temp_dir)
|
||||
temp_dir = os.path.join(clone_path, repo_name)
|
||||
try:
|
||||
os.makedirs(temp_dir)
|
||||
except FileExistsError:
|
||||
msg = "The repository already exists in the given path. Either " \
|
||||
"provide a new clone path or pass in the path of the local " \
|
||||
"repository as the site repository (-r)."
|
||||
LOG.error(msg)
|
||||
raise
|
||||
|
||||
env_vars = _get_clone_env_vars(repo_url, ref, auth_key)
|
||||
ssh_cmd = env_vars.get('GIT_SSH_COMMAND')
|
||||
|
|
|
|||
|
|
@ -14,7 +14,10 @@
|
|||
|
||||
import copy
|
||||
|
||||
import os
|
||||
import pytest
|
||||
import shutil
|
||||
import tempfile
|
||||
|
||||
from pegleg import config
|
||||
"""Fixtures that are applied to all unit tests."""
|
||||
|
|
@ -30,3 +33,25 @@ def restore_config():
|
|||
yield
|
||||
finally:
|
||||
config.GLOBAL_CONTEXT = original_global_context
|
||||
|
||||
|
||||
@pytest.fixture(scope="class", autouse=True)
|
||||
def clean_temporary_git_repos():
|
||||
"""Iterates through all temporarily created directories and deletes each
|
||||
one that was created for testing.
|
||||
|
||||
"""
|
||||
|
||||
def temporary_git_repos():
|
||||
root_tempdir = tempfile.gettempdir()
|
||||
tempdirs = os.listdir(root_tempdir)
|
||||
for tempdir in tempdirs:
|
||||
path = os.path.join(root_tempdir, tempdir)
|
||||
if os.path.isdir(path) and os.access(path, os.R_OK):
|
||||
if any(p.startswith('airship') for p in os.listdir(path)):
|
||||
yield path
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
for tempdir in temporary_git_repos():
|
||||
shutil.rmtree(tempdir, ignore_errors=True)
|
||||
|
|
|
|||
|
|
@ -26,20 +26,6 @@ from pegleg.engine.util import git
|
|||
from tests.unit import test_utils
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clean_git_repos():
|
||||
"""Iterates through all temporarily created directories and deletes each
|
||||
one that was created for testing.
|
||||
|
||||
"""
|
||||
|
||||
root_tempdir = tempfile.gettempdir()
|
||||
for tempdir in os.listdir(root_tempdir):
|
||||
path = os.path.join(root_tempdir, tempdir)
|
||||
if git.is_repository(path):
|
||||
shutil.rmtree(path, ignore_errors=True)
|
||||
|
||||
|
||||
def _validate_git_clone(repo_dir, fetched_ref=None, checked_out_ref=None):
|
||||
"""Validate that git clone/checkout work.
|
||||
|
||||
|
|
@ -121,7 +107,7 @@ def test_git_clone_behind_proxy(mock_log):
|
|||
url = 'https://github.com/openstack/airship-armada'
|
||||
commit = 'cba78d1d03e4910f6ab1691bae633c5bddce893d'
|
||||
|
||||
for proxy_server in _PROXY_SERVERS.values():
|
||||
for proxy_server in test_utils._PROXY_SERVERS.values():
|
||||
git_dir = git.git_handler(url, commit, proxy_server=proxy_server)
|
||||
_validate_git_clone(git_dir, commit)
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@
|
|||
from __future__ import absolute_import
|
||||
import copy
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
|
||||
import pytest
|
||||
|
|
@ -154,3 +155,13 @@ schema: pegleg/SiteDefinition/v1
|
|||
files._create_tree(cicd_path, tree=test_structure)
|
||||
|
||||
yield
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def temp_clone_path():
|
||||
temp_folder = tempfile.mkdtemp()
|
||||
try:
|
||||
yield temp_folder
|
||||
finally:
|
||||
if os.path.exists(temp_folder):
|
||||
shutil.rmtree(temp_folder, ignore_errors=True)
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ from pegleg import cli
|
|||
from pegleg.engine import errorcodes
|
||||
from pegleg.engine.util import git
|
||||
from tests.unit import test_utils
|
||||
|
||||
from tests.unit.fixtures import temp_clone_path
|
||||
|
||||
@pytest.mark.skipif(
|
||||
not test_utils.is_connected(),
|
||||
|
|
@ -52,6 +52,97 @@ class BaseCLIActionTest(object):
|
|||
repo_url = "https://github.com/openstack/%s.git" % cls.repo_name
|
||||
cls.treasuremap_path = git.git_handler(repo_url, ref=cls.repo_rev)
|
||||
|
||||
|
||||
class TestSiteCLIOptions(BaseCLIActionTest):
|
||||
"""Tests site-level CLI options."""
|
||||
|
||||
### clone_path tests ###
|
||||
|
||||
def test_list_sites_using_remote_repo_and_clone_path_option(
|
||||
self,
|
||||
temp_clone_path):
|
||||
"""Validates clone_path (-p) option is working properly with site list
|
||||
action when using remote repo. Verify that the repo was cloned in the
|
||||
clone_path
|
||||
"""
|
||||
# Scenario:
|
||||
#
|
||||
# 1) List sites (should clone repo automatically to `clone_path`
|
||||
# location if `clone_path` is set)
|
||||
|
||||
repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name,
|
||||
self.repo_rev)
|
||||
|
||||
# Note that the -p option is used to specify the clone_folder
|
||||
site_list = self.runner.invoke(cli.site, ['-p', temp_clone_path,
|
||||
'-r', repo_url, 'list'])
|
||||
|
||||
assert site_list.exit_code == 0
|
||||
# Verify that the repo was cloned into the clone_path
|
||||
assert os.path.exists(os.path.join(temp_clone_path,
|
||||
self.repo_name))
|
||||
assert git.is_repository(os.path.join(temp_clone_path,
|
||||
self.repo_name))
|
||||
|
||||
def test_list_sites_using_local_repo_and_clone_path_option(self,
|
||||
temp_clone_path):
|
||||
"""Validates clone_path (-p) option is working properly with site list
|
||||
action when using a local repo. Verify that the clone_path has NO
|
||||
effect when using a local repo
|
||||
"""
|
||||
# Scenario:
|
||||
#
|
||||
# 1) List sites (when using local repo there should be not cloning
|
||||
# even if the clone_path is passed in)
|
||||
|
||||
repo_path = self.treasuremap_path
|
||||
|
||||
# Note that the -p option is used to specify the clone_folder
|
||||
site_list = self.runner.invoke(cli.site, ['-p', temp_clone_path,
|
||||
'-r', repo_path, 'list'])
|
||||
|
||||
assert site_list.exit_code == 0
|
||||
# Verify that passing in clone_path when using local repo has no effect
|
||||
assert not os.path.exists(os.path.join(temp_clone_path, self.repo_name))
|
||||
|
||||
|
||||
class TestSiteCLIOptionsNegative(BaseCLIActionTest):
|
||||
"""Negative Tests for site-level CLI options."""
|
||||
|
||||
### Negative clone_path tests ###
|
||||
|
||||
def test_list_sites_using_remote_repo_and_reuse_clone_path_option(self,
|
||||
temp_clone_path):
|
||||
"""Validates clone_path (-p) option is working properly with site list
|
||||
action when using remote repo. Verify that the same repo can't be
|
||||
cloned in the same clone_path if it already exists
|
||||
"""
|
||||
# Scenario:
|
||||
#
|
||||
# 1) List sites (should clone repo automatically to `clone_path`
|
||||
# location if `clone_path` is set)
|
||||
|
||||
repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name,
|
||||
self.repo_rev)
|
||||
|
||||
# Note that the -p option is used to specify the clone_folder
|
||||
site_list = self.runner.invoke(cli.site, ['-p', temp_clone_path,
|
||||
'-r', repo_url, 'list'])
|
||||
|
||||
assert git.is_repository(os.path.join(temp_clone_path,
|
||||
self.repo_name))
|
||||
|
||||
# Run site list for a second time to validate that the repo can't be
|
||||
# cloned twice in the same clone_path
|
||||
site_list = self.runner.invoke(cli.site, ['-p', temp_clone_path,
|
||||
'-r', repo_url, 'list'])
|
||||
|
||||
assert site_list.exit_code == 1
|
||||
msg = "The repository already exists in the given path. Either " \
|
||||
"provide a new clone path or pass in the path of the local " \
|
||||
"repository as the site repository (-r)."
|
||||
assert msg in site_list.output
|
||||
|
||||
@classmethod
|
||||
def teardown_class(cls):
|
||||
# Cleanup temporary Git repos.
|
||||
|
|
|
|||
Loading…
Reference in New Issue