Update create_ec2.sh - #24
Conversation
complete debugged shell code ----> ready to run code
📝 WalkthroughWalkthrough
ChangesEC2 provisioning
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@day03/create_ec2.sh`:
- Line 16: Update the curl download command in create_ec2.sh to include the
--fail flag, ensuring HTTP 4xx/5xx responses produce a nonzero exit status and
do not create an invalid awscliv2.zip for the subsequent unzip step.
- Line 80: Remove the trailing whitespace from the value assigned to SUBNET_ID
so the subnet ID passed to the AWS CLI is exactly the valid identifier.
- Line 89: Fix the script entry invocation by separating the main function name
from the forwarded arguments, so `main` runs correctly with or without
positional parameters and passes all arguments unchanged.
- Around line 30-37: Update the polling loop around the EC2 state query to
enforce a timeout or maximum retry count, exit with a clear failure for terminal
states such as terminated, stopped, or shutting-down, and retry transient AWS
CLI errors instead of exiting immediately under set -e. Preserve the existing
success message and break when the instance reaches running.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| # Download and install AWS CLI v2 | ||
| # Download and install AWS CLI v2 | ||
| curl -s "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o "awscliv2.zip" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add --fail flag to curl to handle HTTP errors.
Without --fail, curl -s returns exit code 0 on HTTP 404/500 and saves the error page as awscliv2.zip. The subsequent unzip then fails with a confusing message instead of indicating the download failed.
🔧 Proposed fix
- curl -s "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o "awscliv2.zip"
+ curl -sfS "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o "awscliv2.zip"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl -s "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o "awscliv2.zip" | |
| curl -sfS "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o "awscliv2.zip" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@day03/create_ec2.sh` at line 16, Update the curl download command in
create_ec2.sh to include the --fail flag, ensuring HTTP 4xx/5xx responses
produce a nonzero exit status and do not create an invalid awscliv2.zip for the
subsequent unzip step.
| while true; do | ||
| state=$(aws ec2 describe-instances --instance-ids "$instance_id" --query 'Reservations[0].Instances[0].State.Name' --output text) | ||
| if [[ "$state" == "running" ]]; then | ||
| echo "Instance $instance_id is now running." | ||
| break | ||
| fi | ||
| sleep 10 | ||
| if [[ "$state" == "running" ]]; then | ||
| echo "Instance $instance_id is now running." | ||
| break | ||
| fi | ||
| sleep 10 | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add timeout and terminal-state detection to the polling loop.
The while true loop has no maximum retry count or timeout. If the instance never reaches running (e.g., enters terminated, stopped, or shutting-down), the script hangs indefinitely. Additionally, with set -e, any transient AWS API error exits the script immediately with no retry.
🔧 Proposed fix
wait_for_instance() {
local instance_id="$1"
+ local max_attempts=30
+ local attempt=0
echo "Waiting for instance $instance_id to be in running state..."
- while true; do
+ while [[ $attempt -lt $max_attempts ]]; do
state=$(aws ec2 describe-instances --instance-ids "$instance_id" --query 'Reservations[0].Instances[0].State.Name' --output text)
if [[ "$state" == "running" ]]; then
echo "Instance $instance_id is now running."
break
fi
+ if [[ "$state" == "terminated" || "$state" == "shutting-down" || "$state" == "stopped" ]]; then
+ echo "Instance $instance_id entered terminal state: $state" >&2
+ exit 1
+ fi
sleep 10
+ ((attempt++))
done
+ if [[ $attempt -eq $max_attempts ]]; then
+ echo "Timeout waiting for instance $instance_id to reach running state" >&2
+ exit 1
+ fi
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@day03/create_ec2.sh` around lines 30 - 37, Update the polling loop around the
EC2 state query to enforce a timeout or maximum retry count, exit with a clear
failure for terminal states such as terminated, stopped, or shutting-down, and
retry transient AWS CLI errors instead of exiting immediately under set -e.
Preserve the existing success message and break when the instance reaches
running.
| AMI_ID="ami-0b6d9d3d33ba97d99" | ||
| INSTANCE_TYPE="t2.micro" | ||
| KEY_NAME="AWSCLI_demo" | ||
| SUBNET_ID="subnet-01845bee6f9f54abb " |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove trailing space in SUBNET_ID.
SUBNET_ID="subnet-01845bee6f9f54abb " has a trailing space that will be passed to --subnet-id, causing an InvalidSubnetID.NotFound error from the AWS API.
🐛 Proposed fix
- SUBNET_ID="subnet-01845bee6f9f54abb "
+ SUBNET_ID="subnet-01845bee6f9f54abb"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SUBNET_ID="subnet-01845bee6f9f54abb " | |
| SUBNET_ID="subnet-01845bee6f9f54abb" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@day03/create_ec2.sh` at line 80, Remove the trailing whitespace from the
value assigned to SUBNET_ID so the subnet ID passed to the AWS CLI is exactly
the valid identifier.
|
|
||
| # Call the function to create the EC2 instance | ||
| create_ec2_instance "$AMI_ID" "$INSTANCE_TYPE" "$KEY_NAME" "$SUBNET_ID" "$SECURITY_GROUP_IDS" "$INSTANCE_NAME" | ||
| main"$@" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix script entry: main"$@" concatenates function name with first argument.
main"$@" merges "main" with "$1" into a single token (e.g., mainfoo), so the script fails with "command not found" whenever arguments are passed. It only works by accident when no arguments are given. Shellcheck confirms this as SC2145.
🐛 Proposed fix
-main"$@"
+main "$@"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| main"$@" | |
| main "$@" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 89-89: Argument mixes string and array. Use * or separate argument.
(SC2145)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@day03/create_ec2.sh` at line 89, Fix the script entry invocation by
separating the main function name from the forwarded arguments, so `main` runs
correctly with or without positional parameters and passes all arguments
unchanged.
Source: Linters/SAST tools
1 participant
------>complete debugged shell code ----> ready to run code
#!/bin/bash
set -euo pipefail
check_awscli() {
if ! command -v aws &> /dev/null; then
echo "AWS CLI is not installed. Please install it first." >&2
return 1
}
install_awscli() {
echo "Installing AWS CLI v2 on Linux..."
#verify installation
aws --version
clean up
}
wait_for_instance() {
local instance_id="$1"
echo "Waiting for instance $instance_id to be in running state..."
}
create_ec2_instance(){
local ami_id="$1"
local instance_type="$2"
local key_name="$3"
local subnet_id="$4"
local security_group_ids="$5"
local instance_name="$6"
#run AWS CLI command t create EC2 instance
instance_id=$(aws ec2 run-instances
--image-id "$ami_id"
--instance-type "$instance_type"
--key-name "$key_name"
--subnet-id "$subnet_id"
--security-group-ids "$security_group_ids"
--tag-specifications "ResourceType=instance,Tags=[{Key=Name,Value=$instance_name}]"
--query 'Instances[0].InstanceId'
--output text
)
if [[ -z "$instance_id" ]]; then
echo "failed to create instance"
exit 1
fi
echo "instance $instance_id created"
#wait for instance to be in running "
wait_for_instance "$instance_id"
}
main() {
if ! check_awscli ; then
install_awscli || exit 1
fi
echo "creating EC2 instance ...."
# Specify the parameters for creating the EC2 instance
AMI_ID="ami-0b6d9d3d33ba97d99"
INSTANCE_TYPE="t2.micro"
KEY_NAME="AWSCLI_demo"
SUBNET_ID="subnet-01845bee6f9f54abb "
SECURITY_GROUP_IDS="sg-0ddba97c267e46e49" # Add your security group IDs separated by space
INSTANCE_NAME="Shell-Script-EC2-Demo"
}
main"$@"
Summary by CodeRabbit
New Features
Bug Fixes