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

Reorganize binaries to reduce image size #5005

Closed
jessesuen opened this issue Dec 8, 2020 · 11 comments · Fixed by #5247
Closed

Reorganize binaries to reduce image size #5005

jessesuen opened this issue Dec 8, 2020 · 11 comments · Fixed by #5247
Assignees
Labels
component:distribution Manifests, docker files, CLI distrubution etc enhancement New feature or request type:scalability Issues related to scalability and performance related issues
Milestone

Comments

@jessesuen
Copy link
Member

Summary

It doesn't make sense to have separate: argocd-server, argocd-application-controller, argocd-repo-server binaries, since these are not user-facing binaries and only bloat the image.

Meanwhile, argocd-util, is a useful binary meant for operators.

Proposal

  1. merge all backend binaries (argocd-server, argocd-application-controller, argocd-repo-server) to a single binary which is included in argocd image
  2. Split dex wrapper out of argocd-util and move it into consolidated backend binary
  3. remove argocd-util from image.

This should reduce our image size considerably (by estimated ~200MB)

@jessesuen jessesuen added the enhancement New feature or request label Dec 8, 2020
@jessesuen jessesuen added this to the v1.9 milestone Dec 8, 2020
@jannfis jannfis added component:distribution Manifests, docker files, CLI distrubution etc type:scalability Issues related to scalability and performance related issues labels Dec 10, 2020
@jannfis
Copy link
Member

jannfis commented Dec 10, 2020

Do you think about something like busybox does, so for example having a single binary (i.e. argocd-server) that behaves differently depending on how it was called (so, a symbolic link argocd-repo-server -> argocd-server that changes argv[0] and therefore starts repo server) or about introducing new sub-commands for a central binary?

For the latter, I would propose naming this new binary argocd-server and following sub-commands:

  • argocd-server api [flags] starts API server
  • argocd-server repo [flags] starts repository server
  • argocd-server controller [flags] starts application controller

I guess for starting locally, testing etc the sub command approach would be more suitable than a busybox-esque approach.

@iam-veeramalla
Copy link
Member

I hope this task will also look at any unused binaries that are left out or forgot to delete in the build process. Example: we might need a lot of binaries to build the application but a-lot of them are not needed to run the container, I see that we are deleting the binaries that are not required in runtime container but I also think all of them are not removed and few are left out.

@jannfis
Copy link
Member

jannfis commented Dec 12, 2020

What binaries exactly do you mean, @iam-veeramalla?

Actually, we use a multi-stage build process and copy only the required binaries to the final target image that gets distributed. Are there some we have erroneously copied, or do you mean binaries that come from the base image debian:10-slim?

@iam-veeramalla
Copy link
Member

iam-veeramalla commented Dec 13, 2020

What binaries exactly do you mean, @iam-veeramalla?

Actually, we use a multi-stage build process and copy only the required binaries to the final target image that gets distributed. Are there some we have erroneously copied, or do you mean binaries that come from the base image debian:10-slim?

@jannfis Yes, I went through the build process, I feel it is well organised with multi-stage build concept. I wonder why we still need binaries likes kubectl in the final target. Argo controller makes api calls with kube-api to fetch the resource information right? Am I missing something here? Please correct me as its my early days with argo.

@jannfis
Copy link
Member

jannfis commented Dec 13, 2020

Ah, good catch about the kubectl binary @iam-veeramalla. You are right that we moved away from fork/exec of kubectl to leverage the K8s API directly using client libraries, since quite a while now.

@jessesuen / @alexmt - this seems like a relict, can we remove it from the distribution or are there any non-obvious dependencies to kubectl still existing?

@iam-veeramalla
Copy link
Member

Ah, good catch about the kubectl binary @iam-veeramalla. You are right that we moved away from fork/exec of kubectl to leverage the K8s API directly using client libraries, since quite a while now.

@jessesuen / @alexmt - this seems like a relict, can we remove it from the distribution or are there any non-obvious dependencies to kubectl still existing?

@jannfis Let me know if you need any PR for this

@jessesuen
Copy link
Member Author

Do you think about something like busybox does, so for example having a single binary (i.e. argocd-server) that behaves differently depending on how it was called (so, a symbolic link argocd-repo-server -> argocd-server that changes argv[0] and therefore starts repo server) or about introducing new sub-commands for a central binary?

Interesting. I didnt realize busybox did that, nor realize that was an option. I don't really have a strong opinion on this, other than that I don't think most people will understand/know that argv[0] will then be significant, and it might make local development annoying.

this seems like a relict, can we remove it from the distribution or are there any non-obvious dependencies to kubectl still existing?

AFAIK, kubectl can/should be removed.

@kshamajain99
Copy link
Contributor

I will take a look at this issue.

@jessesuen
Copy link
Member Author

Spoke with @alexmt and we agree the both the busy box approach as well as sub commands is the way to do this. Busybox prevents upgrade/migration release notes. sub commands allow easier development.

@kshamajain99
Copy link
Contributor

kshamajain99 commented Jan 13, 2021

  • Merging argocd(cli), argocd-util, argocd-server, argocd-application-controller, argocd-repo-server into single binary
  • Moving dex tool from argocd-util to argocd-dex

ArgoCD Binaries Structure:

argocd/argocd-all: (busybox format)

  1. argocd(cli)
  2. argocd-util
  3. argocd-server
  4. argocd-repo-server
  5. argocd-controller
  6. argocd-dex

iam-veeramalla added a commit to iam-veeramalla/argo-cd that referenced this issue Jan 20, 2021
alexmt pushed a commit that referenced this issue Jan 21, 2021
@jangraefen
Copy link
Contributor

I just tried to work through the Contribution guide and noticed that running make verify-kube-connection is now no longer working, since kubectl is no longer part of the test tools image.

Is that an intended site-effect of this issue? If so, the documentation would need a rework. It might also be an option to have kubectl only as part of the test image.

shubhamagarwal19 pushed a commit to shubhamagarwal19/argo-cd that referenced this issue Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:distribution Manifests, docker files, CLI distrubution etc enhancement New feature or request type:scalability Issues related to scalability and performance related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants