Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup VMSS bootstrapping bash scripts #3515

Closed
wants to merge 39 commits into from

Conversation

s-fairchild
Copy link
Collaborator

@s-fairchild s-fairchild commented Apr 10, 2024

Which issue this PR addresses:

ARO-6773

Fixes

What this PR does / why we need it:

This is needed for a resilent to failure RP deployment.
Our current implementation has shown some faults over time with RP deployments, and need improvements in general.

Test plan for issue:

I have been testing rpVMSS.sh in an Azure VM using image RedHat:RHEL:8-lvm-gen2:latest.
Mainly to catch missed syntax errors, and ensure files written are formatted as expected.

I will also need to deploy this to INT, as E2E tests do not spin up the VMSS instances, which utilize these scripts.

Example Log Output

Abort
Error: Unable to find a match: azsec-clamav azsec-monitor azure-cli azure-mdsd azure-security
dnf_install_pkgs: failed to install required packages
Log
configure_sshd: setting ssh password authentication
configure_disk_partitions: extending partition table

Is there any documentation that needs to be updated for this PR?

No

How do you know this will function as expected in production?

Per testing mentioned in the test plan.

@s-fairchild s-fairchild added enhancement New feature or request work-in-progress chainsaw Pull requests or issues owned by Team Chainsaw labels Apr 10, 2024
@s-fairchild s-fairchild force-pushed the s-fairchild/improve-bash-bootstrap branch from e082074 to 91bcccd Compare April 10, 2024 20:51
This will make adding new packages and keys easier
… reuse

Editing the packages and keys passed will allow for easier modification later.
This also allows for function reuse.
@s-fairchild s-fairchild force-pushed the s-fairchild/improve-bash-bootstrap branch from 4ddffa9 to db2b722 Compare April 12, 2024 11:45
…ption

Add starting log message for each function call
rpm database corruption has been seen in testing, and Prod deployments due to concurrent rpm database operations between rpm and dnf. wait is needed due to this.
remove ERR trap, we aren't attempting to trap any specific errors yet.
@s-fairchild s-fairchild force-pushed the s-fairchild/improve-bash-bootstrap branch from db2b722 to 3643a06 Compare April 12, 2024 11:48
Pass wait time as a nameref variable to allow easier modification
Make gateway log directory to be passed in to functions where it is needed, this allows the value to be set in one location.

Move retry steps into a single retry function call

This is to reduce code duplication
Move configure_fiewall_rules file creation to use write_file
Correct accept_ra_conf variable name spelling
Move docker file creation to more logical location
@s-fairchild s-fairchild force-pushed the s-fairchild/improve-bash-bootstrap branch from 6f444c9 to df11d61 Compare April 16, 2024 14:54
Copy link
Collaborator

@SudoBrendan SudoBrendan left a comment

Choose a reason for hiding this comment

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

Feels like a good start. Ping me when this is out of draft.


# This name is used in the case that az acr login searches for this in it's environment
local -r REGISTRY_AUTH_FILE="/root/.docker/config.json"
local -r registry_config_file="{
"auths": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing escape characters since we are no longer EOF on this.

…file writing

The config file was being written twice
… PROXY_IMAGE to configure_service_proxy function
Move create_required_dirs to be the first step that runs, to ensure they
exist when needed. This was mostly for the devProxyVMSS
Move dnf package and repo operations into function, move all service configuration into a function
Move repository configuration higher in sequence of events, make unchanging variables readonly
Add write_file function to cleanup creation of new files, appending to existing files
move packages to install and rpm keys to import into arrays, this will make adding new packages and keys easier
Add retry loop for rpm repo key imports, remove erroneous package from install_pkgs array
Pass dnf packages and rpm keys to function calls via nameref to allow
reuse, This also allows for function reuse.
Add wait for all rpm related transactions to avoid rpm database corruption
Add starting log message for each function call
rpm database corruption has been seen in testing, and Prod deployments due to concurrent rpm database operations between rpm and dnf. wait is needed due to this.
remove ERR trap, we aren't attempting to trap any specific errors yet.
Increase dnf & rpm retry time to 1 minute, 5 minutes total
Pass wait time as a nameref variable to allow easier modification
Add parse_run_options to run individual steps for testing
Add az login -i comment found in gatewayVMSS.sh, change variable syntex to snake case for consistency
Port changes made in rpVMSS.sh to gatewayVMSS.sh
Make gateway log directory to be passed in to functions where it is needed, this allows the value to be set in one location.
Move retry steps into a single retry function call
This is to reduce code duplication
Move configure_fiewall_rules file creation to use write_file
Correct accept_ra_conf variable name spelling
Pass usage options via nameref variable that is also used by getops
Move docker file creation to more logical location
@s-fairchild s-fairchild force-pushed the s-fairchild/improve-bash-bootstrap branch from df11d61 to bdf8d12 Compare April 23, 2024 22:15
…hild/improve-bash-bootstrap

Move all shared code into functions located in commonVMSS.sh to allow
for reusability
@s-fairchild
Copy link
Collaborator Author

Created a new branch when moving shared functions over to commonVMSS.sh, rebasing got kind of hairy.
Opened a new PR #3534.

@s-fairchild s-fairchild deleted the s-fairchild/improve-bash-bootstrap branch April 30, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chainsaw Pull requests or issues owned by Team Chainsaw enhancement New feature or request work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants