Skip to content

cleanup files created by deploy.sh #192#199

Merged
jjansen23 merged 7 commits intomainfrom
fixcleansh
May 11, 2021
Merged

cleanup files created by deploy.sh #192#199
jjansen23 merged 7 commits intomainfrom
fixcleansh

Conversation

@jjansen23
Copy link
Copy Markdown
Contributor

@jjansen23 jjansen23 commented May 7, 2021

Description

Deleted the .mlzconfig and the .tfvars that was created by deploy.sh

Issue reference

The issue this PR will close: #192

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • [x ] Code compiles or validates correctly
  • BASH scripts have been validated using shellcheck
  • [x ] All tests pass (manual and automated)
  • The documentation is updated to cover any new or changed features
  • Markdown files have been linted using the recommended linter. (See .vscode/extensions.json.)
  • [ x] Relevant issues are linked to this PR

Comment thread src/clean.sh Outdated
@jjansen23 jjansen23 requested review from glennmusa and removed request for brooke-hamilton May 10, 2021 17:30
Comment thread src/clean.sh
echo "INFO: cleaning up MLZ resources with tag 'DeploymentName=${mlz_env_name}'..."
. "${this_script_path}/scripts/config/config_clean.sh" "${mlz_config_file}"
# clean up reources that was created by deploy.sh
rm -rf "${configuration_output_path}/${mlz_env_name}.mlzconfig" "${configuration_output_path:?}/${tfvars_filename}"
Copy link
Copy Markdown
Contributor

@glennmusa glennmusa May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the unset validator :?? I want to say set -e should throw on a bad reference/path.

Suggested change
rm -rf "${configuration_output_path}/${mlz_env_name}.mlzconfig" "${configuration_output_path:?}/${tfvars_filename}"
rm -rf "${configuration_output_path}/${mlz_env_name}.mlzconfig" "${configuration_output_path}/${tfvars_filename}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with removing it. This is the output from running shellcheck.
$ shellcheck src/clean.sh

In src/clean.sh line 102:
rm -rf "${configuration_output_path}/${mlz_env_name}.mlzconfig" "${configuration_output_path}/${tfvars_filename}"
^-- SC2115: Use "${var:?}" to ensure this never expands to / .

.... So I added ':?' to resolve it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Thanks for sharing!

Comment thread src/clean.sh Outdated
Comment thread src/clean.sh
echo "INFO: cleaning up MLZ resources with tag 'DeploymentName=${mlz_env_name}'..."
. "${this_script_path}/scripts/config/config_clean.sh" "${mlz_config_file}"
# clean up reources that was created by deploy.sh
rm -rf "${configuration_output_path}/${mlz_env_name}.mlzconfig" "${configuration_output_path:?}/${tfvars_filename}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Thanks for sharing!

Adding the word INFO for consistency.

Co-authored-by: Glenn Musa <4622125+glennmusa@users.noreply.github.com>
@jjansen23 jjansen23 closed this May 11, 2021
@jjansen23 jjansen23 reopened this May 11, 2021
@jjansen23 jjansen23 merged commit 94e0a74 into main May 11, 2021
@jjansen23 jjansen23 deleted the fixcleansh branch May 11, 2021 09:26
jwaltireland pushed a commit to ARPA-H/AzureMissionLZ that referenced this pull request Nov 14, 2024
* cleanup files created by deploy.sh Azure#192

* removing all components .tfvars files

* used explicit tiers instead of '*'

* added function to delete files in dir by name

* changed order file deletes

* Update src/clean.sh

Adding the word INFO for consistency.

Co-authored-by: Glenn Musa <4622125+glennmusa@users.noreply.github.com>

Co-authored-by: jjansen23 <jerome@DESKTOP-IMETN7E.localdomain>
Co-authored-by: Glenn Musa <4622125+glennmusa@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clean.sh should clean up the resources on disk that deploy.sh creates

2 participants