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 ipmi-user-list catalog data #523

Merged
merged 1 commit into from
Apr 12, 2018

Conversation

gavin-scott
Copy link
Contributor

Code that parsed sudo ipmitool -c user list N assumed there
would be a header row printed and built the user hash keys from
the first line. However ipmitool user list only prints the
headers when invoked without -c which puts it into CSV output
mode. When invoked with -c there is no header line and the
first user was getting eaten up and treated as the header.

Example ipmitool output:

monorail@monorail-micro:~$ sudo ipmitool user list 1
ID  Name             Callin  Link Auth  IPMI Msg   Channel Priv Limit
2   root             true    true       true       ADMINISTRATOR
monorail@monorail-micro:~$ sudo ipmitool user -c list 1
2,root,true,true,true,ADMINISTRATOR

This fix hard-codes the header values as the header that is
printed when invoked without -c looks nigh-impossible to
parse. The previous code was also eating the first user because
that line was taken as the header line.

@gavin-scott gavin-scott force-pushed the ipmi-user-list branch 2 times, most recently from b6c9b67 to b16603c Compare October 11, 2017 21:14
@RackHDCI
Copy link

RackHDCI commented Nov 1, 2017

The Copyright Check has failed!
The files listed below do not have a valid copyright statement:

  • spec/lib/utils/job-utils/stdout-helper.js

@gavin-scott
Copy link
Contributor Author

@geoff-reid , @keedya any changes needed here? The copyright check failed on a a pre-existing file, but I can update that if you want.

@lanchongyizu
Copy link
Member

@gavin-scott could you fix the Copyright failure?

@RackHDCI
Copy link

The Copyright Check has failed!
The files listed below do not have a valid copyright statement:

  • spec/lib/utils/job-utils/stdout-helper.js

@gavin-scott gavin-scott force-pushed the ipmi-user-list branch 3 times, most recently from 78f7cd6 to 82dd5ec Compare March 17, 2018 15:06
@gavin-scott
Copy link
Contributor Author

@lanchongyizu sorry for delay, I added the copyright message to stdout-helper.js now. Looks like concourse CI failed though. I don't think I have access to that to check the issue.

@lanchongyizu
Copy link
Member

@gavin-scott Sorry for the delay. The concourse fail case is as below, which I think is caused by concourse instability. Could you push a new commit to trigger the concourse again?

23:52:16
======================================================================
23:52:16
FAIL: test09_check_discovery (test.deploy.rackhd_stack_init.rackhd_stack_init)
23:52:16
----------------------------------------------------------------------
23:52:16
Traceback (most recent call last):
23:52:16
  File "/tmp/build/a9eb6f47/RackHD/test/deploy/rackhd_stack_init.py", line 199, in test09_check_discovery
23:52:16
    self.assertTrue(self.check_for_active_workflows(MAX_CYCLES), "Node discovery not completed")
23:52:16
AssertionError: Node discovery not completed

Code that parsed `sudo ipmitool -c user list N` assumed there
would be a header row printed and built the user hash keys from
the first line. However `ipmitool user list` only prints the
headers when invoked without `-c` which puts it into CSV output
mode. When invoked with `-c` there is no header line and the
first user was getting eaten up and treated as the header.

Example ipmitool output:

    monorail@monorail-micro:~$ sudo ipmitool user list 1
    ID  Name             Callin  Link Auth  IPMI Msg   Channel Priv Limit
    2   root             true    true       true       ADMINISTRATOR
    monorail@monorail-micro:~$ sudo ipmitool user -c list 1
    2,root,true,true,true,ADMINISTRATOR

This fix hard-codes the header values as the header that is
printed when invoked without `-c` looks nigh-impossible to
parse. The previous code was also eating the first user because
that line was taken as the header line.
@gavin-scott
Copy link
Contributor Author

Sure @lanchongyizu -- just did.

@lanchongyizu lanchongyizu merged commit 32f2bea into RackHD:master Apr 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants