neondatabase / autoscaling

Postgres vertical autoscaling in k8s
Apache License 2.0
153 stars 21 forks source link

Implement docker auth for vm-builder #814

Closed bayandin closed 3 weeks ago

bayandin commented 7 months ago

Problem description / Motivation

vm-builder might fail if we send too many requests to docker hub from our hosts:

2024/02/19 15:57:36 Setup docker connection
2024/02/19 [15](https://github.com/neondatabase/neon/actions/runs/7960954540/job/21732689596#step:5:16):57:36 Build docker image for virtual machine (disk size 1G): 369495373322.dkr.ecr.eu-central-1.amazonaws.com/vm-compute-node-v15:7960954540
Step 1/53 : FROM debian:bullseye-slim as libcgroup-builder
bullseye-slim: Pulling from library/debian
2024/02/19 15:57:38 toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit

Ref https://github.com/neondatabase/neon/actions/runs/7960954540/job/21732689596

To avoid this, we, usually, use authorised access to docker hub, but it seems docker go SDK doesn't pick up auth from DOCKER_CONFIG directory automatically (https://github.com/moby/moby/issues/39377#issuecomment-503589708), so we need to make it possible to pass docker user/password as additional vm-builder parameters.

Feature idea(s) / DoD

Since we're doing docker login in CI (and locally), maybe it's better to rely on the credentials stored in the docker cli config (~/.docker/config.json)? As shown in the example in this comment.

  • Also, to think if it should support different registries (like ECR) and to check it if it should
  • Alternatively, we can add to vm-builder --docker-user/--docker-password parameters

Implementation ideas

cicdteam commented 7 months ago

Since we're doing docker login in CI (and locally), maybe it's better to rely on the credentials stored in the docker cli config (~/.docker/config.json)? As shown in the example in this comment.

bayandin commented 7 months ago

Since we're doing docker login in CI (and locally), maybe it's better to rely on the credentials stored in the docker cli config (~/.docker/config.json)? As shown in the example in this comment.

Nice! Added it to the Issue description as an idea for implementation

Omrigan commented 7 months ago

Assigning to @bayandin, as there is a draft PR from him.

sharnoff commented 3 months ago

Just hit this here, I think: https://github.com/neondatabase/autoscaling/actions/runs/9574058662/job/26396646325?pr=974

Unassigning @bayandin ; the PR has been closed for now.