Repository navigation
fix(volumebind): create a missing read-write sub_path at the bind - #239
Merged
Merged
Conversation
A caller that names the actor's directory inside its own create call (a session id chosen at create, the directory sessions/<id> of a shared workspace volume) could not make the directory before the actor existed, so every such actor failed at its first resume with "sub_path does not exist on the volume". ateom now creates the last component of a read-write mount's sub_path when it is missing, with mkdirat beneath the parent opened the way the bind opens a sub-path (RESOLVE_BENEATH, RESOLVE_NO_SYMLINKS, RESOLVE_NO_XDEV). The directory belongs to the user the mounting container runs as, read from its OCI bundle, and is 0770 whatever ateom's umask, so a container that runs as its image's USER can write it. The parent must exist, since an actor is given a directory of its own and not a tree. A symbolic link in the path, a file under the name, a read-only mount, a READ_ONLY_MANY volume and a read-only file system keep failing with their reason, and every creation happens before any bind, so a read-only mount of the directory a read-write mount creates finds it whatever the order of the mounts. MountDir moves to ocispec, which volumebind now imports for the bundle's spec. A template repoint no longer requires the existing-volume declarations and their mounts to be unchanged, the way system_info volumes are exempt: their data lives outside Substrate, the actor supplies them at create or not at all, and no snapshot carries their content. Adding an existing-volume slot to a template therefore repoints the template's actors instead of failing every one with FailedPrecondition. The existingvolumes e2e now leaves one session directory for Substrate to create and checks its mode. Signed-off-by: Timo Derstappen <teemow@gmail.com>
Signed-off-by: Timo Derstappen <teemow@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Since the existing-volume mounts (#234),
CreateActortakes asub_pathper existing volume, but the bind refuses one that does not exist yet (internal/volumebind, "sub_path does not exist on the volume"). A caller that assigns the directory name inside its own create call, kagent'sCreateSessionfor one (it picks the Session id and mountssessions/<id>/of the workspace volume), cannot create the directory before the actor exists, so every such actor fails at its first resume.Beside it,
validateTemplateVolumesUnchangedtreated an existing-volume declaration like a durable volume: adding an existing-volume slot to a template made every actor's repointFailedPrecondition, although such a volume holds no snapshot data.Proposed solution
sub_path's last component is missing, ateom'svolumebind.Preparecreates it:mkdiratbeneath the parent, which is opened the way the bind resolves a sub-path (RESOLVE_BENEATH | RESOLVE_NO_SYMLINKS | RESOLVE_NO_XDEV), then owned by the user the mounting container runs as (read from its OCI bundle, so an image that runs as a non-rootUSERcan write it) and0770whatever ateom's umask. The parent must exist: an actor is given a directory of its own, not a tree. A missing parent, a symbolic link in the path, a file under the name, a read-only mount, aREAD_ONLY_MANYvolume and a read-only file system keep failing with their reason. Every creation runs before any bind, so a read-only mount of a directory a read-write mount creates finds it whatever the order of the mounts.MountDirmoves fromvolumebindtoocispec, whichvolumebindnow imports for the bundle's spec (the other direction was a cycle).system_infovolumes: the actor supplies them at create or not at all, and an actor without a reference to a newly declared one gets no mount of it.docs/csi-volumes.mdand theVolumeMount.sub_pathcomment say so; theexistingvolumese2e leaves sessiona's directory for Substrate to create and checks its mode, sessionb's stays made ahead.Upstream: follows agent-substrate#1637's shape like #234; the same change is prepared for upstream once that lands.
Acceptance criteria
CreateActorwith an existingREAD_WRITE_MANYvolume and asub_pathwhose last component is missing creates it (0770, the container's user) and binds it; a missing parent, a symlink in the path, a file, an unwritable parent and a path outside the volume are refused with the reason:TestCreateVolumeDir(eleven cases),TestProcessUser,TestBoundMountsTestExistingVolumes/MountsSubPathsAtDeclaredPathsPASS at head 754f595 on the e2e-test lane (https://github.com/giantswarm/substrate/actions/runs/37979928552/job/113987543465) and both runtimes of the E2E job (https://github.com/giantswarm/substrate/actions/runs/37979928804/job/113987745031); the unit tests inrun-testsof the same runFailedPrecondition:TestValidateTemplateVolumesUnchanged(six new cases),TestUpdateActor_RepointTemplate(tmpl-g)FORK.mdrow (754f595,fork-ledgergreen); released as v1.7.0-rc.2 (tag at the rebase merge 8e4fb7e; images and charts on gsoci, CircleCI pipeline 556)Closes #238.
This pull request was written by an agent.