Skip to content

Conversation

@leftwo
Copy link
Contributor

@leftwo leftwo commented Jan 11, 2023

When we create a disk, the saga generates both a disk_id and a volume_id

        builder.append(Node::action(
            "disk_id",
            "GenerateDiskId",
            ACTION_GENERATE_ID.as_ref(),
        ));
       
        builder.append(Node::action(
            "volume_id",
            "GenerateVolumeId",
            ACTION_GENERATE_ID.as_ref(),
        ));

We use both ids when we create the disk database record:

    let disk = db::model::Disk::new(
        disk_id,
        params.project_id,
        volume_id,
        params.create_params.clone(),
        block_size,
        db::model::DiskRuntimeState::new(),
    )

However, the volume_id here is only used as the key to store the volume data
in the database. That volume_id is never exposed to anyone outside Nexus.

For the actual volume data structure and later the VolumeConstructionRequest,
we use the value in the disk_id, but call it volume_id

This is nice because it means the volume_id that propolis receives in the VCR
will match what the console has for a disk uuid, and it means stats will be
recorded under the same UUID.

However, Propolis (who needs to call Nexus and tell it to remove a read only parent
only has the top level disk UUID. The internal "volume_id" is not exposed.

This PR adds a new internal API endpoint that will take the disk UUID, then lookup
the volume_id and then call the volume layer remove read only parent saga. Adding
a layer like this also allows tests that operate only on a volume level to not require
any changes.

Created a new internal API endpoint that allows a caller to remove
a read only parent from a volume if they only know the UUID of the
disk associated with that volume.  This is basically just a wrapper
that takes a disk UUID, extracts the volume_id from that disk, then
calls the volume saga to remove the read only parent.
@leftwo leftwo requested a review from smklein January 11, 2023 23:55
@leftwo leftwo merged commit 081c4b9 into main Jan 13, 2023
@leftwo leftwo deleted the alan/volume_id_is_not_disk_id branch January 13, 2023 16:55
smklein pushed a commit that referenced this pull request Jan 18, 2023
…2157)

When we create a disk, the saga generates both a disk_id and a
volume_id We use both ids when we create the disk database record
However, the `volume_id` here is only used as the key to store the
volume data in the database. That volume_id is never exposed to
anyone outside Nexus.

For the actual volume data structure and later the 
VolumeConstructionRequest, we use the value in the disk_id, but 
call it volume_id.

However, Propolis (who needs to call Nexus and tell it to remove a read
only parent only has the top level disk UUID. The internal volume_id is not
exposed.

This adds a new internal API endpoint that will take the disk UUID,
then lookup the volume_id and then call the volume layer remove 
read only parent saga. Adding a layer like this also allows tests that
operate only on a volume level to not require any changes.

Co-authored-by: Alan Hanson <alan@oxide.computer>
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.

3 participants