Repository navigation
Conversation
DeepEqual handed a slice of proto messages to reflect.DeepEqual, which compares each message's internal state as well as its fields. The RPC logger's marshal fills the size cache of a request's messages, so a repeated message field and its clone compared unequal, and declarative validation's unchanged-value shortcut then ran the field's update checks: an immutable repeated field failed every create, because the create validates the stored object as an update of the request. A slice of messages is now compared element by element with proto.Equal, a nil and an empty one being the same field value. Signed-off-by: Timo Derstappen <teemow@gmail.com>
Signed-off-by: Timo Derstappen <teemow@gmail.com>
Actors that work in one shared workspace need part of a volume that exists outside Substrate and outlives them: their own directory read-write and another directory of the same volume read-only. External volumes were created per actor, attached single-node-writer and deleted with the actor, and a mount had no sub-path or read-only flag. An ActorTemplate now declares an existing volume by name (Volume.existing_volume), and CreateActor supplies it per actor (Actor.existing_volumes: name, CSI driver, volume handle, access mode READ_WRITE_MANY or READ_ONLY_MANY, and the sub-path this actor sees as the volume's root). A VolumeMount gains sub_path and read_only, so one volume may be mounted at several paths. CreateActor refuses a reference to a volume the template does not declare as existing, a driver without a CSIDriverConfig, a handle no PersistentVolume of the driver holds and an access mode the PersistentVolume does not permit; the PersistentVolume's volume attributes are passed to the driver at resume. An existing volume the actor does not supply contributes neither the volume nor its mounts. Substrate never creates, deletes or detaches an existing volume: pause, resume and delete only unmount and mount it. A volume of a multi-node mode is staged once per target, so one actor's unmount never unstages it under another actor on the node. atelet binds each sub-path from a descriptor opened beneath the volume's root without following symbolic links, so a link another actor wrote on the volume fails the mount rather than redirecting it, and a missing directory fails it too. An actor with existing volumes boots from its image instead of the template's golden snapshot, which was captured without their mounts, and cannot be created from a tag. The existingvolumes e2e suite runs on kind with the CSI NFS driver. Signed-off-by: Timo Derstappen <teemow@gmail.com>
Records the existing-volume change, the upstream issue it follows and when it leaves the line. Signed-off-by: Timo Derstappen <teemow@gmail.com>
Signed-off-by: Timo Derstappen <teemow@gmail.com>
atelet runs without capabilities and sees the actors directory without mount propagation, so it neither sees the volume the CSI node plugin published there nor may mount: every sub-path read as missing. ateom, which composes the bundle rootfs for the same reason, now binds the directories a mount names a sub-path of, or mounts read-only, before the sandbox starts and releases them when it stops, on both runtimes (internal/volumebind). Signed-off-by: Timo Derstappen <teemow@gmail.com>
Member
Author
|
Written by an agent: a parallel implementation of this contract (unpushed, for reference) is the local worktree ~/projects/giantswarm/substrate-wt-227-refs, branch feat/actor-volume-refs at 046ec015. |
5 of 6 tasks
Member
Author
|
Replaced by #234: the same tree, with the feature as one commit for this rebase-merged line. Written by an agent. |
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
Actors that work in one shared workspace need part of a volume that exists outside Substrate and outlives them: their own directory read-write, and another directory of the same volume read-only. On this line an external volume is created per actor, attached single-node-writer and deleted with the actor; a
VolumeMounthas no sub-path and no read-only flag; anActorcarries no volume of its own.Proposed solution
The shape proposed upstream in agent-substrate#1637 (agent-substrate#1637 (comment)), on top of the access modes of agent-substrate#1988:
Template:
Volume.existing_volume(an empty marker) declares a volume each actor supplies. Mounts of it may set the newVolumeMount.sub_pathandread_only. Templates still refuse a mount of an undeclared volume, andsub_path/read_onlyon any other kind of volume.Actor:
CreateActortakesActor.existing_volumes(immutable). EachExistingVolumehas a name, a CSI driver, a volume handle, an access mode (READ_WRITE_MANYorREAD_ONLY_MANY, the shape of volume: support configurable access modes agent-substrate/substrate#1988'sVolumeAccessMode), and asub_path: the directory this actor sees as the volume's root. Template mount sub-paths are relative to it, so one template serves every actor, each in a directory of its own (sessions/<id>read-write,mirrorsread-only, both from one handle).Refused at create, with the reason:
CSIDriverConfig;ate-api-server reads PersistentVolumes (a new ClusterRole rule). The PersistentVolume's
volumeAttributesreach the driver at resume.Unsupplied volumes: an existing volume the actor does not supply contributes neither the volume nor its mounts. The create request of such an actor is unchanged.
Lifecycle: Substrate never creates, deletes or detaches an existing volume (other actors on the node may be using it). Pause, resume (on another node too) and delete only unmount and mount it.
CSI: a multi-node volume is published with
MULTI_NODE_MULTI_WRITERorMULTI_NODE_READER_ONLY. It is staged once per target, so one actor's unmount never unstages it under another actor on the node.Sub-paths on the node: ateom (which sees the volume the CSI node plugin published and may mount; atelet may do neither) binds each sub-path, on both runtimes (
internal/volumebind), before the sandbox starts and releases it when the sandbox stops. It binds from a descriptor opened beneath the volume's root withopenat2(RESOLVE_BENEATH | RESOLVE_NO_SYMLINKS | RESOLVE_NO_XDEV), then remounts it read-only where asked. A symbolic link another actor wrote on the volume therefore fails the mount instead of redirecting it, and so does a missing directory: nothing is auto-created. The OCI spec binds the per-mount directory, read-only when asked. On the microvm runtime the bind is made before the volumes directory is shared into the guest, so the guest sees it too.Snapshots and tags: an actor with existing volumes boots from its image instead of restoring the template's golden snapshot, which was captured without their mounts. It cannot be created from a tag, since a tag does not record the mounts its guest state was captured with.
Gate: upstream puts this behind the Preview gate of Add a "preview gate" concept, use it for external volumes agent-substrate/substrate#1994, which this line does not carry, so it is ungated here.
Commits:
FORK.mdrow with the upstream issue and the exit. The repeated-message equality fix is carried as its own commit with its own row.resources.DeepEqualcompared repeated message fields withreflect.DeepEqual, so an immutable repeated field (hereexisting_volumes) failed everyCreateActorwith "field is immutable".Design notes:
ValidateVolumeCapabilities(the NFS CSI driver does), so the check is the cluster's own record: a PersistentVolume of the driver with that handle, permitting the access mode.NodeUnstageVolumetouches only its own staging. Existing volumes are neverControllerUnpublishVolumed. Neither needs a per-node count of the actors using the volume.VolumeAccessModeenum is mirrored. Its own prerequisites (CSI volume support: follow-up improvements agent-substrate/substrate#1729, csi: add publishContext to actor volumes agent-substrate/substrate#1578) are therefore not needed here.Replaces #231 (the snapshot-seeded per-actor volume, superseded by this design).
Acceptance criteria
sessions/aread-write andmirrorsread-only at the declared paths (TestExistingVolumes/MountsSubPathsAtDeclaredPaths)ActorsWriteOnlyTheirOwnSubPath)PauseResumeDeleteLeaveTheVolume)TestWorkloadSpecExistingVolumes/without_a_reference_the_request_is_unchanged; e2eNoReferenceNoMountsTestValidateExistingVolumes; e2eRefusesBadReferencesAtCreateFORK.mdrow naming the upstream issue it follows; released as a line release candidateCloses #227.
This pull request was written by an agent.