-
Notifications
You must be signed in to change notification settings - Fork 117
add exec prerequisites to ecs-anywhere installation script #451
add exec prerequisites to ecs-anywhere installation script #451
Conversation
scripts/ecs-anywhere-install.sh
Outdated
cd && mkdir -p ssm-binaries && cd ssm-binaries | ||
case $ARCHITECTURE in | ||
'x86_64') | ||
curl https://s3.${REGION}.amazonaws.com/amazon-ssm-${REGION}/${BINARY_VERSION}/linux_amd64/amazon-ssm-agent-binaries.tar.gz -o amazon-ssm-agent.tar.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For CN partition, I think, this S3 url will be a bit different -> s3.${REGION}.amazonaws.com.cn
scripts/ecs-anywhere-install.sh
Outdated
fi | ||
|
||
if [ -z "$certs" ]; then | ||
echo "Could not find certificates. Please generate TLS certificates, or provide a valid file path to them" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this message ask the user to rerun the script with the --certs-file flag to provide the vaild certificate path?
4cf7c11
to
c368be7
Compare
scripts/ecs-anywhere-install.sh
Outdated
@@ -35,6 +35,8 @@ usage() { | |||
(optional) Version of the ECS agent rpm/deb package to use. If not specified, default to latest. | |||
--skip-registration | |||
(optional) if this is enabled, SSM agent install and instance registration with SSM is skipped. | |||
--certs-file | |||
(optional) file for TLS certs. Defaults to predefined location for a given platform. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add more details such as - TLS certs for Execute command feature
might also consider renaming flag to exec-certs-file
c368be7
to
b5275a9
Compare
scripts/ecs-anywhere-install.sh
Outdated
} | ||
|
||
download-ssm-binaries-exec() { | ||
BINARY_VERSION="3.1.338.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a specific reason we use this version? Is there a latest
that we can use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will be a stable version endpoint available 12/08. We're hardcoding version for now to access what's currently available, but around that date I'll update the script in this branch to use that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're currently using 3.1.90.0 in the ecs ami: https://github.com/aws/amazon-ecs-ami/blob/main/variables.pkr.hcl#L59
we should probably use the same version, either by downgrading this version or following up with an ecs ami PR to update the version the AMI uses.
If we do update, might be worth bumping both to SSM's latest available version which is 3.1.501.0: https://github.com/aws/amazon-ssm-agent/releases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out, it looks like there are a couple places where this might be inconsistent. For now at least I'll update to 3.1.501.0
in this script, though before releasing this we will be updating to a url that just points to the latest version. As part of that, I can look into how we can get that elsewhere and remove some of the hardcoded versions
scripts/ecs-anywhere-install.sh
Outdated
fail | ||
else | ||
echo "Using $certs" | ||
mkdir -p $CERTS_PATH && cd $CERTS_PATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if cd
would break other assumptions in the script now or in the future. Perhaps it's more convenient to not change the current dir and use absolute paths?
b5275a9
to
ab54fe8
Compare
echo "Provided certs file does not exist, looking for certs in known possible locations" | ||
fi | ||
for f in "${CERTS_FILES_LOCATIONS[@]}" | ||
do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q. Just a thought. Would it provide better UX if we printing out what certs have seen searched?
echo "Searching certs file `/etc/ssl/certs/ca-certificates.crt` "
...
echo "Could not find `/etc/ssl/certs/ca-certificates.crt`"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting point. I can see why some users might want to know these if the script doesn't find the certs. My thinking against doing that is that it would pollute the output a bit to log all of them, and that it's not super useful for the user to know given that these are often already present and easily able to be found just by looking at the script. Also, users can simply specify the location if the certs are not found by default. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree doing this will a kind of messing up output, and users can always provide a path if the certs cannot be found.
266badd
ab54fe8
to
266badd
Compare
(accidentally removed |
Summary
This pr makes changes to the ECS Anywhere installation script in order for exec to be supported on ECS Anywhere. Two changes are required - copying TLS certs and downloading ssm binaries. Both of these will be moved to a directory that will be bind-mounted into the customer's container at start, which is consistent with the current behavior for ECS exec on EC2.
For further context on exec, see:
Implementation details
The changes to installation script are:
Testing
Description for the changelog
Add exec prerequisites to ecs-anywhere installation script
Licensing
This contribution is under the terms of the Apache 2.0 License: yes