Skip to content

Handle varying output formats for rbd status #454

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 23, 2017
Merged

Handle varying output formats for rbd status #454

merged 1 commit into from
Mar 23, 2017

Conversation

@codenrhoden
Copy link
Contributor

@codenrhoden codenrhoden commented Mar 14, 2017

The output for rbd status --format json has been updated recently
to have the "watchers" field be an array instead of a map. Account
for that.

Fixes #451

@codecov-io
Copy link

codecov-io commented Mar 14, 2017 •
edited
Loading

Codecov Report

Merging #454 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #454      +/-   ##
==========================================
- Coverage   30.61%   30.55%   -0.06%     
==========================================
  Files          29       29              
  Lines        1777     1777              
==========================================
- Hits          544      543       -1     
- Misses       1175     1176       +1     
  Partials       58       58
Impacted Files Coverage Δ
api/types/types_localdevices.go 77.35% <0%> (-1.89%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c1beda...544afc0. Read the comment docs.

@codenrhoden codenrhoden added this to the 2017.04 milestone Mar 21, 2017
akutz
akutz requested changes Mar 22, 2017
View reviewed changes
drivers/storage/rbd/storage/rbd_storage.go Outdated
}

// Init initializes the driver.
func (d *driver) Init(context types.Context, config gofig.Config) error {
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Since this PR needs to be rebased onto master anyway, can you please fix the name of the context variable to be ctx. It's the standard way of naming the first parameter of a function when it's of type Context. Thank you.

drivers/storage/rbd/utils/utils.go Outdated

cmd := exec.Command(radosCmd, "lspools")
log.Debugf("running command: %v", cmd.Args)
ctx.Debugf("running command: %v", cmd.Args)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead:

ctx.WithFields(map[string]interface{}{
	"cmd":  radosCmd,
	"args": cmd.Args,
}).Debug("running rados cmd")

drivers/storage/rbd/utils/utils.go Outdated
if exiterr, ok := err.(*exec.ExitError); ok {
stderr := string(exiterr.Stderr)
log.Errorf("Unable to get pools: %s", stderr)
ctx.Errorf("Unable to get pools: %s", stderr)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead (formatting aside):

ctx.WithError(
	exiterr,
).WithField(
	"stderr", stderr,
).Error("error getting pools")

drivers/storage/rbd/utils/utils.go Outdated
if exiterr, ok := err.(*exec.ExitError); ok {
stderr := string(exiterr.Stderr)
log.Errorf("Unable to get rbd images: %s", stderr)
ctx.Errorf("Unable to get rbd images: %s", stderr)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead (formatting aside):

ctx.WithError(
	exiterr,
).WithField(
	"stderr", stderr,
).Error("error getting rbd images")

drivers/storage/rbd/utils/utils.go Outdated
rbdCmd, "info", "-p", *pool, *name, formatOpt, jsonArg)

log.Debugf("running command: %v", cmd.Args)
ctx.Debugf("running command: %v", cmd.Args)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead:

ctx.WithFields(map[string]interface{}{
	"cmd":  rbdCmd,
	"args": cmd.Args,
}).Debug("running rados cmd")

drivers/storage/rbd/utils/utils.go Outdated
if exiterr, ok := err.(*exec.ExitError); ok {
stderr := string(exiterr.Stderr)
log.Errorf("Unable to map RBD: %s", stderr)
ctx.Errorf("Unable to map RBD: %s", stderr)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead (formatting aside):

ctx.WithError(
	exiterr,
).WithField(
	"stderr", stderr,
).Error("error mapping rbd")

drivers/storage/rbd/utils/utils.go Outdated
if exiterr, ok := err.(*exec.ExitError); ok {
stderr := string(exiterr.Stderr)
log.Errorf("Unable to unmap RBD: %s", stderr)
ctx.Errorf("Unable to unmap RBD: %s", stderr)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead (formatting aside):

ctx.WithError(
	exiterr,
).WithField(
	"stderr", stderr,
).Error("error unmapping rbd")

drivers/storage/rbd/utils/utils.go Outdated
if exiterr, ok := err.(*exec.ExitError); ok {
stderr := string(exiterr.Stderr)
log.Errorf("Unable to get RBD status: %s", stderr)
ctx.Errorf("Unable to get RBD status: %s", stderr)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead (formatting aside):

ctx.WithError(
	exiterr,
).WithField(
	"stderr", stderr,
).Error("error getting rbd status")

drivers/storage/rbd/utils/utils.go Outdated

cmd := exec.Command(rbdCmd, "unmap", *device)
log.Debugf("running command: %v", cmd.Args)
ctx.Debugf("running command: %v", cmd.Args)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead:

ctx.WithFields(map[string]interface{}{
	"cmd":  rbdCmd,
	"args": cmd.Args,
}).Debug("running rados cmd")

drivers/storage/rbd/utils/utils.go Outdated
rbdCmd, "status", poolOpt, *pool, *image, formatOpt, jsonArg,
)
log.Debugf("running command: %v", cmd.Args)
ctx.Debugf("running command: %v", cmd.Args)
Copy link
Collaborator

@akutz akutz Mar 22, 2017

Choose a reason for hiding this comment

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

Hi @codenrhoden,

Please use this instead:

ctx.WithFields(map[string]interface{}{
	"cmd":  rbdCmd,
	"args": cmd.Args,
}).Debug("running rados cmd")

@codenrhoden
Handle varying output formats for rbd status
544afc0 
The output for rbd status --format json has been updated recently
to have the "watchers" field be an array instead of a map. Account
for that.
@codenrhoden codenrhoden force-pushed the bugfix/ceph_parse_watchers branch from da8e8e4 to 544afc0 Compare March 23, 2017 17:57
@codenrhoden
Copy link
Contributor Author

codenrhoden commented Mar 23, 2017

Thanks for the guidance on better structured logging, @akutz. A definite improvement. I think I've made all the requested changes.

@akutz akutz merged commit efd3a91 into thecodeteam:master Mar 23, 2017
@akutz akutz removed the in progress label Mar 23, 2017
@codenrhoden codenrhoden deleted the bugfix/ceph_parse_watchers branch April 11, 2017 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akutz akutz akutz requested changes

Assignees

No one assigned

Labels

rbd

Projects

None yet

Milestone

2017.04

Development

Successfully merging this pull request may close these issues.

3 participants

@codenrhoden @codecov-io @akutz