Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1517834 - corosync.rng doesn't have all the options available.
Summary: corosync.rng doesn't have all the options available.
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: clufter
Version: 7.4
Hardware: All
OS: Linux
medium
medium
Target Milestone: rc
: ---
Assignee: Jan Pokorný [poki]
QA Contact: cluster-qe@redhat.com
URL:
Whiteboard:
Depends On:
Blocks: 1122832
TreeView+ depends on / blocked
 
Reported: 2017-11-27 14:50 UTC by Raul Mahiques
Modified: 2018-04-10 18:48 UTC (History)
7 users (show)

Fixed In Version: clufter-0.77.0-2.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-04-10 18:48:04 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2018:1009 None None None 2018-04-10 18:48:12 UTC
Red Hat Bugzilla 1183103 None None None 2019-04-08 14:52:59 UTC

Internal Links: 1183103

Description Raul Mahiques 2017-11-27 14:50:35 UTC
Description of problem:
/usr/lib/python2.7/site-package/clufter/formats/corosync/corosync.rng doesn't have all the options available for pacemaker therefore commands such as pcs config export pcs-commands
fail because it can't validate the config.

Version-Release number of selected component (if applicable):
clufter-common-0.76.0-1.el7

How reproducible:
Always

Steps to Reproduce:
1. Add for example:
nodelist {
     node {
          name: xyz
         .....

2. sync the cluster config: pcs sync 
3. The cluster will take the new name, it's a valid parameter, but then if we want to see the commands that could be used to configure the cluster with such configuration we will get an error:
pcs config export pcs-commands-verbose 




Actual results:
The validation fails.

Expected results:
Validation should work

Additional info:

Comment 2 Raul Mahiques 2017-11-27 15:02:16 UTC
(In reply to Raul Mahiques from comment #0)
typo: s/pacemaker/corosync/
> Description of problem:
> /usr/lib/python2.7/site-package/clufter/formats/corosync/corosync.rng
> doesn't have all the options available for pacemaker therefore commands such
> as pcs config export pcs-commands
> fail because it can't validate the config.
> 
> Version-Release number of selected component (if applicable):
> clufter-common-0.76.0-1.el7
> 
> How reproducible:
> Always
> 
> Steps to Reproduce:
> 1. Add for example:
> nodelist {
>      node {
>           name: xyz
>          .....
> 
> 2. sync the cluster config: pcs sync 
> 3. The cluster will take the new name, it's a valid parameter, but then if
> we want to see the commands that could be used to configure the cluster with
> such configuration we will get an error:
> pcs config export pcs-commands-verbose 
> 
> 
> 
> 
> Actual results:
> The validation fails.
> 
> Expected results:
> Validation should work
> 
> Additional info:

Comment 4 Jan Pokorný [poki] 2017-11-29 16:01:29 UTC
I believe that's not an issue with clufter, but rather with pacemaker
expecting that a particular configuration key in corosync tracking of
the settings, which was introduced in

https://github.com/ClusterLabs/pacemaker/commit/8511b3e118682d43e28765a47bca678ed3a53096#diff-677c4887fedde67f7205539b2231bf95R888

Corosync doesn't appear to be using the particular key in any way,
and it's strange for higher level tool to hijack proper configuration
management of the lower level tool, especially since pacemaker has
configuration management on its own.

Hence I believe the respective recommendation item should be dropped
from
https://clusterlabs.org/doc/en-US/Pacemaker/1.1/html/Pacemaker_Explained/s-node-name.html

Using ring0_addr key, which also gets effectively used by corosync,
should be enough. and might also avoid unnecessary ambiguities.

Adding corosync and pacemaker maintainers for their inputs.

Comment 5 Jan Pokorný [poki] 2017-11-29 16:17:42 UTC
Anyway, the workaround is to append --force switch to
"pcs config export pcs-commands{,-verbose}" commands
(or add --nocheck switch when running clufter pcs2pcscmd directly).

Comment 6 Jan Friesse 2017-11-29 16:48:33 UTC
@poki: Not sure what exact input you want, so I will try to explain my stand point.

corosync.conf is parsed and stored into cmap. Cmap is designed to contain various corosync "internal" data (parsed configuration, auto-generated items, statistics, ...) together with "user" data.

We could do very strict checking of all options in the corosync.conf and refuse config file with unknown option. But to make this work we would need to do the same checking also in cmap itself because of consistency. So cmap would then loose ability to contain "user" data.

So as you asked, corosync.conf itself is "conceptually expected to be extended arbitrarily" to some extend.

On the other hand, I must fully agree pcs shouldn't store (or require) this "user" data. And pacemaker should ether use ring0_addr or we can standardize name (what looks like neat feature anyway).

Comment 7 Ken Gaillot 2017-11-29 17:09:50 UTC
FYI, regarding the name option in particular, pacemaker needs it to connect a node name to a corosync ID when the node name is not the uname and ring0_addr is an IP address, which is a fairly common situation.

But I think the central point is that corosync does allow arbitrary options specified by client applications, so it's just not correct to consider these invalid.

Comment 8 Jan Pokorný [poki] 2017-11-29 17:15:40 UTC
Wouldn't it make sense then to dedicate extra label, call it "private",
to be usable either as a direct key or a subtree root in arbitrary
location within the existing public hierarchy used by corosync?
Because now, if corosync decides to use nodelist.node.name for its
own purpose (sure it has liberty to do so), pacemaker (or whatever other
corosync user happening to bet on the unlucky name is basically screwed,
marking this a bad design.

Comment 9 Jan Pokorný [poki] 2017-11-29 18:14:40 UTC
Indeed, in this case the bilateral agreement [comment 6] would be
optimal (even when settled rather late), so take [comment 8] merely
as a suggestion for a clean namespace separation in general, then.

Clufter (and perhaps also corosync proper might if there's
an interest to ressurect
https://github.com/corosync/corosync/commits/topic-relax-conf-gen)
would certainly benefit from a generic expressability by the means of
RelaxNG schema and sadly, static-prefix cannot be expressed, just
a full-blown label (like suggested "private") or "anything"
(which would work for the elements/attribute of the "private"
subtree).

Comment 10 Jan Pokorný [poki] 2017-11-29 19:54:31 UTC
re [comment 6]:

> [...] or we can standardize name (what looks like neat feature anyway)

Ok, I was rather excessively dismissive of this kind of unannounced
namespace squatting by external party from corosync's POV (would be
limited if there was a right way to integrate, something akin to
"private" subtrees per above), but as also explained in [comment 7],
identification-guiding fallback otherwise not interfering with the
messaging layer itself may come indispensable.

-> https://github.com/corosync/corosync/pull/286

Comment 11 Jan Pokorný [poki] 2017-11-29 20:00:39 UTC
When that PR gets resolved, I'll extend corosync.rng carried by
clufter respectively.

Comment 12 Jan Pokorný [poki] 2017-11-30 19:24:39 UTC
Round trip is complete now with this commit scheduled for
the nearest future point release:
https://pagure.io/clufter/c/4b406662507b0f1d1343f468a849adab2a243398?branch=next

Comment 13 Jan Pokorný [poki] 2017-11-30 19:29:23 UTC
I mean, this commit (fixed a typo in commit message):
https://pagure.io/clufter/c/ed02b8477e7d32791020d5f253138d3a60a4de5c?branch=next

Comment 14 Jan Pokorný [poki] 2017-12-01 11:08:01 UTC
Actually also modified ring0_addr description per what's been
changed in corosync.conf(5) along:
https://pagure.io/clufter/c/a75e1456f11725b7a58bc81148a6d6403b2530d2?branch=next

Comment 16 Jan Pokorný [poki] 2017-12-01 14:01:48 UTC
To test this:

$ clufter --dist=rhel,7.5 pcs2pcscmd -gqs - {cib2pcscmd.in_format.void_file} <<EOF
totem {
  cluster_name: test
  transport: udpu
  version: 2
}
nodelist {
  node {
    name: arpeggione
    nodeid: 1
    ring0_addr: 191.168.121.1
  }
  node {
    nodeid: 2
    name: balalaika
    ring0_addr: 191.168.121.2
  }
  node {
    name: cythara
    nodeid: 3
    ring0_addr: 191.168.121.3
  }
}
quorum {
  provider: corosync_votequorum
}
EOF

BEFORE (something like):
> WARNING:clufter.format:Invalid as per RNG file
>   `/usr/lib/python2.7/site-package/clufter/formats/corosync/corosync.rng'
> WARNING:clufter.format:None of the validation attempts succeeded with
>   validator spec: /usr/lib/python2.7/site-package/clufter/formats/corosync/corosync.rng
> coroxml-needle: Validation: 0:0:Extra element nodelist in interleave, 0:0:
>   Element corosync failed to validate content

AFTER (something like):
> #!/bin/sh
> # sequence generated on 2017-12-01 14:56:19 with: clufter 0.77.1a
> # invoked as: ['./run-dev', '--dist=rhel,7.5', 'pcs2pcscmd', '-gqs',
> # '-', '{cib2pcscmd.in_format.void_file}']
> # targeting system: ('linux', 'redhat', '7.5')
> # using interpreter: CPython 2.7.14
> pcs cluster auth 191.168.121.1 191.168.121.2 191.168.121.3 <> /dev/tty
> pcs cluster setup --name test \
>   191.168.121.1 191.168.121.2 191.168.121.3 --transport udpu
> pcs cluster start --all --wait=60
> pcs cluster cib tmp-cib.xml
> cp tmp-cib.xml tmp-cib.xml.deltasrc
> pcs cluster cib-push tmp-cib.xml diff-against=tmp-cib.xml.deltasrc

Comment 20 errata-xmlrpc 2018-04-10 18:48:04 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:1009


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