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

chore: simplify Dockerfiles #3565

Merged
merged 5 commits into from
Nov 28, 2024
Merged

chore: simplify Dockerfiles #3565

merged 5 commits into from
Nov 28, 2024

Conversation

alecthomas
Copy link
Collaborator

Consolidates on a single Dockerfile for all primary services. Language-specific runners require a separate Dockerfile each in the form Dockerfile.runner-<language>. Build all of these with just build-docker <name>, eg. just build-docker runner-jvm.

Also adds a just list-docker-images, and updates ci.yml to build via a matrix using this.

Warning

In order to simplify the Dockerfiles this changes a bunch of environment variables , which might break due to downstream infra that set the old values.

@alecthomas alecthomas requested a review from a team as a code owner November 28, 2024 08:38
@alecthomas alecthomas requested review from stuartwdouglas and removed request for a team November 28, 2024 08:38
This was referenced Nov 28, 2024
@stuartwdouglas stuartwdouglas added the run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue label Nov 28, 2024
Copy link
Collaborator

@stuartwdouglas stuartwdouglas left a comment

Choose a reason for hiding this comment

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

I think we also need to update the release workflows.

Copy link
Collaborator

@stuartwdouglas stuartwdouglas left a comment

Choose a reason for hiding this comment

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

Also the Justfile in ./deployments needs to be updated, as it was relying on the .test dockerfiles.

@alecthomas
Copy link
Collaborator Author

I think we also need to update the release workflows.

Already done :)

@alecthomas
Copy link
Collaborator Author

Also the Justfile in ./deployments needs to be updated, as it was relying on the .test dockerfiles.

Ah yep, will do that now.

@alecthomas
Copy link
Collaborator Author

Also the Justfile in ./deployments needs to be updated, as it was relying on the .test dockerfiles.

Updated, but I'm not sure it's being exercised in CI (?), so it might need a manual check potentially.

@alecthomas
Copy link
Collaborator Author

Also the Justfile in ./deployments needs to be updated, as it was relying on the .test dockerfiles.

Updated, but I'm not sure it's being exercised in CI (?), so it might need a manual check potentially.

Mmm well some of the CI jobs are failing, so I guess that answers that question 😂

@stuartwdouglas
Copy link
Collaborator

Smoke Test Upgrade Path got broken by a different PR, so don't worry about that one. I can look at fixing TestKubeScaling tomorrow if you want.

@alecthomas
Copy link
Collaborator Author

Smoke Test Upgrade Path got broken by a different PR, so don't worry about that one. I can look at fixing TestKubeScaling tomorrow if you want.

Ah, kk. I think I have TestKubeScaling working, it was just some stupid stuff in the builds.

@alecthomas
Copy link
Collaborator Author

Mmm okay I spoke too soon; everything is working except TestKubeScaling :(

@stuartwdouglas stuartwdouglas merged commit c6bac1e into main Nov 28, 2024
95 of 96 checks passed
@stuartwdouglas stuartwdouglas deleted the aat/simplify-docker branch November 28, 2024 21:47
jvmakine added a commit that referenced this pull request Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants