-
Notifications
You must be signed in to change notification settings - Fork 96
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
✨ Adding aws-cli to docker image #729
✨ Adding aws-cli to docker image #729
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #729 +/- ##
==========================================
- Coverage 63.51% 63.32% -0.19%
==========================================
Files 185 186 +1
Lines 17838 17900 +62
==========================================
+ Hits 11329 11335 +6
- Misses 5576 5629 +53
- Partials 933 936 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
eec0701
to
76486e0
Compare
2e4e81a
to
1251732
Compare
Signed-off-by: Gaurav Jaswal <[email protected]>
1251732
to
2100e7e
Compare
build/Dockerfile.aws-cli
Outdated
RUN apt-get update | ||
RUN apt-get install unzip | ||
RUN unzip -v | ||
RUN curl "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o "awscliv2.zip" |
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 we make this as arch agnostic? we would also need to support other arch.
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.
Pushed the change for quick feedback, please advise if it looks ok. Will fix the commit sign-off tomorrow moning.
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.
looks good
also would you help to remind me on why we do not use aws go sdk? https://pkg.go.dev/github.com/aws/aws-sdk-go-v2. I remember we discussed this before. It will be good to leave some comments so we do not miss the reason. |
Yes, that is because the eks kubeconfig uses aws cli. We discussed about it and @deads2k suggested an option to copy aws cli from init container to klusterlet agent container. With this we do not have to maintain 2 copies of registration operator docker image, one for csr and other for aws auth. |
@@ -0,0 +1,25 @@ | |||
FROM golang:1.22-bullseye AS builder | |||
ARG OS=linux | |||
ARG ARCH=x86_64 |
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 ARM
ARG ARCH=aarch64
79690e3
to
7fce5f4
Compare
Signed-off-by: Gaurav Jaswal <[email protected]>
7fce5f4
to
c077275
Compare
Signed-off-by: Gaurav Jaswal <[email protected]>
Signed-off-by: Gaurav Jaswal <[email protected]>
/approve @mikeshng I think we also need to create a repo in quay for this image. |
Ok. I have submitted the repo creation request. Thank you all for your patience. |
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.
/lgtm
repo has been created https://quay.io/repository/open-cluster-management/aws-cli
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jaswalkiranavtar, mikeshng, qiujian16 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 |
b506698
into
open-cluster-management-io:main
Summary
This image will be use as initContainer to copy the aws-cli to klusterlet-agent when initialized with awsirsa auth type.
This will allow us to keep registration-operator docker image clean.
Related issue(s)
Fixes # #514