Hi Dejan,
On Tue, 2011-03-08 at 19:26 +0100, Holger Teutsch wrote:
> Hi Dejan,
>
> On Tue, 2011-03-08 at 12:07 +0100, Dejan Muhamedagic wrote:
> > Hi Holger,
> >
> > On Tue, Mar 08, 2011 at 09:15:01AM +0100, Holger Teutsch wrote:
> > > On Fri, 2011-03-04 at 13:06 +0100, Holger Teutsch wrote:
> > > > On Thu, 2011-03-03 at 10:55 +0100, Florian Haas wrote:
> > > > > On 2011-03-03 10:43, Holger Teutsch wrote:
> > > > > > Hi,
> > > > > > I submit a patch for
> > > > > > "bugzilla 2541: Shell should warn if parameter uniqueness is
> > > > > > violated"
> > > > > > for discussion.
> > > > >
> ...
> > It looks good, just a few notes. The check function should move
> > to the CibObjectSetRaw class and be invoked from
>
> Will move it there.
>
> > semantic_check(). There's
> >
> > rc1 = set_obj_verify.verify()
> > if user_prefs.check_frequency != "never":
> > rc2 = set_obj_semantic.semantic_check()
> >
> > The last should be changed to:
> >
> > rc2 = set_obj_semantic.semantic_check(set_obj_verify)
> >
> > set_obj_verify always contains all CIB elements (well, that means
> > that its name should probably be changed too :). Now, the code
> > should check _only_ new and changed primitives which are
> > contained in set_obj_semantic. That's because we don't want to
> > repeatedly print warnings for all objects on commit, but only for
> > those which were added/changed in the meantime. On the other
> > hand, verify is an explicit check and in that case the whole CIB
> > is always verified.
> >
> > >
> > > + ra_class = prim.getAttribute("class")
> > > + ra_provider = prim.getAttribute("provider")
> > > + ra_type = prim.getAttribute("type")
> > > + ra_id = prim.getAttribute("id")
> > > +
> > > + ra = RAInfo(ra_class, ra_type, ra_provider)
> >
> > There's a convenience function get_ra(node) for this.
> >
>
> I did not use this as I need all ra_XXX value anyhow later in the code
> for building k.
>
> > > + if ra == None:
> > > + return
> > > + ra_params = ra.params()
> > > +
> > > + attributes = prim.getElementsByTagName("instance_attributes")
> > > + if len(attributes) == 0:
> > > + return
> > > +
> > > + for p in attributes[0].getElementsByTagName("nvpair"):
> > > + name = p.getAttribute("name")
> > > + if ra_params[ name ]['unique'] == '1':
> > > + value = p.getAttribute("value")
> > > + k = (ra_class, ra_provider, ra_type, name, value)
> > > + try:
> > > + clash_dict[k].append(ra_id)
> > > + except:
> > > + clash_dict[k] = [ra_id]
> > > + return
> > > +
> > > + clash_dict = {}
> > > + for p in cib_factory.mkobj_list("xml","type:primitive"):
> >
> > This would become:
> >
> > for p in all_obj_list: # passed from _verify()
> > if is_primitive(p.node):
> >
> > > + process_primitive(p.node, clash_dict)
> >
> > Or perhaps to loop through self.obj_list and build clash_dict
> > against all elements? Otherwise, you'll need to skip elements
> > which don't pass the check but are not new/changed (in
> > self.obj_list).
> >
>
I did not pass "set_obj_verify" in semantic check as this variable "only
by chance" contains the right values.
- holger
Output:
crm(live)configure# primitive ip1a ocf:heartbeat:IPaddr2 params ip="1.2.3.4"
meta target-role="stopped"
crm(live)configure# primitive ip1b ocf:heartbeat:IPaddr2 params ip="1.2.3.4"
meta target-role="stopped"
crm(live)configure# commit
WARNING: Resources ip1a,ip1b violate uniqueness for parameter "ip": "1.2.3.4"
Do you still want to commit? y
crm(live)configure# primitive ip2a ocf:heartbeat:IPaddr2 params ip="1.2.3.5"
meta target-role="stopped"
crm(live)configure# commit
crm(live)configure# primitive ip2b ocf:heartbeat:IPaddr2 params ip="1.2.3.5"
meta target-role="stopped"
crm(live)configure# primitive ip3 ocf:heartbeat:IPaddr2 params ip="1.2.3.6"
meta target-role="stopped"
crm(live)configure# commit
WARNING: Resources ip2a,ip2b violate uniqueness for parameter "ip": "1.2.3.5"
Do you still want to commit? y
crm(live)configure# primitive dummy_1 ocf:heartbeat:Dummy params fake="abc"
meta target-role="stopped"
crm(live)configure# primitive dummy_2 ocf:heartbeat:Dummy params fake="abc"
meta target-role="stopped"
crm(live)configure# primitive dummy_3 ocf:heartbeat:Dummy meta
target-role="stopped"
crm(live)configure# commit
crm(live)configure# verify
WARNING: Resources ip1a,ip1b violate uniqueness for parameter "ip": "1.2.3.4"
WARNING: Resources ip2a,ip2b violate uniqueness for parameter "ip": "1.2.3.5"
crm(live)configure#
diff -r a35d8d6d0ab1 shell/modules/cibconfig.py
--- a/shell/modules/cibconfig.py Wed Mar 09 11:21:03 2011 +0100
+++ b/shell/modules/cibconfig.py Wed Mar 09 13:20:14 2011 +0100
@@ -230,11 +230,66 @@
See below for specific implementations.
'''
pass
+
+ def __check_unique_clash(self):
+ 'Check whether resource parameters with attribute "unique" clash'
+
+ def process_primitive(prim, clash_dict):
+ '''
+ Update dict clash_dict with
+ (ra_class, ra_provider, ra_type, name, value) -> [ resourcename ]
+ if parameter "name" should be unique
+ '''
+ ra_class = prim.getAttribute("class")
+ ra_provider = prim.getAttribute("provider")
+ ra_type = prim.getAttribute("type")
+ ra_id = prim.getAttribute("id")
+
+ ra = RAInfo(ra_class, ra_type, ra_provider)
+ if ra == None:
+ return
+ ra_params = ra.params()
+
+ attributes = prim.getElementsByTagName("instance_attributes")
+ if len(attributes) == 0:
+ return
+
+ for p in attributes[0].getElementsByTagName("nvpair"):
+ name = p.getAttribute("name")
+ if ra_params[ name ]['unique'] == '1':
+ value = p.getAttribute("value")
+ k = (ra_class, ra_provider, ra_type, name, value)
+ try:
+ clash_dict[k].append(ra_id)
+ except:
+ clash_dict[k] = [ra_id]
+ return
+
+ # we check the whole CIB for clashes as a clash may originate between
+ # an object already committed and a new one
+ clash_dict = {}
+ for p in cib_factory.mkobj_list("xml","type:primitive"):
+ process_primitive(p.node, clash_dict)
+
+ # but we only warn if a 'new' object is involved
+ check_set = set([o.node.getAttribute("id") for o in self.obj_list if is_primitive(o.node)])
+
+ rc = 0
+ for param, resources in clash_dict.items():
+ # at least one new object must be involved
+ if len(resources) > 1 and len(set(resources) & check_set) > 0:
+ rc = 2
+ msg = 'Resources %s violate uniqueness for parameter "%s": "%s"' %\
+ (",".join(sorted(resources)), param[3], param[4])
+ common_warning(msg)
+
+ return rc
+
def semantic_check(self):
'''
Test objects for sanity. This is about semantics.
'''
- rc = 0
+ rc = self.__check_unique_clash()
for obj in self.obj_list:
rc |= obj.check_sanity()
return rc
_______________________________________________
Pacemaker mailing list: Pacemaker@oss.clusterlabs.org
http://oss.clusterlabs.org/mailman/listinfo/pacemaker
Project Home: http://www.clusterlabs.org
Getting started: http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
Bugs: http://developerbugs.linux-foundation.org/enter_bug.cgi?product=Pacemaker
