Bug 1561617 - crm_diff shows diffs even when it shouldn't
Summary: crm_diff shows diffs even when it shouldn't
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: pacemaker
Version: 7.5
Hardware: All
OS: Linux
urgent
urgent
Target Milestone: rc
: 7.6
Assignee: Ken Gaillot
QA Contact: Patrik Hagara
URL:
Whiteboard:
Depends On:
Blocks: 1568720
TreeView+ depends on / blocked
 
Reported: 2018-03-28 15:28 UTC by Michele Baldessari
Modified: 2018-10-30 07:59 UTC (History)
8 users (show)

Fixed In Version: pacemaker-1.1.18-12.el7
Doc Type: Bug Fix
Doc Text:
Cause: When comparing two CIB inputs, the crm_diff tool always reported differences in the order of XML attributes. Consequence: Changes that do not affect the operation of the cluster would always be reported. Fix: When using the --cib option, attribute order differences will not be reported as a change. Result: The user can now choose whether all differences or only significant differences are reported as a change.
Clone Of:
: 1568720 (view as bug list)
Environment:
Last Closed: 2018-10-30 07:57:56 UTC
Target Upstream Version:


Attachments (Terms of Use)
original unchanged cib (152.28 KB, text/plain)
2018-03-28 15:29 UTC, Michele Baldessari
no flags Details
cib with a noop update applied to it (152.34 KB, text/plain)
2018-03-28 15:30 UTC, Michele Baldessari
no flags Details


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 1568353 None CLOSED "pcs resource update" should not create an empty meta-attributes element 2019-11-04 14:21:39 UTC
Red Hat Product Errata RHBA-2018:3055 None None None 2018-10-30 07:59:04 UTC

Internal Links: 1568353

Description Michele Baldessari 2018-03-28 15:28:57 UTC
Description of problem:
We're exploring using crm_diff to understand if a resource has changed or not, so that puppet can be a bit smarter when updating cluster resources (today we basically never update a pacemaker resource in puppet)

So if I had an IP resource that was created with :
pcs resource create ip-172.17.1.15 cidr_netmask=32 ip=172.17.1.15

And it correctly runs:
Online: [ controller-0 controller-1 controller-2 ]
RemoteOnline: [ novacomputeiha-0 ]
GuestOnline: [ galera-bundle-0@controller-1 galera-bundle-1@controller-2 galera-bundle-2@controller-0 rabbitmq-bundle-0@controller-1 rabbitmq-bundle-1@controller-2 rabbitmq-bundle-2@controller-0 redis-bundle-0@controller-1 redis-bundle-1@controller-2 redis-bundle-2@controller-0 ]

Full list of resources:

 novacomputeiha-0       (ocf::pacemaker:remote):        Started controller-0
 Docker container set: rabbitmq-bundle [192.168.24.1:8787/rhosp13/openstack-rabbitmq:pcmklatest]
   rabbitmq-bundle-0    (ocf::heartbeat:rabbitmq-cluster):      Started controller-1
   rabbitmq-bundle-1    (ocf::heartbeat:rabbitmq-cluster):      Started controller-2
   rabbitmq-bundle-2    (ocf::heartbeat:rabbitmq-cluster):      Started controller-0
 Docker container set: galera-bundle [192.168.24.1:8787/rhosp13/openstack-mariadb:pcmklatest]
   galera-bundle-0      (ocf::heartbeat:galera):        Master controller-1
   galera-bundle-1      (ocf::heartbeat:galera):        Master controller-2
   galera-bundle-2      (ocf::heartbeat:galera):        Master controller-0
 Docker container set: redis-bundle [192.168.24.1:8787/rhosp13/openstack-redis:pcmklatest]
   redis-bundle-0       (ocf::heartbeat:redis): Master controller-1
   redis-bundle-1       (ocf::heartbeat:redis): Slave controller-2
   redis-bundle-2       (ocf::heartbeat:redis): Slave controller-0
 ip-192.168.24.13       (ocf::heartbeat:IPaddr2):       Started controller-1
 ip-10.0.0.101  (ocf::heartbeat:IPaddr2):       Started controller-2
 ip-172.17.1.15 (ocf::heartbeat:IPaddr2):       Started controller-0
 ip-172.17.1.18 (ocf::heartbeat:IPaddr2):       Started controller-1
 ip-172.17.3.12 (ocf::heartbeat:IPaddr2):       Started controller-2
 ip-172.17.4.16 (ocf::heartbeat:IPaddr2):       Started controller-0
 Docker container set: haproxy-bundle [192.168.24.1:8787/rhosp13/openstack-haproxy:pcmklatest]
   haproxy-bundle-docker-0      (ocf::heartbeat:docker):        Started controller-1
   haproxy-bundle-docker-1      (ocf::heartbeat:docker):        Started controller-2
   haproxy-bundle-docker-2      (ocf::heartbeat:docker):        Started controller-0
 stonith-fence_compute-fence-nova       (stonith:fence_compute):        Started controller-1
 Clone Set: compute-unfence-trigger-clone [compute-unfence-trigger]
     Started: [ novacomputeiha-0 ]
     Stopped: [ controller-0 controller-1 controller-2 ]
 nova-evacuate  (ocf::openstack:NovaEvacuate):  Started controller-2
 Docker container: openstack-cinder-volume [192.168.24.1:8787/rhosp13/openstack-cinder-volume:pcmklatest]
   openstack-cinder-volume-docker-0     (ocf::heartbeat:docker):        Started controller-0

Daemon Status:
  corosync: active/enabled
  pacemaker: active/enabled
  pcsd: active/enabled

Now I do the following:
# Dump the CIB and make a copy of the file
pcs cluster cib cib.xml
cp cib.xml cib-orig.xml
# Amend cib.xml with a noop *i.e. we don't change the resource in any way*
pcs -f cib.xml resource update ip-172.17.1.15 cidr_netmask=32 ip=172.17.1.15

# Yet crm_diff gives a truckload of changes (bundles and empty metadata on the VIP):
[root@controller-0 ~]# crm_diff -u --original cib-orig.xml --new cib.xml 
<diff format="2">
  <change operation="modify" path="/cib/configuration/resources/bundle[@id=&apos;galera-bundle&apos;]/storage/storage-mapping[@id=&apos;mysql-foo&apos;]">
    <change-list>
      <change-attr name="id" operation="set" value="mysql-foo"/>
      <change-attr name="target-dir" operation="set" value="/foo"/>
    </change-list>
    <change-result>
      <storage-mapping id="mysql-foo" options="rw" source-dir="/foo" target-dir="/foo"/>
    </change-result>
  </change>
  <change operation="modify" path="/cib/configuration/resources/bundle[@id=&apos;galera-bundle&apos;]/storage/storage-mapping[@id=&apos;mysql-bar&apos;]">
    <change-list>
      <change-attr name="id" operation="set" value="mysql-bar"/>
      <change-attr name="target-dir" operation="set" value="/bar"/>
    </change-list>
    <change-result>
      <storage-mapping id="mysql-bar" options="rw" source-dir="/bar" target-dir="/bar"/>
    </change-result>
  </change>
  <change operation="create" path="/cib/configuration/resources/primitive[@id=&apos;ip-172.17.1.15&apos;]" position="2">
    <meta_attributes id="ip-172.17.1.15-meta_attributes"/>
  </change>
</diff>


Version-Release number of selected component (if applicable):
[root@controller-0 ~]# rpm -q pacemaker pcs
pacemaker-1.1.18-11.el7.x86_64
pcs-0.9.162-5.el7.x86_64


Expected results:
crm_diff in this case should be empty and return no changes.

Comment 2 Michele Baldessari 2018-03-28 15:29:32 UTC
Created attachment 1414291 [details]
original unchanged cib

Comment 3 Michele Baldessari 2018-03-28 15:30:01 UTC
Created attachment 1414293 [details]
cib with a noop update applied to it

Comment 5 Andrew Beekhof 2018-04-15 10:41:30 UTC
With https://github.com/ClusterLabs/pacemaker/commit/42dc2bb

[08:38 PM] beekhof@minidell ~/sources/pacemaker/upstream ☹  # PCMK_trace_functions=__xml_diff_object tools/crm_diff -o input1.xml -n input2.xml -Vc
(       xml.c:4209  )    info: __xml_diff_object:	nvpair.nodes-3-haproxy-role moved from 4 to 3
(       xml.c:4209  )    info: __xml_diff_object:	nvpair.nodes-3-redis-role moved from 3 to 4
<diff format="2" digest="a49a3002b8a2c872d3f0c6d1a09c528d">
  <version>
    <source admin_epoch="0" epoch="104" num_updates="12"/>
    <target admin_epoch="0" epoch="104" num_updates="12"/>
  </version>
  <change operation="move" path="/cib/configuration/nodes/node[@id=&apos;3&apos;]/instance_attributes[@id=&apos;nodes-3&apos;]/nvpair[@id=&apos;nodes-3-haproxy-role&apos;]" position="3"/>
  <change operation="move" path="/cib/configuration/nodes/node[@id=&apos;3&apos;]/instance_attributes[@id=&apos;nodes-3&apos;]/nvpair[@id=&apos;nodes-3-redis-role&apos;]" position="4"/>
</diff>

 : git : ✗ master@f73f955 Test: rhbz#1565187 - Ensure failures that cause fencing are not removed until after fencing completes
[08:38 PM] beekhof@minidell ~/sources/pacemaker/upstream ☹  # PCMK_trace_functions=__xml_diff_object tools/crm_diff -o input1.xml -n input2.xml -V
(       xml.c:4126  )    info: __xml_diff_object:	Moved nvpair@id (0 -> 1)
(       xml.c:4126  )    info: __xml_diff_object:	Moved nvpair@name (1 -> 0)
(       xml.c:4209  )    info: __xml_diff_object:	nvpair.nodes-3-haproxy-role moved from 4 to 3
(       xml.c:4209  )    info: __xml_diff_object:	nvpair.nodes-3-redis-role moved from 3 to 4
<diff format="2">
  <version>
    <source admin_epoch="0" epoch="104" num_updates="12"/>
    <target admin_epoch="0" epoch="104" num_updates="12"/>
  </version>
  <change operation="modify" path="/cib/configuration/nodes/node[@id=&apos;2&apos;]/instance_attributes[@id=&apos;nodes-2&apos;]/nvpair[@id=&apos;nodes-2-haproxy-role&apos;]">
    <change-list>
      <change-attr name="name" operation="set" value="haproxy-role"/>
      <change-attr name="id" operation="set" value="nodes-2-haproxy-role"/>
    </change-list>
    <change-result>
      <nvpair name="haproxy-role" id="nodes-2-haproxy-role" value="true"/>
    </change-result>
  </change>
  <change operation="move" path="/cib/configuration/nodes/node[@id=&apos;3&apos;]/instance_attributes[@id=&apos;nodes-3&apos;]/nvpair[@id=&apos;nodes-3-haproxy-role&apos;]" position="3"/>
  <change operation="move" path="/cib/configuration/nodes/node[@id=&apos;3&apos;]/instance_attributes[@id=&apos;nodes-3&apos;]/nvpair[@id=&apos;nodes-3-redis-role&apos;]" position="4"/>
</diff>

Comment 6 Andrew Beekhof 2018-04-15 10:43:57 UTC
Turns out I explicitly wanted attribute ordering changes detected in https://github.com/ClusterLabs/pacemaker/commit/5125ecbdf, so that behaviour is retained everywhere except crm_diff and even then only when comparing with -c or --cib

Comment 7 Jan Pokorný [poki] 2018-04-16 11:45:39 UTC
What's so appealing on a library feature above and beyond what
the standard mandates?  This also means not working on plain XML
basis (with all of implications of that model) anymore.
Original model has other means of working with order-sensitivity
where desired.

https://www.w3.org/TR/2008/REC-xml-20081126/#sec-starttags

> Note that the order of attribute specifications in a start-tag
> or empty-element tag is not significant.

I can see that deterministic/preserving serialization/deserialization
is very useful in some contexts (e.g. for libxslt), but when the
question stands "are these XML instances equivalent", the original
model has a clear answer about the isomorphism on attributes
(just as it has about whitespace related differences etc.).

Comment 8 Michele Baldessari 2018-04-16 14:52:50 UTC
So I built a pacemaker package and installed it:
pacemaker-1.1.18.crm_diff.0-11.el7.x86_64

Definitely an improvement:
pcs cluster cib cib.xml
cp cib.xml cib.xml.orig
# This below is effectively a noop:
pcs -f cib.xml resource update ip-172.16.11.97 cidr_netmask=32

[root@foobar-0 crm_diff_tests]# crm_diff --cib --original cib.xml.orig --new cib.xml
<diff format="2" digest="e9eb1ba31a86c41a406e86f79bcad861">
  <version>
    <source admin_epoch="0" epoch="28" num_updates="13"/>
    <target admin_epoch="0" epoch="29" num_updates="0"/>
  </version>
  <change operation="modify" path="/cib">
    <change-list>
      <change-attr name="epoch" operation="set" value="29"/>
      <change-attr name="num_updates" operation="set" value="0"/>
    </change-list>
    <change-result>
      <cib crm_feature_set="3.0.14" validate-with="pacemaker-2.10" epoch="29" num_updates="0" admin_epoch="0" cib-last-written="Mon Apr 16 16:41:38 2018" update-origin="foobar-1" update-client="crmd" update-user="hacluster" have-quorum="1" dc-uuid="2"/>
    </change-result>
  </change>
  <change operation="create" path="/cib/configuration/resources/primitive[@id=&apos;ip-172.16.11.97&apos;]" position="2">
    <meta_attributes id="ip-172.16.11.97-meta_attributes"/>
  </change>
</diff>


I wonder if we can avoid crm_diff signaling that the empty meta attributes tag ha been added:
[root@foobar-0 crm_diff_tests]# diff -u cib.xml.orig cib.xml
--- cib.xml.orig        2018-04-16 16:42:32.997958929 +0200
+++ cib.xml     2018-04-16 16:42:50.590301314 +0200
@@ -1,4 +1,4 @@
-<cib crm_feature_set="3.0.14" validate-with="pacemaker-2.10" epoch="28" num_updates="13" admin_epoch="0" cib-last-written="Mon Apr 16 16:41:38 2018" update-origin="foobar-1" update-client="crmd" update-user="hacluster" have-quorum="1" dc-uuid="2">
+<cib crm_feature_set="3.0.14" validate-with="pacemaker-2.10" epoch="29" num_updates="0" admin_epoch="0" cib-last-written="Mon Apr 16 16:41:38 2018" update-origin="foobar-1" update-client="crmd" update-user="hacluster" have-quorum="1" dc-uuid="2">
   <configuration>
     <crm_config>
       <cluster_property_set id="cib-bootstrap-options">
@@ -84,6 +84,7 @@
           <op id="ip-172.16.11.97-start-interval-0s" interval="0s" name="start" timeout="20s"/>
           <op id="ip-172.16.11.97-stop-interval-0s" interval="0s" name="stop" timeout="20s"/>
         </operations>
+        <meta_attributes id="ip-172.16.11.97-meta_attributes"/>
       </primitive>
       <clone id="rabbitmq-clone">
         <primitive class="ocf" id="rabbitmq" provider="heartbeat" type="rabbitmq-cluster">


If not I'll have to add some code to ignore empty tags (although I can't predict yet how many and which ones are relevant and which aren't)

Comment 9 Ken Gaillot 2018-04-16 18:30:10 UTC
Do we need a 7.5.z for this?

Comment 10 Andrew Beekhof 2018-04-17 03:35:00 UTC
(In reply to Michele Baldessari from comment #8)
> I wonder if we can avoid crm_diff signaling that the empty meta attributes
> tag ha been added:

Nope :)

> If not I'll have to add some code to ignore empty tags (although I can't
> predict yet how many and which ones are relevant and which aren't)

Seems like something pcs could avoid creating in the first place.

Comment 11 Andrew Beekhof 2018-04-17 03:35:20 UTC
(In reply to Ken Gaillot from comment #9)
> Do we need a 7.5.z for this?

Yes please

Comment 12 Andrew Beekhof 2018-04-17 03:36:49 UTC
(In reply to Jan Pokorný from comment #7)
> What's so appealing on a library feature above and beyond what
> the standard mandates? 

Because the cib comparision and digest algorithms are built on it.
Do we have your permission now?

Comment 13 Michele Baldessari 2018-04-17 06:32:00 UTC
(In reply to Andrew Beekhof from comment #10)
> (In reply to Michele Baldessari from comment #8)
> > I wonder if we can avoid crm_diff signaling that the empty meta attributes
> > tag ha been added:
> 
> Nope :)

Slacker ;)

> > If not I'll have to add some code to ignore empty tags (although I can't
> > predict yet how many and which ones are relevant and which aren't)
> 
> Seems like something pcs could avoid creating in the first place.

Right, but I still need to care for that case because a) there might be other occurrences where pcs might do this and we might not slay them all and b) I'd have to conditionalize code on is pcs running a fixed version -> user crm_diff, otherwise use crm_simulate, which would become a pita to maintain and test.

Let me explore this on the ruby side of things and let's see how it goes.
Thanks a lot for the fix in the meantime!

Comment 15 Tomas Jelinek 2018-04-17 11:00:33 UTC
(In reply to Michele Baldessari from comment #13)
> (In reply to Andrew Beekhof from comment #10)
> > (In reply to Michele Baldessari from comment #8)
> > > I wonder if we can avoid crm_diff signaling that the empty meta attributes
> > > tag ha been added:
> > 
> > Nope :)
> 
> Slacker ;)
> 
> > > If not I'll have to add some code to ignore empty tags (although I can't
> > > predict yet how many and which ones are relevant and which aren't)
> > 
> > Seems like something pcs could avoid creating in the first place.

This happens in old pcs code at several places I think. In new code (new pcs library) we take care not to leave empty elements in the CIB. Creating an empty meta_attributes on resource update was easy to fix. So here you are: bz1568353

> Right, but I still need to care for that case because a) there might be
> other occurrences where pcs might do this and we might not slay them all and

Yes, there probably are a few more.

> b) I'd have to conditionalize code on is pcs running a fixed version -> user
> crm_diff, otherwise use crm_simulate, which would become a pita to maintain
> and test.
> 
> Let me explore this on the ruby side of things and let's see how it goes.
> Thanks a lot for the fix in the meantime!

Comment 17 Ken Gaillot 2018-04-18 16:14:57 UTC
(In reply to Jan Pokorný from comment #7)
> What's so appealing on a library feature above and beyond what
> the standard mandates?  This also means not working on plain XML
> basis (with all of implications of that model) anymore.
> Original model has other means of working with order-sensitivity
> where desired.
> 
> https://www.w3.org/TR/2008/REC-xml-20081126/#sec-starttags
> 
> > Note that the order of attribute specifications in a start-tag
> > or empty-element tag is not significant.
> 
> I can see that deterministic/preserving serialization/deserialization
> is very useful in some contexts (e.g. for libxslt), but when the
> question stands "are these XML instances equivalent", the original
> model has a clear answer about the isomorphism on attributes
> (just as it has about whitespace related differences etc.).

crm_diff is a CIB tool, not an XML tool, i.e. it operates at a higher level than pure XML. There are two useful questions it can be used to answer:

The first is: Given two CIB inputs, what changes are necessary to apply to the second one to make it identical to the first one, as verified by a hash of the contents? For the hash verification, the order of XML attributes is consequential, even if it is not semantically.

The second is: Given two CIB inputs, are they functionally equivalent? In this case, the order of XML attributes is not consequential.

The fix here is to allow the user to choose via the --cib option. I'm not sure it's the best option to overload that, but it maintains backward compatibility and works for the necessary purpose.

Comment 18 Jan Pokorný [poki] 2018-04-19 11:34:36 UTC
re [comment 18]:

> crm_diff is a CIB tool, not an XML tool, i.e. it operates
> at a higher level than pure XML.

This is exactly why CIB would ideally _not_ break the assumptions
set forth with the lower level (in the natural order of things,
CIB would not attempt to be superset of XML, overridding its natural
axioms, when it's conceptually its subset) :-)


re [comment 12]:

> Because the cib comparison and digest algorithms are built on it.

That's not the root cause I was after.  I can presume the answer
is "because it was _easy_ and without quadratic complexity at the
attribute level comparison".  Well, it's not necessarily quadratic:

For elements A, B of lenghts m, n, respectively:

* for each element, iterate through the attributes, for each attribute,
  compute (not necessarily cryptographic) hash of "<name>=<value>"
  (for instance), XOR it to the accumulated value for the particular
  element
  - complexity:
    m * ("mean length of "<name>=<value>" in A" + "constant for XOR") \
    + n * (<mean length of "<name>=<value>" in B + "constant for XOR")

* compare the resulting hashes
  - complexity: constant

-> if we, conservatively, deem "(mean length of "<name>=<value>" in X)"
   is 64, we get overall complexity:
   (64 + "constant for XOR") * (m + n) + "constant for comparison",
   and because constants are less significant order of the complexity
   growth, it can be neglected: m + n, i.e., linear complexity
   (see above, having the luxury of being _subset_ of XML, CIB could
   arbitrarily impose a limit of either name/value being 64 characters
   at maximum to guarantee predictable timing/avoid DoS'ing when
   processing CIB)

The overall complexity would further decrease if the hashes were
maintained throughout the life scope of the XML object systemically.

Digest algorithm could then directly leverage the already precomputed
hashes of the attribute sets, moreover this precomputation could be
leveraged in bottom-up level, i.e., at the root, aggregate precomputed
(or ad-hoc recursively computed) hashes of its subtrees.

Such a design could also prevent headaches with sensitive data
contributing directly to the digest computation, [bug 1164861 comment 6],
but then, said hash function would have to be cryptographically strong
so as to avoid bruteforcing pre-image (omitted raw passwords) easily.


> Do we have your permission now?

Anytime, barring a possibility of a time machine, apparently I cannot
revert the history :-)

Comment 20 Patrik Hagara 2018-07-16 13:20:54 UTC
before:
=======

> [root@virt-242 ~]# rpm -q pacemaker pcs
> pacemaker-1.1.18-11.el7.x86_64
> pcs-0.9.165-1.el7.x86_64
> [root@virt-242 ~]# wget -qO cib.xml https://bugzilla.redhat.com/attachment.cgi?id=1414291
> [root@virt-242 ~]# sha1sum cib.xml
> 58e12006c0facee54fedd5c57c63f62a986183d5  cib.xml
> [root@virt-242 ~]# cp cib.xml cib.xml.orig
> [root@virt-242 ~]# pcs -f cib.xml resource update ip-172.17.1.15 cidr_netmask=32 ip=172.17.1.15   
> [root@virt-242 ~]# crm_diff --cib --original cib.xml.orig --new cib.xml
> <diff format="2" digest="b64d3000a4d06df03baf6e769fbdeeb2">
>   <version>
>     <source admin_epoch="0" epoch="86" num_updates="125"/>
>     <target admin_epoch="0" epoch="87" num_updates="0"/>
>   </version>
>   <change operation="modify" path="/cib">
>     <change-list>
>       <change-attr name="epoch" operation="set" value="87"/>
>       <change-attr name="num_updates" operation="set" value="0"/>
>     </change-list>
>     <change-result>
>       <cib crm_feature_set="3.0.14" validate-with="pacemaker-2.10" epoch="87" num_updates="0" admin_epoch="0" cib-last-written="Tue Mar 27 08:12:24 2018" update-origin="controller-0" update-client="cibadmin" update-user="root" have-quorum="1" dc-uuid="2"/>
>     </change-result>
>   </change>
>   <change operation="modify" path="/cib/configuration/resources/bundle[@id=&apos;galera-bundle&apos;]/storage/storage-mapping[@id=&apos;mysql-foo&apos;]">
>     <change-list>
>       <change-attr name="id" operation="set" value="mysql-foo"/>
>       <change-attr name="target-dir" operation="set" value="/foo"/>
>     </change-list>
>     <change-result>
>       <storage-mapping id="mysql-foo" options="rw" source-dir="/foo" target-dir="/foo"/>
>     </change-result>
>   </change>
>   <change operation="modify" path="/cib/configuration/resources/bundle[@id=&apos;galera-bundle&apos;]/storage/storage-mapping[@id=&apos;mysql-bar&apos;]">
>     <change-list>
>       <change-attr name="id" operation="set" value="mysql-bar"/>
>       <change-attr name="target-dir" operation="set" value="/bar"/>
>     </change-list>
>     <change-result>
>       <storage-mapping id="mysql-bar" options="rw" source-dir="/bar" target-dir="/bar"/>
>     </change-result>
>   </change>
> </diff>
> 
> [root@virt-242 ~]# echo $?
> 1
> [root@virt-242 ~]# crm_diff --no-version --original cib.xml.orig --new cib.xml
> <diff format="2">
>   <change operation="modify" path="/cib/configuration/resources/bundle[@id=&apos;galera-bundle&apos;]/storage/storage-mapping[@id=&apos;mysql-foo&apos;]">
>     <change-list>
>       <change-attr name="id" operation="set" value="mysql-foo"/>
>       <change-attr name="target-dir" operation="set" value="/foo"/>
>     </change-list>
>     <change-result>
>       <storage-mapping id="mysql-foo" options="rw" source-dir="/foo" target-dir="/foo"/>
>     </change-result>
>   </change>
>   <change operation="modify" path="/cib/configuration/resources/bundle[@id=&apos;galera-bundle&apos;]/storage/storage-mapping[@id=&apos;mysql-bar&apos;]">
>     <change-list>
>       <change-attr name="id" operation="set" value="mysql-bar"/>
>       <change-attr name="target-dir" operation="set" value="/bar"/>
>     </change-list>
>     <change-result>
>       <storage-mapping id="mysql-bar" options="rw" source-dir="/bar" target-dir="/bar"/>
>     </change-result>
>   </change>
> </diff>
> 
> [root@virt-242 ~]# echo $?
> 1



after:
======

> [root@virt-241 ~]# rpm -q pacemaker pcs
> pacemaker-1.1.19-3.el7.x86_64
> pcs-0.9.165-1.el7.x86_64
> [root@virt-241 ~]# wget -qO cib.xml https://bugzilla.redhat.com/attachment.cgi?id=1414291
> [root@virt-241 ~]# sha1sum cib.xml
> 58e12006c0facee54fedd5c57c63f62a986183d5  cib.xml
> [root@virt-241 ~]# cp cib.xml cib.xml.orig
> [root@virt-241 ~]# pcs -f cib.xml resource update ip-172.17.1.15 cidr_netmask=32 ip=172.17.1.15
> [root@virt-241 ~]# crm_diff --cib --original cib.xml.orig --new cib.xml
> <diff format="2" digest="b64d3000a4d06df03baf6e769fbdeeb2">
>   <version>
>     <source admin_epoch="0" epoch="86" num_updates="125"/>
>     <target admin_epoch="0" epoch="87" num_updates="0"/>
>   </version>
>   <change operation="modify" path="/cib">
>     <change-list>
>       <change-attr name="epoch" operation="set" value="87"/>
>       <change-attr name="num_updates" operation="set" value="0"/>
>     </change-list>
>     <change-result>
>       <cib crm_feature_set="3.0.14" validate-with="pacemaker-2.10" epoch="87" num_updates="0" admin_epoch="0" cib-last-written="Tue Mar 27 08:12:24 2018" update-origin="controller-0" update-client="cibadmin" update-user="root" have-quorum="1" dc-uuid="2"/>
>     </change-result>
>   </change>
> </diff>
> 
> [root@virt-241 ~]# echo $?                                             
> 1
> [root@virt-241 ~]# crm_diff --no-version --original cib.xml.orig --new cib.xml
> <diff format="2">
>   <change operation="modify" path="/cib/configuration/resources/bundle[@id=&apos;galera-bundle&apos;]/storage/storage-mapping[@id=&apos;mysql-foo&apos;]">
>     <change-list>
>       <change-attr name="id" operation="set" value="mysql-foo"/>
>       <change-attr name="target-dir" operation="set" value="/foo"/>
>     </change-list>
>     <change-result>
>       <storage-mapping id="mysql-foo" options="rw" source-dir="/foo" target-dir="/foo"/>
>     </change-result>
>   </change>
>   <change operation="modify" path="/cib/configuration/resources/bundle[@id=&apos;galera-bundle&apos;]/storage/storage-mapping[@id=&apos;mysql-bar&apos;]">
>     <change-list>
>       <change-attr name="id" operation="set" value="mysql-bar"/>
>       <change-attr name="target-dir" operation="set" value="/bar"/>
>     </change-list>
>     <change-result>
>       <storage-mapping id="mysql-bar" options="rw" source-dir="/bar" target-dir="/bar"/>
>     </change-result>
>   </change>
> </diff>
> 
> [root@virt-241 ~]# echo $?
> 1


Passing the '-c/--cib' flag to crm_diff now shows less differences (effectively, only the expected version/epoch metadata changes) between original cib and cib with a no-op resource update applied.
The output with '-u/--no-version' remains unchanged, as different diffing logic is used when this flag is passed.
Please note that the missing empty meta attribute change before the fix as compared to the original bug description is due to bz#1568353 being fixed (hence the rpm -q for pcs package).

Marking as verified in 1.1.19-3.el7.

Comment 22 errata-xmlrpc 2018-10-30 07:57:56 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:3055


Note You need to log in before you can comment on or make changes to this bug.