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

updated docker to version 24 #29

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

updated docker to version 24 #29

wants to merge 2 commits into from

Conversation

fideloper
Copy link

@fideloper fideloper commented Jan 4, 2024

Supercedes #15

Withing that PR, Jerome got errors. I believe I resolved those here (or at least I have it working in my own app!). The changes were related to needing an older iptables and ip6tables package (apk add iptables-legacy).

This PR updates Docker to 24, and Buildx to 0.12 (along with AlpineJS under the hood).

I tested this on my own repository, but not in rchab directly. The main difference in my own test repository was:

  1. Using docker:24-dind base image (this repo never used the dind variant and is likely fine without it, this PR does not use it)
  2. Running ip6tables-legacy -t nat -A POSTROUTING -s 2001:db8:1::/64 ! -o docker0 -j MASQUERADE in the docker-entrypoint.d/docker script (which was just a copy/paste/tweak from previous work I did to create my own docker builder app that wasn't the official Fly builder. It likely isn't needed here.)

I didn't test this change locally first in this specific repository, mostly because I didn't want to install Vagrant. I can do this work if no one else wants to :P

@jsierles
Copy link
Member

jsierles commented Jan 4, 2024

It's possible to test in production by pushing to Docker hub and assigning the image in org settings in a production console.

@jeromegn
Copy link
Member

jeromegn commented Jan 4, 2024

I think that happens automatically: flyio/rchab:docker-24,flyio/rchab:sha-991f788

@fideloper
Copy link
Author

fideloper commented Jan 4, 2024

Automagically would be great. I don't have access to those Docker repositories so I think I'll need help pushing this in the next step (which sounds like testing it out in testflight)

(like this might be happening in buildkite already but I don't have access to see :P )

Copy link
Member

@jsierles jsierles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, assuming testing works out. To put this build into production, you'll need to update the default image in the backend.

@fideloper
Copy link
Author

Tests I've run:

  1. ✅ Regular old apps (JS/PHP in this case)
  2. ✅ Buildpacks (Go, Python via Flask)
  3. ✅ Buildkite features

Buildkite cache mount test:

# fly deploy --build-secret MY_SUPER_SECRET=some_value 

FROM ubuntu:22.04

COPY some-file.txt /some-file.txt

# 1️⃣ Cache package downloads
RUN \
    --mount=type=cache,target=/var/cache/apt \
    apt update && apt-get --no-install-recommends install -y gcc curl wget ca-certificates 

# Set secrets
RUN --mount=type=secret,id=MY_SUPER_SECRET \
   cat /run/secrets/MY_SUPER_SECRET

CMD ["sleep", "inf"]

@fideloper
Copy link
Author

I believe with our kernel updates we can remove the iptables-legacy stuff (and then retest it)

@fideloper
Copy link
Author

Deleted my builder and the one it re-created is working. I believe @jsierles still has my personal org using this builder when creating a new one, so the lack of iptables-legacy appears to be working.

Would love a double check on that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants