oxidized
oxidized copied to clipboard
ytti
Create hpebladevcmodul.rb
@STetzel Thanks for the PR!
This model seems to be a subset of the hpebladesystem.rb model that is currently in the repository.
Could you provide some context as to why this new model is needed? Have you attempted to run the existing hpebladesystem.rb model against your equipment? Both models appear to target HPE's Virtual Connect product line.
@wk Sorry about my late reply. No it is not a subset. The model hpebladecenter backuped the OA modules (management and configuration). But the VC (Virtual Connect) modules are for the network configuration with so-called Flex10 network cards. They are separate modules with their own configuration.
@STetzel Can you take a look at the failed tests please.
You will also need to add this model to https://github.com/ytti/oxidized/blob/master/docs/Supported-OS-Types.md and also update the changelog as per the top post.
Codecov Report
Merging #1342 into master will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## master #1342 +/- ##
=======================================
Coverage 63.32% 63.32%
=======================================
Files 30 30
Lines 1475 1475
=======================================
Hits 934 934
Misses 541 541
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 765c108...8880f4d. Read the comment docs.
@STetzel Can you add this model to the docs as per my last comment as well please?
@STetzel thanks for working on this further; the intention of the request by @laf was that you introduce these additional changes as part of this PR by appending them as additional commits to STetzel:master. You seem to have opened two additional PRs instead, which I've closed as they are not stand-alone PRs that require independent evaluation.
This looks like a welcome addition to the model collection, but for the sake of completeness please:
- Apply the changes from #1544 and $1545 to this PR instead as additional commits
- Consider if perhaps a more suitable name can be found for this model that's concise, complete, and relevant (perhaps
hpebladevcm?) or attempt to identify the internal name of the operating system on these devices so that the model can follow that name (rather than a specific device name / model).
This looks good to go in principle, the only remaining question is the name.
Could you take a moment to clarify as to why you've chosen to shorten "module" to "modul" in both the filename and OS Type? What is the actual exact product name for this as the vendor itself describes it? Is there a reason the proposed "hpebladevcm" is inadequate?
Since "modul" is not an English word, it'd be great if we could arrive at something that's intuitive now - since renaming anything later will require inconveniencing users who already have the name in their configuration.
Product name is "Virtual Connect" (https://www.hpe.com/us/en/product-catalog/servers/virtual-connect.hits-12.html) that said: show network command is not useful imho, it is included in config
i would do:
show domain
show firmware *
show enclosure *
show server *
show interconnect *
show config -includePoolInfo
the * in the commands above change the output format to extended, so it does not wrap item names the -includePoolInfo is useful so the configuration includes the auto-assigned wwn from the pool else they are hidden in the output
@lberra That sounds reasonable; would you consider:
- Taking this PR and renaming the model to
hpbladevc.rb - Implementing the suggested changes
- Testing it against real hardware and documenting the hardware and firmware version in a comment at the top of the module (you can see other modules for an example of this)
- Submitting this as a new PR (or as a pull against @STetzel fork if he concurs with these changes)
@STetzel, what are your thoughts on the suggestions of @lberra above?
