neonvm: continuous synchronisation of files in pod/vm #1222
Conversation
No changes to the coverage.
HTML Report |
conradludgate
force-pushed
the
conrad/vm-fsnotify
branch
from
e54e858 to
a931733
Compare
conradludgate
force-pushed
the
conrad/vm-fsnotify
branch
from
f2456ad to
26f66b5
Compare
|
open questions:
|
|
@conradludgate @sharnoff Have you considered instead of relying on inotify and push model to modify neonvm-daemon to subscribe for secret changes via kube-API to reduce the number of moving parts?
|
|
I think this inevitably has the same issue. The service account token updates in the pod, and the VM needs to notice that change |
|
I've looked into service accounts. They also use the same form of atomic directory updates. Currently there's no projected volume disk source for VM objects, but this is the kinda thing that could be hard-coded in the neomvm-runner code. Add |
conradludgate
force-pushed
the
conrad/vm-fsnotify
branch
from
03c3135 to
546f869
Compare
|
High level, feels like we need a shared file system primitive for the VMs, I keep seeing items that this would potentially solve. |
Not sure exactly what this means |
There was a problem hiding this comment.
Broadly looking good -- left a thorough review; I expect 1, maybe 2, much smaller rounds remaining
Also, could you update the PR title to have the structure component: Title ? (ordinarily the PR title format checker would catch that, but feat: is a sufficient prefix to trick the regex 😄)
In this case, s/feat/neonvm/ would suffice, but it's up to you
| @@ -187,6 +187,10 @@ func createISO9660runtime( | |||
| if disk.MountPath != "" { | |||
| mounts = append(mounts, fmt.Sprintf(`/neonvm/bin/mkdir -p %s`, disk.MountPath)) | |||
| } | |||
| if diskNeedsSynchronisation(disk) { | |||
| // do nothing as we will mount it into the VM via neonvm-daemon later | |||
There was a problem hiding this comment.
question: Does this mean that programs starting inside the VM may initially not have access to those secrets? (if so: how difficult would it be to guarantee the initial versions of the secret once everything's mounted?)
There was a problem hiding this comment.
This is correct. Currently the solution is the "fast" 1-second loop to ping neonvm-daemon.
I suppose there is a solution we can do where we could mount the initial values as a static disk with a symlink, but I'm not sure how complicated that would end up being
| return fmt.Errorf("could not open file: %w", err) | ||
| } | ||
|
|
||
| dto := make(map[string]File) |
There was a problem hiding this comment.
question: What does dto mean here?
There was a problem hiding this comment.
data transfer object (it's very java-y). I couldn't think of a better name at the time
pkg/util/checksum.go
Outdated
| var length []byte | ||
|
|
||
| // hash as "{name}\0{len(data)}{data}" | ||
| // this prevents any possible hash confusion problems |
There was a problem hiding this comment.
this prevents any possible hash confusion problems
Why? (can you elaborate briefly in the comment? (≤ 1 sentence))
conradludgate
changed the title
For #1213 I need the renewed certificate secret to be updated within the vm.
It's ok if there's some lag on this synchronisation as certificates are renewed with generous amount of time left until expiry.
Design:
neonvm-runner will subscribe to inotify events for a specific list of secrets. If an event is detected, the secret contents are sent in a http request to neonvm-daemon.
The files will be owned by the daemon user, and read-only by postgres.
If there's an error when uploading, then there's a secondary loop every 5 minutes to catch up, using a checksum to compare.
We mirror kubernetes and use atomicwriter to atomically update multiple files in the secret.
Useful links:
https://ahmet.im/blog/kubernetes-secret-volumes-delay/
https://ahmet.im/blog/kubernetes-inotify/
Some discussion here:
https://neondb.slack.com/archives/C03TN5G758R/p1737723555448339
https://neondb.slack.com/archives/C03TN5G758R/p1738669103059979