[DNM, WIP] feat: rework BD naming to avoid requiring WWNs#266
Draft
tserong wants to merge 10 commits intoharvester:masterfrom
Draft
[DNM, WIP] feat: rework BD naming to avoid requiring WWNs#266tserong wants to merge 10 commits intoharvester:masterfrom
tserong wants to merge 10 commits intoharvester:masterfrom
Conversation
283c11a to
8d79c78
Compare
…ices harvester#253 updated BD CRs to use stable /dev/mapper paths for multipath devices. These paths need to be resolved back to /dev/dm-0 when calling GetDiskByDevPath(), otherwise fetching details like the WWN and Serial number from /sys/block will fail, which means we lose the device WWN, SerialNumber and Capacity information when multipath devices are provisioned. Signed-off-by: Tim Serong <[email protected]>
TODO: Drop all the PartFilter stuff (should no longer apply) Signed-off-by: Tim Serong <[email protected]>
Signed-off-by: Tim Serong <[email protected]>
With debug logging turned on, you will periodically see something like the following: time="2026-02-19T09:55:30Z" level=debug msg="Prepare to checking block device ac1b8d0086729ec4840eac4e2319cfaf" time="2026-02-19T09:55:30Z" level=debug msg="Update block device ac1b8d0086729ec4840eac4e2319cfaf tags (Status) from [] to []" Note that it's triggering an update even though the tags don't appear to have changed (they're both rendered in the logs as empty slices). The problem is that bd.Status.Tags is specified as `json:"tags,omitempty"` but disk.Tags is specified as `json:"tags"`. This means that if both are empty, bd.Status.Tags will be nil, while disk.Tags will be an empty slice. In this case, reflect.DeepEqual() will return false, which will result in an unnecessary update. We can avoid this by skipping the DeepCopy and DeepEqual entirely if both bd.Status.Tags and disk.Tags are already empty. Signed-off-by: Tim Serong <[email protected]>
54c17c2 to
d6011c8
Compare
TODO: explain exactly WTF I've done here ;-) Related-to: harvester/harvester#6261 Signed-off-by: Tim Serong <[email protected]>
Signed-off-by: Tim Serong <[email protected]>
There's a couple of changes we need to make here: 1. We're actually starting with two blockdevices now, not one, because the new NDM code picks up /dev/vdb in the vagrant host 2. The duplicated WWN device needs a new backing file, otherwise it'll pick up the duplicate disk UUID, not the duplicate WWN. Signed-off-by: Tim Serong <[email protected]>
There'll be more than the two expected block devices thanks for NDM now picking up /dev/vdb. It's more robust to check how many we started with then expect to end up with two more than whatever that original number was. Signed-off-by: Tim Serong <[email protected]>
Signed-off-by: Tim Serong <[email protected]>
d6011c8 to
aa16be7
Compare
Signed-off-by: Tim Serong <[email protected]>
aa16be7 to
b800238
Compare
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:
We can't really cope properly with disks that don't have WWNs, or whose WWNs change (the latter should never happen, but it did at least once in the past when a kernel update resulted in IDs for a certain make and model of NVMe changing).
Solution:
TODO: explain WTF I'm doing here in more detail ;-)
Related Issue:
harvester/harvester#6261
Test plan:
TBD
NOTE:
I'm still working on this. Don't worry too much about reviewing it yet.