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

[reconfigurator] Fix DatasetsEditor internal zpool,kind -> ID map #7216

Merged
merged 5 commits into from
Dec 9, 2024

Conversation

jgallagher
Copy link
Contributor

Blueprint zones don't contain explicit dataset IDs (yet; see #7214), so DatasetsEditor maintained an internal cache mapping (zpool, kind) -> dataset ID. However, that mapping is only unique for in service datasets, and DatasetsEditor was erroneously trying to build it for all datasets (both in-service and expunged).

This PR adds a few property tests and fixes for the maintenance of this cache, and should ensure that we only try to maintain the "at most one dataset of a given kind on a given zpool" map for in-service datasets.

Blueprint zones don't contain explicit dataset IDs (yet; see #7214), so
`DatasetsEditor` maintained an internal cache mapping `(zpool, kind) ->
dataset ID`. However, that mapping is only unique for _in service_
datasets, and `DatasetsEditor` was erroneously trying to build it for
_all_ datasets (both in-service and expunged).

This PR adds a few property tests and fixes for the maintenance of this
cache, and should ensure that we only try to maintain the "at most one
dataset of a given kind on a given zpool" map for in-service datasets.
Copy link
Contributor

@andrewjstone andrewjstone left a comment

Choose a reason for hiding this comment

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

Great tests!

@jgallagher jgallagher merged commit 2568a2e into main Dec 9, 2024
17 checks passed
@jgallagher jgallagher deleted the john/fix-datasets-editor-zpool-kind-cache branch December 9, 2024 17:36
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.

2 participants