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

bump VM_BUILDER_VERSION to v0.16.2 #5031

Merged
merged 3 commits into from
Aug 18, 2023
Merged

Conversation

fprasx
Copy link
Contributor

@fprasx fprasx commented Aug 17, 2023

A very slight change that allows us to configure the UID of the neon-postgres cgroup owner. We start postgres in this cgroup so we can scale it with the cgroups v2 api. Currently, the control plane overwrites the entrypoint set by vm-builder, so compute_ctl (and thus postgres), is not started in the neon-postgres cgroup. Having compute_ctl start postgres in the cgroup should fix this. However, at the moment appears like it does not have the correct permissions. Configuring the neon-postgres UID to postgres (which is the UID compute_ctl runs under) should hopefully fix this.

See #4920 - the PR to modify compute_ctl to start postgres in the cgorup.
See: neondatabase/autoscaling#480, neondatabase/autoscaling#477. Both these PR's are part of an effort to increase vm-builder's configurability and allow us to adjust it as we integrate in the monitor.

A very slight change that allows us to configure the UID of the
neon-postgres cgroup. This will possibly unlock neon/#4920, having
`compute_ctl` start postgres in a cgroup instead of `vm-builder`.
@fprasx fprasx requested a review from sharnoff August 17, 2023 21:53
This change is new to vm-builder v0.16.x. We'll remove these flags
after the monitor is started by compute-ctl and in later release of
vm-builder that include autoscaling #442, which allows the agent to
communicate directly with the monitor
@fprasx fprasx force-pushed the fprasx/bump-vm-builder-version branch from 0be29fe to 65d6d42 Compare August 17, 2023 22:00
Copy link
Member

@sharnoff sharnoff left a comment

Choose a reason for hiding this comment

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

Would be good to link relevant autoscaling repo PRs to help answer immediate questions (i.e. "why do we need the new args?" and "what's this UID business?"), but otherwise looks good

Also NB: if you just write #442 in a commit message, it'll link to this repo. To link to another repo, you can write neondatabase/autoscaling#442 (I think)

Copy link
Member

@sharnoff sharnoff left a comment

Choose a reason for hiding this comment

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

meant to click approve, oops

@github-actions
Copy link

github-actions bot commented Aug 17, 2023

1612 tests run: 1537 passed, 0 failed, 75 skipped (full report)


The comment gets automatically updated with the latest test results
76c5f95 at 2023-08-18T16:47:44.964Z :recycle:

@fprasx fprasx merged commit 5c6a692 into main Aug 18, 2023
@fprasx fprasx deleted the fprasx/bump-vm-builder-version branch August 18, 2023 18:29
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.

2 participants