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

[RM-36583] align with create by using short hostname #478

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[RM-36583] align with create by using short hostname #478

wants to merge 1 commit into from

Conversation

changchengx
Copy link
Contributor

If using fully qualified domain name to check monitor
status after "mon add", it will hit the error that can't
find the node file under /var/run/ceph/ which result
in aboring add monitor

Signed-off-by: Changcheng Liu changcheng.liu@intel.com

If using fully qualified domain name to check monitor
status after "mon add", it will hit the error that can't
find the node file under /var/run/ceph/ which result
in aboring add monitor

Signed-off-by: Changcheng Liu <changcheng.liu@intel.com>
@ceph-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@changchengx
Copy link
Contributor Author

changchengx commented Oct 25, 2018

@dmick
When using "mon add" command, such as "ceph-deploy mon add nstcloudcc1.sh.intel.com",
It will check the monitor status with command " sudo ceph --cluster=ceph --admin-daemon /var/run/ceph/ceph-mon.nstcloudcc1.sh.intel.com.asok mon_status". However, the actual file name is "/var/run/ceph/ceph-mon.nstcloudcc1.asok". The reason is it use "fully qualified domain name" as part of the file name. Actually, it should use short hostname.

I've checked the "mon create" command, such as "ceph-deploy mon create nstcloudcc1.sh.intel.com", it use the short hostname to check the monitor status "sudo ceph --cluster=ceph --admin-daemon /var/run/ceph/ceph-mon.nstcloudcc1.asok mon_status" and succeed.

@dmick
Copy link
Member

dmick commented Oct 25, 2018

Yes, but why is the call to next() added, and why is the statement order changed?

@changchengx
Copy link
Contributor Author

changchengx commented Oct 25, 2018

@dmick

  1. The function mon_hosts is a yield generator. It need use "next" function to get the value.
  2. I'st better to use the same logic as "create" function: check the monitor status, then catch any error.

Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

@changchengx could you split this commit into two? one for using name returned by mon_hosts(), another for transposing mon_status() nad catch_mon_errors()?

@changchengx
Copy link
Contributor Author

@tchaikov
I'll check this PR within this week. It was filed about 2 years ago.

@tchaikov
Copy link
Contributor

tchaikov commented Sep 17, 2020

@tchaikov
I'll check this PR within this week. It was filed about 2 years ago.

the original maintainer does not work on this project anymore. guess that's why your PR was unattended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants