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

Use go-chef for image builds #989

Merged
merged 3 commits into from
Sep 12, 2024
Merged

Use go-chef for image builds #989

merged 3 commits into from
Sep 12, 2024

Conversation

sharnoff
Copy link
Member

@sharnoff sharnoff commented Jun 23, 2024

It dramatically reduces the amount of time it takes to re-run image building if the set of packages used hasn't changed. It's hard to get an objective measurement (because we do get some caching from being careful what we add), but:

On my machine, for a rebuild after a change in 'pkg/api', it reduces make docker-build time from 249s -> 22s.

And, it also slightly improves the time it takes to run a clean build. After docker system prune -af, it reduces make docker-build on my machine from 345s -> 126s.

@sharnoff sharnoff requested a review from Omrigan June 23, 2024 21:21
@sharnoff
Copy link
Member Author

Another fun datapoint: Seems like it may drop clean image builds in CI from ~2m50s → ~1m45s

Copy link
Contributor

@Omrigan Omrigan left a comment

Choose a reason for hiding this comment

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

The main thing I am concerned here is the status of go-chef going forward. Who is going to be the owner of it? Maybe @neondatabase/developer-productivity? Do we want it to use for other go-based images?

That's unfortunate that there is no standard way of solving this problem in the go community.

neonvm/Dockerfile Show resolved Hide resolved
Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

Copy link
Member

@bayandin bayandin left a comment

Choose a reason for hiding this comment

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

@sharnoff said it works correctly

It *dramatically* reduces the amount of time it takes to re-run image
building if the set of packages used hasn't changed.
It's hard to get an objective measurement (because we do get *some*
caching from being careful what we add), but:

On my machine, for a rebuild after a change in 'pkg/api', it reduces
`make docker-build` time from 249s -> 22s.

And, it also slightly improves the time it takes to run a clean build.
After `docker system prune -af`, it reduces `make docker-build` on my
machine from 345s -> 126s.
@sharnoff sharnoff merged commit d42ed7c into main Sep 12, 2024
21 checks passed
@sharnoff sharnoff deleted the sharnoff/go-chef branch September 12, 2024 20:22
sharnoff added a commit that referenced this pull request Dec 3, 2024
Probably an inadvertent merge conflict between #1090 and #989 meaning we
accidentally weren't using go-chef for neonvm-daemon.

Noticed this while developing locally and saw that it was re-downloading
all of the dependencies for neonvm-daemon every time, even though I was
making changes in the scheduler and the dependencies hadn't changed.
sharnoff added a commit that referenced this pull request Dec 3, 2024
Probably an inadvertent merge conflict between #1090 and #989 meaning we
accidentally weren't using go-chef for neonvm-daemon.

Noticed this while developing locally and saw that it was re-downloading
all of the dependencies for neonvm-daemon every time, even though I was
making changes in the scheduler and the dependencies hadn't changed.
sharnoff added a commit that referenced this pull request Dec 4, 2024
Probably an inadvertent merge conflict between #1090 and #989 meaning we
accidentally weren't using go-chef for neonvm-daemon.

Noticed this while working on #1163 locally and saw that it was
re-downloading all of the dependencies for neonvm-daemon every time,
even though I was making changes in the scheduler and the dependencies
hadn't changed.
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.

4 participants