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

Compatibility fix of centos release version #494

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

Compatibility fix of centos release version #494

wants to merge 1 commit into from

Conversation

Zhiwei-Dai
Copy link
Contributor

    modified:   ceph_deploy/hosts/centos/__init__.py

Fix: https://tracker.ceph.com/issues/44365

Signed-off-by: Dai Zhiwei daizhiwei3@huawei.com
Signed-off-by: luo rixin luorixin@huawei.com

@ceph-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@Zhiwei-Dai
Copy link
Contributor Author

@alfredodeza, i was wondering if u could review the pr, ths

@HacKanCuBa
Copy link
Contributor

Hello! Quick question: why are you printing the file error? I mean, what is the user supposed to do with that information?
On the other hand, would you consider using open as context manager?

PS: not a maintainer, just someone like you.

@Zhiwei-Dai
Copy link
Contributor Author

Ths your question.
when follow three 'if' have right 'return' type , the printing info just show a fact.
if can't return right type, the printing provides infos to enhance compatibility of derivative os release.
eg. Here is an Linux distribution os AOS 2.0 which is close to CentOS 7. if macros.dist opening error, we can make this file and define the macro like:

# dist macros.
%centos 7

Well, context manager is a nice choice.

Hello! Quick question: why are you printing the file error? I mean, what is the user supposed to do with that information?
On the other hand, would you consider using open as context manager?

PS: not a maintainer, just someone like you.

@HacKanCuBa
Copy link
Contributor

Well, another thing I forgot to ask was: whenever that file exists and has the string centos in a given line, how sure are we that splitting by spaces and converting to int whatever comes next to centos is going to be OK and not explode? As in, are we sure something like centos-something cool won't exist in that file?
My point is whether exists or not a convention over that file. If there isn't, then we might want to catch the ValueError of int(...) or even IndexError of release_info[1].

@Zhiwei-Dai
Copy link
Contributor Author

Usually, this file /etc/rpm/macros.dist is existing for rpm & distribution building. If not existing, maybe it is deleted by mistake.
Look this system file in centos7.6:

# dist macros.

%centos_ver 7
%centos 7
%rhel 7
%dist .el7
%el7 1

in euleros 2.0:

# dist macros.

%centos 7
%rhel 7
%dist %{nil}
%el7 1
%euleros 2

So, I mean different system files have different aspects. If something like centos-something cool exist in that file, we should discuss why it was here.

	modified:   ceph_deploy/hosts/centos/__init__.py

Fix: https://tracker.ceph.com/issues/44365

Signed-off-by: Dai Zhiwei daizhiwei3@huawei.com
Signed-off-by: luo rixin luorixin@huawei.com
@Zhiwei-Dai
Copy link
Contributor Author

@HacKanCuBa Well, when I switch to use context manager to open file, exception handling prompts me to catch the ValueError of int(...). Nice job.

@@ -18,6 +18,13 @@ def choose_init(module):

Returns the name of a init system (upstart, sysvinit ...).
"""
with open("/etc/rpm/macros.dist", 'r') as file:
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think we should rely on an rpm macro for determining the distro's release version. user does not necessarily have this package installed.

@alamintech
Copy link

I have some problem. I am used ceph storage for my CloudStack primary storage pool but after create vm then reboot ceph server osd service can't start. How can I solve this. this is ceph issue or cloudstack issue ? I am not sure. Please help me....Thanks.

@tchaikov
Copy link
Contributor

I have some problem. I am used ceph storage for my CloudStack primary storage pool but after create vm then reboot ceph server osd service can't start. How can I solve this. this is ceph issue or cloudstack issue ? I am not sure. Please help me....Thanks.

@alamintech you might want to send you questions to ceph-user mailing list or the #ceph IRC channel instead. see https://ceph.io/irc/

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