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

Fix additional mdadm parsing cases #346

Merged
merged 4 commits into from
Nov 17, 2016

Conversation

mcdan
Copy link
Contributor

@mcdan mcdan commented Nov 16, 2016

Rebased correctly, couldn't reopen PR so this duplicates closed PR #312

@SuperQ
Copy link
Member

SuperQ commented Nov 16, 2016

Of course, there's now a merge conflict. :-(

@mcdan
Copy link
Contributor Author

mcdan commented Nov 16, 2016

Working on it. This is cause two fixes for the super 1.2x token are going in, so no surprise. Rebasing my branch and fixing conflicts.

@mcdan mcdan force-pushed the people/mcdan/issues/219 branch from 525e349 to 1f6b5ae Compare November 16, 2016 19:51
@SuperQ SuperQ changed the title Fixes #219 Fix additional mdadm parsing cases Nov 17, 2016
@SuperQ
Copy link
Member

SuperQ commented Nov 17, 2016

Updated the PR title to be more in line with our changelog naming style. For linking reference, this fixes #219

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM

statuslineRE = regexp.MustCompile(`(\d+) blocks .*\[(\d+)/(\d+)\] \[[U_]+\]`)
raid0lineRE = regexp.MustCompile(`(\d+) blocks( super ([0-9\.])*)? \d+k chunks`)
buildlineRE = regexp.MustCompile(`\((\d+)/\d+\)`)
unknownPersonalityLine = regexp.MustCompile(`(\d+) blocks (.*)`)

Choose a reason for hiding this comment

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

Missing RE suffix

for _, possiblePersonality := range mainLine {
if raidPersonalityRE.MatchString(possiblePersonality) {
personality = possiblePersonality
// break

Choose a reason for hiding this comment

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

Are multiple personalities supported? are we just aiming to match the last?
Either uncomment the break, or remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think a raid array can have many personalities. Today the code isn't really setup to handle that either, it determines the personality of array and then uses one of the other REs to locate the data it wants. I think this break got juggled around with the merge conflicts, I'm going to add it back in.

@@ -158,19 +175,28 @@ func parseMdstat(mdStatusFilePath string) ([]mdStatus, error) {
}
currentMD = mainLine[0] // The name of the md-device.
isActive := (mainLine[2] == "active") // The activity status of the md-device.
personality = mainLine[3] // The personality type of the md-device.
personality = ""
for _, possiblePersonality := range mainLine {

Choose a reason for hiding this comment

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

range mainLine[3:] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes good catch, the personality should only be in the 3rd item

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're correct, good catch.

@SuperQ SuperQ merged commit f8af350 into prometheus:master Nov 17, 2016
@SuperQ SuperQ mentioned this pull request Nov 25, 2016
tamcore pushed a commit to gitgrave/node_exporter that referenced this pull request Oct 22, 2024
Signed-off-by: Wisniewski, Krzysztof2 <Krzysztof2.Wisniewski@intel.com>
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.

3 participants