diff --git a/buildstockbatch/eagle.py b/buildstockbatch/eagle.py index 2a90cd5f..7b682420 100644 --- a/buildstockbatch/eagle.py +++ b/buildstockbatch/eagle.py @@ -58,8 +58,7 @@ def get_bool_env_var(varname): class EagleBatch(BuildStockBatchBase): CONTAINER_RUNTIME = ContainerRuntime.SINGULARITY - - sys_image_dir = '/shared-projects/buildstock/singularity_images' + DEFAULT_SYS_IMAGE_DIR = '/shared-projects/buildstock/singularity_images' hpc_name = 'eagle' min_sims_per_job = 36 * 2 @@ -79,11 +78,15 @@ def __init__(self, project_filename): logger.debug('Output directory = {}'.format(output_dir)) weather_dir = self.weather_dir # noqa E841 + self.singularity_image = self.get_singularity_image(self.cfg, self.os_version, self.os_sha) + + @classmethod def validate_project(cls, project_file): super(cls, cls).validate_project(project_file) # Eagle specific validation goes here cls.validate_output_directory_eagle(project_file) + cls.validate_singularity_image_eagle(project_file) logger.info("Eagle Validation Successful") return True @@ -95,6 +98,19 @@ def validate_output_directory_eagle(cls, project_file): raise ValidationError(f"`output_directory` must be in /scratch or /projects," f" `output_directory` = {output_dir}") + @classmethod + def validate_singularity_image_eagle(cls, project_file): + cfg = get_project_configuration(project_file) + singularity_image = cls.get_singularity_image( + cfg, + cfg.get('os_version', cls.DEFAULT_OS_VERSION), + cfg.get('os_sha', cls.DEFAULT_OS_SHA) + ) + if not os.path.exists(singularity_image): + raise ValidationError( + f"The singularity image does not exist: {singularity_image}" + ) + @property def output_dir(self): output_dir = path_rel_to_file(self.project_filename, self.cfg['output_directory']) @@ -112,53 +128,15 @@ def clear_and_copy_dir(src, dst): shutil.rmtree(dst, ignore_errors=True) shutil.copytree(src, dst) - @property - def singularity_image_url(self): - if '-' in self.os_version: - prefix_ver = self.os_version.split('-')[0] - else: - prefix_ver = self.os_version - return 'https://s3.amazonaws.com/openstudio-builds/{prefix_ver}/OpenStudio-{ver}.{sha}-Singularity.simg'.format( - prefix_ver=prefix_ver, - ver=self.os_version, - sha=self.os_sha - ) - - @property - def singularity_image(self): - # Check the project yaml specification - if the file does not exist do not silently allow for non-specified simg - if 'sys_image_dir' in self.cfg.keys(): - sys_image_dir = self.cfg['sys_image_dir'] - sys_image = os.path.join(sys_image_dir, 'OpenStudio-{ver}.{sha}-Singularity.simg'.format( - ver=self.os_version, - sha=self.os_sha - )) - if os.path.isfile(sys_image): - return sys_image - else: - raise RuntimeError('Unable to find singularity image specified in project file: `{}`'.format(sys_image)) - # Use the expected HPC environment default if not explicitly defined in the YAML - sys_image = os.path.join(self.sys_image_dir, 'OpenStudio-{ver}.{sha}-Singularity.simg'.format( - ver=self.os_version, - sha=self.os_sha - )) - if os.path.isfile(sys_image): - return sys_image - # Otherwise attempt retrieval from AWS for the appropriate os_version and os_sha - else: - singularity_image_path = os.path.join(self.output_dir, 'openstudio.simg') - if not os.path.isfile(singularity_image_path): - logger.debug(f'Downloading singularity image: {self.singularity_image_url}') - r = requests.get(self.singularity_image_url, stream=True) - if r.status_code != requests.codes.ok: - logger.error('Unable to download simg file from OpenStudio releases S3 bucket.') - r.raise_for_status() - with open(singularity_image_path, 'wb') as f: - for chunk in r.iter_content(chunk_size=1024): - if chunk: - f.write(chunk) - logger.debug('Downloaded singularity image to {}'.format(singularity_image_path)) - return singularity_image_path + @classmethod + def get_singularity_image(cls, cfg, os_version, os_sha): + return os.path.join( + cfg.get('sys_image_dir', cls.DEFAULT_SYS_IMAGE_DIR), + 'OpenStudio-{ver}.{sha}-Singularity.simg'.format( + ver=os_version, + sha=os_sha + ) + ) @property def weather_dir(self): diff --git a/buildstockbatch/test/test_eagle.py b/buildstockbatch/test/test_eagle.py index f21c4f25..a138bf94 100644 --- a/buildstockbatch/test/test_eagle.py +++ b/buildstockbatch/test/test_eagle.py @@ -31,7 +31,6 @@ def test_hpc_run_building(mock_subprocess, monkeypatch, basic_residential_projec cfg = get_project_configuration(project_filename) with patch.object(EagleBatch, 'weather_dir', None), \ - patch.object(EagleBatch, 'singularity_image', '/path/to/singularity.simg'), \ patch.object(EagleBatch, 'create_osw', return_value=osw_dict), \ patch.object(EagleBatch, 'make_sim_dir', return_value=('bldg0000001up00', sim_path)), \ patch.object(EagleBatch, 'local_scratch', tmp_path): @@ -97,21 +96,15 @@ def test_hpc_run_building(mock_subprocess, monkeypatch, basic_residential_projec assert called_kw['input'].decode('utf-8').find(' --measures_only') > -1 -def test_singularity_image_download_url(basic_residential_project_file): - project_filename, _ = basic_residential_project_file() - with patch.object(EagleBatch, 'weather_dir', None): - url = EagleBatch(project_filename).singularity_image_url - r = requests.head(url, timeout=30) - assert r.status_code == requests.codes.ok - - @patch('buildstockbatch.base.BuildStockBatchBase.validate_options_lookup') @patch('buildstockbatch.eagle.EagleBatch.validate_output_directory_eagle') +@patch('buildstockbatch.eagle.EagleBatch.validate_singularity_image_eagle') @patch('buildstockbatch.eagle.subprocess') -def test_user_cli(mock_subprocess, mock_validate_output_directory_eagle, mock_validate_options, - basic_residential_project_file, monkeypatch): +def test_user_cli(mock_subprocess, mock_validate_singularity_image_eagle, mock_validate_output_directory_eagle, + mock_validate_options, basic_residential_project_file, monkeypatch): mock_validate_options.return_value = True mock_validate_output_directory_eagle.return_value = True + mock_validate_singularity_image_eagle.return_value = True project_filename, results_dir = basic_residential_project_file() shutil.rmtree(results_dir) @@ -181,8 +174,7 @@ def test_qos_high_job_submit(mock_subprocess, basic_residential_project_file, mo monkeypatch.setenv('CONDA_PREFIX', 'something') monkeypatch.setenv('SLURM_JOB_QOS', 'high') - with patch.object(EagleBatch, 'weather_dir', None), \ - patch.object(EagleBatch, 'singularity_image', '/path/to/singularity.simg'): + with patch.object(EagleBatch, 'weather_dir', None): batch = EagleBatch(project_filename) for i in range(1, 11): pathlib.Path(results_dir, 'job{:03d}.json'.format(i)).touch() @@ -196,8 +188,7 @@ def test_qos_high_job_submit(mock_subprocess, basic_residential_project_file, mo mock_subprocess.run.return_value.stdout = 'Submitted batch job 1\n' mock_subprocess.PIPE = None - with patch.object(EagleBatch, 'weather_dir', None), \ - patch.object(EagleBatch, 'singularity_image', '/path/to/singularity.simg'): + with patch.object(EagleBatch, 'weather_dir', None): batch = EagleBatch(project_filename) batch.queue_post_processing() mock_subprocess.run.assert_called_once() @@ -231,7 +222,6 @@ def sequential_parallel(**kwargs): mocker.patch('buildstockbatch.eagle.subprocess') mocker.patch.object(EagleBatch, 'local_buildstock_dir', results_dir / 'local_buildstock_dir') - mocker.patch.object(EagleBatch, 'singularity_image', '/path/to/singularity.simg') mocker.patch.object(EagleBatch, 'local_weather_dir', results_dir / 'local_weather_dir') mocker.patch.object(EagleBatch, 'local_output_dir', results_dir) mocker.patch.object(EagleBatch, 'local_housing_characteristics_dir', @@ -318,7 +308,6 @@ def sequential_parallel(**kwargs): mocker.patch('buildstockbatch.eagle.Parallel', sequential_parallel) mocker.patch('buildstockbatch.eagle.subprocess') - mocker.patch.object(EagleBatch, 'singularity_image', '/path/to/singularity.simg') mocker.patch.object(EagleBatch, 'run_building', raise_error) mocker.patch.object(EagleBatch, 'local_output_dir', results_dir) mocker.patch.object(EagleBatch, 'results_dir', results_dir) @@ -341,7 +330,6 @@ def test_rerun_failed_jobs(mocker, basic_residential_project_file): project_filename, results_dir = basic_residential_project_file() os.makedirs(os.path.join(results_dir, 'results_csvs')) os.makedirs(os.path.join(results_dir, 'parquet')) - mocker.patch.object(EagleBatch, 'singularity_image', '/path/to/singularity.simg') mocker.patch.object(EagleBatch, 'weather_dir', None) mocker.patch.object(EagleBatch, 'results_dir', results_dir) process_results_mocker = mocker.patch.object(BuildStockBatchBase, 'process_results') diff --git a/buildstockbatch/test/test_validation.py b/buildstockbatch/test/test_validation.py index f924c5a7..cfa48798 100644 --- a/buildstockbatch/test/test_validation.py +++ b/buildstockbatch/test/test_validation.py @@ -102,7 +102,8 @@ def test_validation_integration(project_file, base_expected, eagle_expected): with patch.object(BuildStockBatchBase, 'validate_options_lookup', lambda _: True), \ patch.object(BuildStockBatchBase, 'validate_measure_references', lambda _: True), \ patch.object(BuildStockBatchBase, 'validate_workflow_generator', lambda _: True), \ - patch.object(BuildStockBatchBase, 'validate_postprocessing_spec', lambda _: True): + patch.object(BuildStockBatchBase, 'validate_postprocessing_spec', lambda _: True), \ + patch.object(EagleBatch, 'validate_singularity_image_eagle', lambda _: True): for cls, expected in [(BuildStockBatchBase, base_expected), (EagleBatch, eagle_expected)]: if expected is not True: with pytest.raises(expected): @@ -306,6 +307,19 @@ def test_validate_eagle_output_directory(): EagleBatch.validate_output_directory_eagle(str(temp_yml)) +def test_validate_singularity_image_eagle(mocker, basic_residential_project_file): + minimal_yml = pathlib.Path(example_yml_dir, 'minimal-schema.yml') + with tempfile.TemporaryDirectory() as tmpdir: + with open(minimal_yml, 'r') as f: + cfg = yaml.load(f, Loader=yaml.SafeLoader) + cfg['sys_image_dir'] = tmpdir + temp_yml = pathlib.Path(tmpdir, 'temp.yml') + with open(temp_yml, 'w') as f: + yaml.dump(cfg, f, Dumper=yaml.SafeDumper) + with pytest.raises(ValidationError, match=r"image does not exist"): + EagleBatch.validate_singularity_image_eagle(str(temp_yml)) + + def test_validate_sampler_good_buildstock(basic_residential_project_file): project_filename, _ = basic_residential_project_file({ 'sampler': { diff --git a/docs/changelog/changelog_dev.rst b/docs/changelog/changelog_dev.rst index eca9615d..cc58811b 100644 --- a/docs/changelog/changelog_dev.rst +++ b/docs/changelog/changelog_dev.rst @@ -21,3 +21,11 @@ Development Changelog :tickets: 385 Removing broken postprocessing tests. + + .. change:: + :tags: bugfix + :pullreq: 386 + :tickets: 256 + + No longer automatically downloads the appropriate singularity image from + S3. Also added validation to ensure the image is in the correct location.