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

Shell Script: script to automate modification of EBS GP2 volumes to GP3 in EC2 service #5631

Closed
wants to merge 8 commits into from

Conversation

razguru
Copy link

@razguru razguru commented Nov 10, 2023

This pull request is to publish the bash scripts to automate the discovery of all EBS volume types and modify all identified GP2 volume type to GP3 with optional snapshot in any region or all regions. It can span across multiple accounts post setting IAM assume role account's profile.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@razguru razguru changed the title Bash Script: script to automate modification of EBS GP2 volumes to GP3 in EC2 service Shell Script: script to automate modification of EBS GP2 volumes to GP3 in EC2 service Nov 10, 2023
@DavidSouther DavidSouther added the Task A general update to the code base for language clarification, missing actions, tests, etc. label Nov 10, 2023
Copy link
Contributor

@meyertst-aws meyertst-aws left a comment

Choose a reason for hiding this comment

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

Hi Rohit,
The PR needs to be linted, formatted and moved.
You should move it to the folder
"aws-cli/bash-linux/ec"
You should lint it with "shellcheck".
You should format it with "shfmt".
You can look at "aws-cli/bash-linux/ec2/change-ec2-instance-type" and follow their example for comments' style and testing.

I will do a more thorough review when it has been linted and formatted.

# Author : @razguru
# Version : 1.1 - - Added cross account action through --profile assume role
# Desc : Discover or/and modify GP2 volumes with/without snapshot

Copy link
Contributor

Choose a reason for hiding this comment

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

The script needs the proper copyright and information about setting up and running AWS CLI.
See

##############################################################################

@@ -0,0 +1,248 @@
#! /bin/bash
# Author : @razguru
Copy link
Contributor

Choose a reason for hiding this comment

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

Author and version should not be here.

manual-disc()
{
if [ ! -z "$file_name" ] && [ -s "$file_name" ] ; then
/bin/cp "$file_name" "$gp2_vol_id" &> /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not use full paths for commands

Suggested change
/bin/cp "$file_name" "$gp2_vol_id" &> /dev/null
cp "$file_name" "$gp2_vol_id" &> /dev/null

io2_vol_id="$account"_"$region"_"$dt"_io2_vol_id.txt

gp2_vol_error="$account"_"$region"_"$dt"_gp2_vol_error.txt
/usr/bin/aws ec2 describe-volumes --region "${region}" --profile "${account}" --filters Name=volume-type,Values=gp2 > /dev/null 2> "$gp2_vol_error"
Copy link
Contributor

Choose a reason for hiding this comment

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

Again here and elsewhere, you should not use full paths

Suggested change
/usr/bin/aws ec2 describe-volumes --region "${region}" --profile "${account}" --filters Name=volume-type,Values=gp2 > /dev/null 2> "$gp2_vol_error"
aws ec2 describe-volumes --region "${region}" --profile "${account}" --filters Name=volume-type,Values=gp2 > /dev/null 2> "$gp2_vol_error"

dt=`date +%F-%T`

# Ensure region is provided
if [ -z $region ] || [[ $reg != --region ]] || [ -z $file_name ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The script should have a main function.

log=""$region"_"$dt"_vol_modification_progress.log"

#Ensure vol_id_list is provided
if [ -z $vol_id_list ] || [ ! -s $vol_id_list ] ; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The script should have a main function.

@meyertst-aws meyertst-aws added the CLI Relates to the AWS CLI label Nov 14, 2023
Comment on lines +1 to +21
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "VisualEditor0",
"Effect": "Allow",
"Action": [
"ec2:ModifyVolume",
"ec2:DescribeVolumes",
"ec2:DescribeVolumesModifications",
"ec2:DescribeVolumeStatus",
"ec2:DescribeTags",
"ec2:CreateTags",
"ec2:DescribeRegions",
"ec2:DescribeSnapshots",
"ec2:CreateSnapshot",
],
"Resource": "*"
}
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommended: rename file to JSON to match data type.

@ford-at-aws
Copy link
Contributor

ford-at-aws commented Dec 8, 2023

Hey @razguru ! Thank you for your contribution. Do you need any assistance with these changes requested by @meyertst-aws ?

@meyertst-aws meyertst-aws mentioned this pull request Jan 22, 2024
@meyertst-aws
Copy link
Contributor

Closing this until the author has time to work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Relates to the AWS CLI Task A general update to the code base for language clarification, missing actions, tests, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants