[Linux-ha-dev] [PATCH][crmsh] deal with the case-insentive hostname

Discussion:

Junko IKEDA

2013-04-10 09:13:45 UTC

Hi,
I set upper-case hostname (GUEST03/GUEST4) and run Pacemaker 1.1.9 +
Corosync 2.3.0.

[***@GUEST04 ~]# crm_mon -1
Last updated: Wed Apr 10 15:12:48 2013
Last change: Wed Apr 10 14:02:36 2013 via crmd on GUEST04
Stack: corosync
Current DC: GUEST04 (3232242817) - partition with quorum
Version: 1.1.9-e8caee8
2 Nodes configured, unknown expected votes
1 Resources configured.

Online: [ GUEST03 GUEST04 ]

dummy (ocf::pacemaker:Dummy): Started GUEST03

for example, call crm shell with lower-case hostname.

[***@GUEST04 ~]# crm node standby guest03
ERROR: bad lifetime: guest03

"crm node standby GUEST03" surely works well,
so crm shell just doesn't take into account the hostname conversion.
It's better to accept the both of the upper/lower-case.

"node standby", "node delete", "resource migrate(move)" get hit with this
issue.
Please see the attached.

Thanks,
Junko

Dejan Muhamedagic

2013-04-11 06:20:45 UTC

Permalink

Hi Junko-san,

This message looks awkward.

Post by Junko IKEDA
"crm node standby GUEST03" surely works well,
so crm shell just doesn't take into account the hostname conversion.
It's better to accept the both of the upper/lower-case.

Yes, indeed.

Post by Junko IKEDA
"node standby", "node delete", "resource migrate(move)" get hit with this
issue.
Please see the attached.

The patch looks correct. Many thanks for the contribution!

Cheers,

Dejan

Post by Junko IKEDA
Thanks,
Junko
_______________________________________________________
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Lars Ellenberg

2013-04-23 13:37:30 UTC

Permalink

Post by Junko IKEDA
Hi,
I set upper-case hostname (GUEST03/GUEST4) and run Pacemaker 1.1.9 +
Corosync 2.3.0.
Last updated: Wed Apr 10 15:12:48 2013
Last change: Wed Apr 10 14:02:36 2013 via crmd on GUEST04
Stack: corosync
Current DC: GUEST04 (3232242817) - partition with quorum
Version: 1.1.9-e8caee8
2 Nodes configured, unknown expected votes
1 Resources configured.
Online: [ GUEST03 GUEST04 ]
dummy (ocf::pacemaker:Dummy): Started GUEST03
for example, call crm shell with lower-case hostname.
ERROR: bad lifetime: guest03
"crm node standby GUEST03" surely works well,
so crm shell just doesn't take into account the hostname conversion.
It's better to accept the both of the upper/lower-case.
"node standby", "node delete", "resource migrate(move)" get hit with this
issue.
Please see the attached.
Thanks,
Junko

Sorry for the late reaction.

Post by Junko IKEDA
diff -r da93d3523e6a modules/ui.py.in
--- a/modules/ui.py.in Tue Mar 26 11:44:17 2013 +0100
+++ b/modules/ui.py.in Mon Apr 08 17:49:00 2013 +0900
@@ -924,10 +924,14 @@
lifetime = None
opt_l = fetch_opts(argl, ["force"])
- lifetime = argl[0]
- node = argl[0]
+ pattern = re.compile(i, re.IGNORECASE)
+ node = argl[1]

This is not exactly equivalent.

Before, we had a string comparison.
Now we have a regexp match.

This may be considered as a new feature.
But it should then be done intentionally.

Otherwise, "i" would need to be "quote-meta"ed first.
In Perl I'd write "\Q$i\E", in python we probably have to
insert some '\' into it first.

I admit in most setups it would not make any difference,
as there should at most be dots in there ".",
and they should be at places where they won't be ambiguous,
especially with the additional "len()" check.

Maybe rather compare argl[0].lower() with listnodes(), which
should also return all elements as .lower().

Lars

Dejan Muhamedagic

2013-04-23 14:16:40 UTC

Permalink

Hi Lars,

Post by Lars Ellenberg

Sorry for the late reaction.

This is not exactly equivalent.
Before, we had a string comparison.
Now we have a regexp match.
This may be considered as a new feature.
But it should then be done intentionally.
Otherwise, "i" would need to be "quote-meta"ed first.
In Perl I'd write "\Q$i\E", in python we probably have to
insert some '\' into it first.
I admit in most setups it would not make any difference,
as there should at most be dots in there ".",
and they should be at places where they won't be ambiguous,
especially with the additional "len()" check.
Maybe rather compare argl[0].lower() with listnodes(), which
should also return all elements as .lower().

Looks like I forgot about this patch, wanted to take a closer
look before applying, thanks for the analysis. There also seems
to be some code repetion, IIRC.

Cheers,

Dejan

Post by Lars Ellenberg
Lars
_______________________________________________________
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Dejan Muhamedagic

2013-04-23 14:44:19 UTC

Permalink

Hi Junko-san,

Can you try the attached patch, instead of this one?

Cheers,

Dejan

Dejan Muhamedagic

2013-05-29 09:38:01 UTC

Permalink

Post by Dejan Muhamedagic
Hi Junko-san,
Can you try the attached patch, instead of this one?

Any news? Was the patch any good?

Cheers,

Dejan

Post by Dejan Muhamedagic
Cheers,
Dejan

# HG changeset patch
# Date 1366728211 -7200
# Node ID cd4d36b347c17b06b76f3386c041947a03c708bb
# Parent 4a47465b1fe1f48123080b4336f0b4516d9264f6
Medium: node: ignore case when looking up nodes (thanks to Junko Ikeda)
diff -r 4a47465b1fe1 -r cd4d36b347c1 modules/ui.py.in
--- a/modules/ui.py.in Tue Apr 23 11:23:10 2013 +0200
+++ b/modules/ui.py.in Tue Apr 23 16:43:31 2013 +0200
lifetime = None
opt_l = fetch_opts(argl, ["force"])
lifetime = argl[0]
node = argl[0]
node = vars.this_node
node = vars.this_node
lifetime = args[0]
'usage: delete <node>'
return False
common_err("node %s not found in the CIB" % node)
return False
rc = True
diff -r 4a47465b1fe1 -r cd4d36b347c1 modules/xmlutil.py
--- a/modules/xmlutil.py Tue Apr 23 11:23:10 2013 +0200
+++ b/modules/xmlutil.py Tue Apr 23 16:43:31 2013 +0200
s2 = "%s:"%ra_provider
return ''.join((s1,s2,ra_type))
+ '''
+ Check if s is in a list of our nodes (ignore case).
+ This is not fast, perhaps should be cached.
+ '''
+ return True
+ return False
nodes_elem = cibdump2elem("nodes")
_______________________________________________________
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Junko IKEDA

2013-05-29 10:01:39 UTC

Permalink

Hi Dejan,

Sorry for no reply.
I tried this, and it works well!
http://hg.savannah.gnu.org/hgweb/crmsh/rev/1ebbf036c6d9

Many thanks for your review.

Thanks,
Junko

Post by Dejan Muhamedagic

Post by Dejan Muhamedagic
Hi Junko-san,
Can you try the attached patch, instead of this one?

Any news? Was the patch any good?
Cheers,
Dejan

Post by Dejan Muhamedagic
Cheers,
Dejan

this

Post by Dejan Muhamedagic

Post by Junko IKEDA
issue.
Please see the attached.
Thanks,
Junko
_______________________________________________________
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/