Capability collector #60
Merged
Conversation
This file contains 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
This interface defines methods to be implemented by utilities to collect Drive capabilities.
- Capabilities seemed a more apt name for this and so the methods and the types were renamed, the shared Capability type is included in bmc-toolbox/common - DriveFeatures renamed to DriveCapabilities - The DriveCapabilities methods returns a []*common.Capabilities type - Pass in context before command execution - Lower case methods that are not exported
…the OS/kernel drive name
psteinbachs
reviewed
Oct 27, 2022
psteinbachs
previously approved these changes
Oct 27, 2022
psteinbachs
previously approved these changes
Oct 27, 2022
….go device This allows the mock fakelsblk to be included in the inventory tests
Various other data fixes based changes on fixes to vetChanges()
This is required for the the dell provider tests and the actions tests to be able to run mock fake lsblk, nvme, hdparm executor, we need to come up with a better way than specifying the relative path to the fixture.
At the moment this code is limited to checking capabilities on SATA and NVME drives.
This is done, since the lsblk fixture data in fixtures/utils/lsblk/json corelates to the drives in the Dell fixture - the serial numbers match. And since the lsblk fixture does not include drives that match the SMC fixture serials, the capabilities data is removed from the SMC fixture.
joelrebel
force-pushed
the
capability-collector
branch
from
d5d627d to
a70a29a
Compare
|
The final set of changes are -
|
psteinbachs
approved these changes
Oct 28, 2022
splaspood
approved these changes
Nov 1, 2022
There was a problem hiding this comment.
Nothing jumped out at me as being obviously wrong or bad form. I noticed us hard coding some specific capability names in utils/nvme.go and wonder if maybe that should be moved into something common, but I didn't feel it was something that would hold up approval.
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.
What does this PR do
The
actions.Collectinventory method now returns drive capabilities.Based on @psteinbachs work to add
hdparm,nvmedrive feature collection,this change set defines and implements the
DriveCapabilities()interface.Device.Commonstruct was extended to include[]*Capabilitiesthat are thenreturned by the
hdparm,nvmedrive capability collectors.Capabilitiesseemed a more apt name instead ofFeaturestheDriveCapabilitymethods in thenvme,hparmwere renamed and a[]*common.Capabilityis returned.nvme,hdparmcommand execution.This change depends on bmc-toolbox/common#4