-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat(dockerfiles/cd/utils/release): add aws-cli tool #533
Conversation
Based on the title of the pull request and the diff, it seems that the developer has added the One potential problem that I see in the diff is that there is a redundant To fix this issue, the developer should remove the first Here is the updated diff: diff --git a/dockerfiles/cd/utils/release/Dockerfile b/dockerfiles/cd/utils/release/Dockerfile
index 3cfa969d..7b8cf57f 100644
--- a/dockerfiles/cd/utils/release/Dockerfile
+++ b/dockerfiles/cd/utils/release/Dockerfile
@@ -34,3 +34,11 @@ RUN OS=linux; ARCH=$([ "$(arch)" = "x86_64" ] && echo amd64 || echo arm64); \
wget -q -O - https://tiup-mirrors.pingcap.com/tiup-v${TIUP_VER}-${OS}-${ARCH}.tar.gz | tar -zxvf - -C /usr/local/bin && \
chmod 755 /usr/local/bin/tiup && \
mkdir -p "$HOME/.tiup/bin"
+
+# install tools: aws-cli
+ARG AWS_CLI_VERSION=2.23.0
+RUN OS=linux; ARCH=$([ "$(arch)" = "x86_64" ] && echo amd64 || echo aarch64); \
+ curl "https://awscli.amazonaws.com/awscli-exe-${OS}-{ARCH}-${AWS_CLI_VERSION}.zip" -o "awscliv2.zip" && \
+ unzip awscliv2.zip && \
+ ./aws/install --bin-dir /usr/local/bin --install-dir /usr/local/aws-cli --update && \
+ rm -rf awscliv2.zip aws Overall, the proposed changes seem reasonable and the addition of the |
57cec55
to
478f187
Compare
Based on the pull request title and diff, the key change made is the addition of the One potential problem with this change is that it may increase the size of the container image, which may lead to longer build and deployment times. Additionally, the use of To fix this, I suggest using the |
Signed-off-by: wuhuizuo <[email protected]>
478f187
to
400ce32
Compare
Based on the PR title and diff, it looks like the change is adding the One potential problem I can see is that the To address these issues, it may be helpful to consider using a smaller base image for the container and removing any unnecessary packages or dependencies. Additionally, it may be helpful to test the container with the added |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wuhuizuo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.