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

mgr/rook: remove dead code and fix bug in url fetching code #26032

Merged
merged 2 commits into from
Jan 22, 2019

Conversation

jtlayton
Copy link
Contributor

@jtlayton jtlayton commented Jan 18, 2019

This deletes some dead code in the rook module, and also adds handling in "service ls" for the case where the cephnfs object is no longer present but the pod is.

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

John evidently added this code early on in development, when we figured
we might be taking over a cluster that is already running under k8s.

I don't see how we'd ever make that work, so just remove this function
for now, and the commented-out callsite.

Reported-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Copy link
Contributor

@sebastian-philipp sebastian-philipp left a comment

Choose a reason for hiding this comment

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

Code that is never executed doesn't work. LGTM

@sebastian-philipp
Copy link
Contributor

found by #26028

It's possible for the fetch of this object to fail if the service is
being torn down. Handle that situation gracefully, but log a message.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
@jtlayton jtlayton changed the title mgr/rook: remove init_rook mgr/rook: remove dead code and fix bug in url fetching code Jan 18, 2019
@sebastian-philipp sebastian-philipp merged commit 116d53d into ceph:master Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants