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

Blueprint execution: Add dataset records for Crucible zones #5143

Merged
merged 11 commits into from
Feb 27, 2024

Conversation

jgallagher
Copy link
Contributor

On each invocation, for every Crucible zone present in the blueprint (all of which have already successfully been sent to sled-agent by this point), we attempt to insert a dataset record for that zone, but only if a record does not already exist. The datasets themselves are created by sled-agent when we send the zone configs.

Fixes #5118.

Copy link
Collaborator

@davepacheco davepacheco left a comment

Choose a reason for hiding this comment

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

Nice! I recommend another review from someone more familiar how we use datasets.

nexus/db-queries/src/db/datastore/dataset.rs Outdated Show resolved Hide resolved
datastore: &DataStore,
all_omicron_zones: impl Iterator<Item = &OmicronZoneConfig>,
) -> anyhow::Result<usize> {
// Before attempting to insert any datasets, first query for any existing
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this behavior (and the comment).

///
/// Does not update existing rows. If a dataset with the given ID already
/// exists, returns `Ok(None)`.
pub async fn dataset_insert_if_not_exists(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like it'll fit in well with the existing "sled agent inserts dataset" structure, as we transition more towards "Nexus controls dataset creation".

filter_kind: Option<DatasetKind>,
) -> ListResultVec<Dataset> {
opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?;
opctx.check_complex_operations_allowed()?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

huh TIL about this opctx call, good to know!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's quite new (from #4989)!

}

#[cfg(test)]
mod test {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for all the tests in this PR!

nexus/db-queries/src/db/datastore/dataset.rs Outdated Show resolved Hide resolved
@jgallagher jgallagher merged commit 2e4287b into main Feb 27, 2024
21 checks passed
@jgallagher jgallagher deleted the john/blueprint-add-datasets branch February 27, 2024 21:27
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.

Reconfigurator needs to create datasets for new crucible zones
3 participants