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.

Sign up for GitHub

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

Jump to bottom

bump registry binary #80

Merged
merged 2 commits into from Apr 21, 2021
Merged

bump registry binary #80

merged 2 commits into from Apr 21, 2021

Conversation

andriisoldatenko
Copy link
Contributor

@andriisoldatenko andriisoldatenko commented Apr 21, 2021 •
edited

Which issue this PR fixes:

resolves https://github.com/astronomer/issues/issues/2836

Summary of changes:

TL;DR:
we are using old docker registry binary, i build new one 2.7.2 based on my fork https://github.com/andriisoldatenko/distribution/tree/main.

Also it's known issue with AWS SDK GO IRSA (distribution/distribution#3097).

We can switch to release version as soon as core team release it distribution/distribution#3311 (comment)

How i build new binary:

distribution git:(main) ✗ make binaries
+ bin/registry
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build   -o bin/registry -ldflags '-s -w -X github.com/distribution/distribution/v3/version.Version=v2.7.0-258-g6891d948.m -X github.com/distribution/distribution/v3/version.Revision=6891d948327ffbee1a9e7ec2f288f708
d611e613.m -X github.com/distribution/distribution/v3/version.Package=github.com/distribution/distribution/v3 '   ./cmd/registry
+ bin/digest
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build   -o bin/digest -ldflags '-s -w -X github.com/distribution/distribution/v3/version.Version=v2.7.0-258-g6891d948.m -X github.com/distribution/distribution/v3/version.Revision=6891d948327ffbee1a9e7ec2f288f708d6
11e613.m -X github.com/distribution/distribution/v3/version.Package=github.com/distribution/distribution/v3 '   ./cmd/digest
+ bin/registry-api-descriptor-template
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build   -o bin/registry-api-descriptor-template -ldflags '-s -w -X github.com/distribution/distribution/v3/version.Version=v2.7.0-258-g6891d948.m -X github.com/distribution/distribution/v3/version.Revision=6891d948
327ffbee1a9e7ec2f288f708d611e613.m -X github.com/distribution/distribution/v3/version.Package=github.com/distribution/distribution/v3 '   ./cmd/registry-api-descriptor-template

Which images are updated by this PR?:

  • registry

Checklist (required)

Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.

  • version.txt is updated in the changed image(s)

Additional notes for your reviewer:

Need final verification from @pritt20 before merging:

@andriisoldatenko
Copy link
Contributor Author

also i pushed asoldatenko/ap-registry:2.7.2 and shared with @pritt20 he can try this new binary, locally in KIND looks like it works.

k get pods -n astronomer
NAME                                               READY   STATUS      RESTARTS   AGE
astronomer-astro-ui-6f9986b76b-fzhkk               1/1     Running     0          20m
astronomer-cli-install-755c5d9bd8-7n7w9            1/1     Running     0          20m
astronomer-commander-6d4c69b854-jld6f              1/1     Running     0          20m
astronomer-houston-c4f665877-rzdtq                 1/1     Running     1          20m
astronomer-houston-db-migrations-zbxm7             0/1     Completed   0          20m
astronomer-houston-upgrade-deployments-t8xqk       0/1     Completed   0          17m
astronomer-houston-worker-7d9969ccf8-qkhcm         1/1     Running     1          20m
astronomer-kubed-56495cb686-fplq2                  1/1     Running     0          20m
astronomer-nats-0                                  2/2     Running     0          20m
astronomer-nginx-7d64fbb58d-nwtbc                  1/1     Running     0          20m
astronomer-nginx-default-backend-9d568cd6f-7bm6d   1/1     Running     0          20m
astronomer-postgresql-0                            1/1     Running     0          20m
astronomer-prometheus-node-exporter-p677h          1/1     Running     0          20m
astronomer-registry-0                              1/1     Running     0          20m
astronomer-stan-0                                  2/2     Running     0          20m

@pritt20
Copy link
Contributor

pritt20 commented Apr 21, 2021

Thanks a lot for the fix @andriisoldatenko . I have performed few test and everything looks good now, Please find below details for same:

  • Pushed a deployment and verify that deployment specific docker registry files are populated on s3 bucket
  • Enabled S3 access logging and verified that calls are being made by the role assumed by service account
  • Negative test:
    Removed the aws role annotation from default service account and restarted registry pod, which forced registry pod to crash as it failed to connect to S3
  • Re-annotated the service account with aws service account and verified that the registry service is being restored .

Please find below S3 access logs for reference:

76603af33ab8db112bc379c0b692ef6dde33ba24e45c721a185dec34f8911f6b pritt-registry [21/Apr/2021:13:55:53 +0000] 65.2.49.167 arn:aws:sts::854527341753:assumed-role/pritt-registry/1619011098662427334 9RDGWDYE2HKARCPJ REST.PUT.PART docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/a8dbac1d-5341-400e-a13a-4939a5b88956/data "PUT /docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/a8dbac1d-5341-400e-a13a-4939a5b88956/data?partNumber=1&uploadId=C9COTrzW_ETi9DmBjvXlIGRmF9nxY1G2jzNIYJr0IxBEWe99uUUQCkQChjon.XwR72Ru4X8DW2zxVO5nJTWL6MGRKl2XVXVFpIR6GsqoBxj06U9gmpjxhsvPSqQ1TxE4_HbV6zbqxDn7Xr.tlWkH5jV.pMWfce6Lm64oCzD7xRU- HTTP/1.1" 200 - - 190 20 8 "-" "aws-sdk-go/1.34.9 (go1.15.6; linux; amd64)" - NGetDyWkc4GvBUO75RPQOXNgDVe+VGa+AWYS20nn29yHqIAuIzJeHBNZOMtQ1B291ZujaBr38K8= SigV4 ECDHE-RSA-AES128-GCM-SHA256 AuthHeader pritt-registry.s3.ap-south-1.amazonaws.com TLSv1.2
76603af33ab8db112bc379c0b692ef6dde33ba24e45c721a185dec34f8911f6b pritt-registry [21/Apr/2021:13:55:53 +0000] 65.2.49.167 arn:aws:sts::854527341753:assumed-role/pritt-registry/1619011098662427334 9RDJ6TF4SAY6C154 REST.GET.OBJECT docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/6f114b6f-f6d0-4dd1-8b17-497b00818cac/startedat "GET /docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/6f114b6f-f6d0-4dd1-8b17-497b00818cac/startedat HTTP/1.1" 206 - 20 20 9 9 "-" "aws-sdk-go/1.34.9 (go1.15.6; linux; amd64)" - We/rCO+QKJ+QpIXP/78N74WDLEjWOKzUfVaslA9Zpp8LUmpHhbvRvezlFmc8NW4TeycCm0AVgzg= SigV4 ECDHE-RSA-AES128-GCM-SHA256 AuthHeader pritt-registry.s3.ap-south-1.amazonaws.com TLSv1.2
76603af33ab8db112bc379c0b692ef6dde33ba24e45c721a185dec34f8911f6b pritt-registry [21/Apr/2021:13:55:53 +0000] 65.2.49.167 arn:aws:sts::854527341753:assumed-role/pritt-registry/1619011098662427334 9RDKK6XPWKY9PWAT REST.PUT.OBJECT docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/6f114b6f-f6d0-4dd1-8b17-497b00818cac/hashstates/sha256/187 "PUT /docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/6f114b6f-f6d0-4dd1-8b17-497b00818cac/hashstates/sha256/187 HTTP/1.1" 200 - - 108 25 16 "-" "aws-sdk-go/1.34.9 (go1.15.6; linux; amd64)" - F7bDCaGJQWhDZYNQuvUDyyU9r2hq0U7IPsLtksRLMMfZBAH+Zpe3KI8IlkV2LajtOZbWxoFvDcU= SigV4 ECDHE-RSA-AES128-GCM-SHA256 AuthHeader pritt-registry.s3.ap-south-1.amazonaws.com TLSv1.2
76603af33ab8db112bc379c0b692ef6dde33ba24e45c721a185dec34f8911f6b pritt-registry [21/Apr/2021:13:55:53 +0000] 65.2.49.167 arn:aws:sts::854527341753:assumed-role/pritt-registry/1619011098662427334 9RDRPGWHF5HNA2VN REST.GET.UPLOAD docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/a8dbac1d-5341-400e-a13a-4939a5b88956/data "GET /docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/a8dbac1d-5341-400e-a13a-4939a5b88956/data?uploadId=C9COTrzW_ETi9DmBjvXlIGRmF9nxY1G2jzNIYJr0IxBEWe99uUUQCkQChjon.XwR72Ru4X8DW2zxVO5nJTWL6MGRKl2XVXVFpIR6GsqoBxj06U9gmpjxhsvPSqQ1TxE4_HbV6zbqxDn7Xr.tlWkH5jV.pMWfce6Lm64oCzD7xRU- HTTP/1.1" 200 - 895 - 18 15 "-" "aws-sdk-go/1.34.9 (go1.15.6; linux; amd64)" - VVXYrOauxkzq6bWSvgCPJZ+aH1iQWnXsxfLek16pzku2ot9meOWcaRV3/UDe1DyvOD8HdtzSAXY= SigV4 ECDHE-RSA-AES128-GCM-SHA256 AuthHeader pritt-registry.s3.ap-south-1.amazonaws.com TLSv1.2
76603af33ab8db112bc379c0b692ef6dde33ba24e45c721a185dec34f8911f6b pritt-registry [21/Apr/2021:13:55:53 +0000] 65.2.49.167 arn:aws:sts::854527341753:assumed-role/pritt-registry/1619011098662427334 9RDSF0PQH9ESCQMK REST.GET.OBJECT docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/a8dbac1d-5341-400e-a13a-4939a5b88956/startedat "GET /docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/a8dbac1d-5341-400e-a13a-4939a5b88956/startedat HTTP/1.1" 206 - 20 20 24 23 "-" "aws-sdk-go/1.34.9 (go1.15.6; linux; amd64)" - 26IkZOhHFUBkDWcU4NfpFjQIuG5zXTVUpoJ8mANo5KNfS0ElT46i12P54YkA2oXcm63Yr1AjVlY= SigV4 ECDHE-RSA-AES128-GCM-SHA256 AuthHeader pritt-registry.s3.ap-south-1.amazonaws.com TLSv1.2
76603af33ab8db112bc379c0b692ef6dde33ba24e45c721a185dec34f8911f6b pritt-registry [21/Apr/2021:13:55:53 +0000] 65.2.49.167 arn:aws:sts::854527341753:assumed-role/pritt-registry/1619011098662427334 9RDSMAFJEWN8KW83 REST.PUT.OBJECT docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/a8dbac1d-5341-400e-a13a-4939a5b88956/hashstates/sha256/190 "PUT /docker/registry/v2/repositories/barren-sun-8961/airflow/_uploads/a8dbac1d-5341-400e-a13a-4939a5b88956/hashstates/sha256/190 HTTP/1.1" 200 - - 108 21 11 "-" "aws-sdk-go/1.34.9 (go1.15.6; linux; amd64)" - ijaA/xnqPWPMW9UFUld7eol47QK7Nzu6PqvbQe1soSzdOTWhsHJrXgikyITF38ObOb5xHC6OCXk= SigV4 ECDHE-RSA-AES128-GCM-SHA256 AuthHeader pritt-registry.s3.ap-south-1.amazonaws.com TLSv1.2

In above logs we can see that now calls are being made by role mapped to service account that is pritt-registry arn:aws:sts::854527341753:assumed-role/pritt-registry

@danielhoherd
Copy link
Member

@andriisoldatenko it's remarkable to me that we are building a binary in from some other repo by hand and storing the compiled result in this repo, as opposed to building it as part of the docker build process within this repo. We need to automate this, and make this process more direct. I've opened a ticket to address this issue: https://github.com/astronomer/issues/issues/2861

@andriisoldatenko
Copy link
Contributor Author

@danielhoherd i 100% agree with you but the problem originally here https://github.com/docker/distribution-library-image/tree/master/amd64. You can review official docker image.

@andriisoldatenko
Copy link
Contributor Author

i think we can create multi stage dockerfile and get/build from source code and copy to empty layer, so we can remove binary. Only we still need to point to specific commit, since https://github.com/distribution/distribution doesn't have tag or release version since last 2 years!!.

@andriisoldatenko andriisoldatenko merged commit d14db1c into main Apr 21, 2021
@andriisoldatenko andriisoldatenko deleted the bump-docker-registry branch April 21, 2021 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpweber jpweber jpweber approved these changes

@vishwas-astro vishwas-astro vishwas-astro approved these changes

@danielhoherd danielhoherd Awaiting requested review from danielhoherd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@andriisoldatenko @pritt20 @danielhoherd @jpweber @vishwas-astro