From: Eric Ball Date: Tue, 3 Mar 2020 17:18:39 +0000 (-0800) Subject: Revert "Change the copy_archives method to use..." X-Git-Tag: v0.31.1^0 X-Git-Url: https://gerrit.linuxfoundation.org/infra/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F66%2F63266%2F1;p=releng%2Flftools.git Revert "Change the copy_archives method to use..." This change is causing logs to not be copied correctly. See IT-19109, IT-19131. This reverts commit 3fad9329a6de8384446fb63215311d29e8aab363. Change-Id: I7b134d2ac5f8c3b93808c1b863f3f01df0cc0a95 Signed-off-by: Eric Ball --- diff --git a/lftools/deploy.py b/lftools/deploy.py index a7bcbe56..6537e13f 100755 --- a/lftools/deploy.py +++ b/lftools/deploy.py @@ -50,14 +50,9 @@ def _compress_text(dir): paths.extend(glob.glob(search, recursive=True)) for _file in paths: - # glob may follow symlink paths that open can't find - if os.path.exists(_file): - log.debug("Compressing file {}".format(_file)) - with open(_file, 'rb') as src, gzip.open('{}.gz'.format(_file), 'wb') as dest: - shutil.copyfileobj(src, dest) - os.remove(_file) - else: - log.info("Could not open path from glob {}".format(_file)) + with open(_file, 'rb') as src, gzip.open('{}.gz'.format(_file), 'wb') as dest: + shutil.copyfileobj(src, dest) + os.remove(_file) os.chdir(save_dir) @@ -222,7 +217,7 @@ def copy_archives(workspace, pattern=None): :arg str pattern: Space-separated list of Unix style glob patterns. (default: None) """ - archives_dir = os.path.join(workspace, "archives") + archives_dir = os.path.join(workspace, 'archives') dest_dir = os.getcwd() log.debug('Copying files from {} with pattern \'{}\' to {}.'.format( @@ -238,14 +233,11 @@ def copy_archives(workspace, pattern=None): for file_or_dir in os.listdir(archives_dir): f = os.path.join(archives_dir, file_or_dir) try: - log.debug('Copying {}'.format(f)) - shutil.copy2(f, dest_dir, follow_symlinks=False) - except IsADirectoryError: - os.makedirs(os.path.join(dest_dir, file_or_dir), - exist_ok=True) + log.debug('Moving {}'.format(f)) + shutil.move(f, dest_dir) except shutil.Error as e: log.error(e) - raise OSError(errno.EPERM, 'Could not copy to', archives_dir) + raise OSError(errno.EPERM, 'Could not move to', archives_dir) else: log.error('Archives dir {} does not exist.'.format(archives_dir)) raise OSError(errno.ENOENT, 'Missing directory', archives_dir) @@ -270,7 +262,6 @@ def copy_archives(workspace, pattern=None): if len(os.path.basename(src)) > 255: log.warn('Filename {} is over 255 characters. Skipping...'.format( os.path.basename(src))) - continue dest = os.path.join(dest_dir, src[len(workspace)+1:]) log.debug('{} -> {}'.format(src, dest)) @@ -278,9 +269,9 @@ def copy_archives(workspace, pattern=None): if os.path.isfile(src): try: shutil.move(src, dest) - except FileNotFoundError as e: + except IOError as e: # Switch to FileNotFoundError when Python 2 support is dropped. log.debug("Missing path, will create it {}.\n{}".format(os.path.dirname(dest), e)) - os.makedirs(os.path.dirname(dest), exist_ok=True) + os.makedirs(os.path.dirname(dest)) shutil.move(src, dest) else: log.info('Not copying directories: {}.'.format(src)) diff --git a/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml b/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml deleted file mode 100644 index ae78b110..00000000 --- a/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml +++ /dev/null @@ -1,8 +0,0 @@ ---- -fixes: - - | - Change the copy_archives method used by deploy_archives to use copy2 with - follow_symlinks set to False, which will copy symlinks as symlinks, rather - than following their path. Since glob does not have an option to not follow - symlinks, we instead just check for file existence to ignore issues with - globbing symlinked paths that "open" fails to find. diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/child/parent b/tests/fixtures/deploy/workspace-symlinks/archives/child/parent deleted file mode 120000 index a96aa0ea..00000000 --- a/tests/fixtures/deploy/workspace-symlinks/archives/child/parent +++ /dev/null @@ -1 +0,0 @@ -.. \ No newline at end of file diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file b/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file deleted file mode 120000 index ef706654..00000000 --- a/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file +++ /dev/null @@ -1 +0,0 @@ -non-existent-file \ No newline at end of file diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/test.log b/tests/fixtures/deploy/workspace-symlinks/archives/test.log deleted file mode 100644 index b3589bf7..00000000 --- a/tests/fixtures/deploy/workspace-symlinks/archives/test.log +++ /dev/null @@ -1 +0,0 @@ -This is a test log. diff --git a/tests/test_deploy.py b/tests/test_deploy.py index e33ac12e..77d0a1cc 100644 --- a/tests/test_deploy.py +++ b/tests/test_deploy.py @@ -11,7 +11,6 @@ import os import sys -import tempfile import pytest import requests @@ -75,26 +74,6 @@ def test_copy_archive_dir(cli_runner, datafiles): assert os.path.exists(os.path.join(stage_dir, 'test.log')) -@pytest.mark.datafiles( - os.path.join(FIXTURE_DIR, 'deploy'), - ) -def test_copy_archive_dir_symlinks(cli_runner, datafiles): - """Test copy_archives() command with symlink loop and a broken symlink, in - order to test that these conditions are handled properly.""" - os.chdir(str(datafiles)) - workspace_dir = os.path.join(str(datafiles), 'workspace-symlinks') - stage_dir = str(datafiles.mkdir("stage_archive")) - - os.chdir(stage_dir) - result = cli_runner.invoke( - cli.cli, - ['--debug', 'deploy', 'copy-archives', workspace_dir], - obj={}) - assert result.exit_code == 0 - - assert os.path.exists(os.path.join(stage_dir, 'test.log')) - - @pytest.mark.datafiles( os.path.join(FIXTURE_DIR, 'deploy'), ) @@ -153,7 +132,7 @@ def test_deploy_archive(cli_runner, datafiles, responses): ) def test_deploy_archive2(datafiles): """Test deploy_archives() command when archives dir is missing.""" - os.chdir(tempfile.mkdtemp(prefix='lftools-test.')) + os.chdir(str(datafiles)) workspace_dir = os.path.join(str(datafiles), 'workspace-noarchives') with pytest.raises(OSError) as excinfo: @@ -166,7 +145,7 @@ def test_deploy_archive2(datafiles): ) def test_deploy_archive3(datafiles): """Test deploy_archives() command when archives dir is a file instead of a dir.""" - os.chdir(tempfile.mkdtemp(prefix='lftools-test.')) + os.chdir(str(datafiles)) workspace_dir = os.path.join(str(datafiles), 'workspace-archivesfile') with pytest.raises(OSError) as excinfo: @@ -179,7 +158,7 @@ def test_deploy_archive3(datafiles): ) def test_deploy_archive4(cli_runner, datafiles, responses): """Test deploy_archives() command when using duplicated patterns.""" - os.chdir(tempfile.mkdtemp(prefix='lftools-test.')) + os.chdir(str(datafiles)) workspace_dir = os.path.join(str(datafiles), 'workspace-patternfile') pattern=["**/*.log", "**/hs_err_*.log", "**/target/**/feature.xml", "**/target/failsafe-reports/failsafe-summary.xml", "**/target/surefire-reports/*-output.txt", "**/target/surefire-reports/*-output.txt", "**/target/failsafe-reports/failsafe-summary.xml", "**/*"] result = deploy_sys.copy_archives(workspace_dir, pattern)