-
Notifications
You must be signed in to change notification settings - Fork 348
Bring rule schemas, documentation, and code in alignment #3409
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
Conversation
…ions The schema defines the "attribute" XML attribute as text, which allows empty strings as values, so the code should follow that (as it did before 9236a29).
…cation constraint
… meta-attributes The scheduler does not evaluate node attribute expressions when getting meta-attributes, so the fencer shouldn't either.
…xpressions The cluster ignores node expressions in rules within resource meta-attribute blocks, so crm_resource should too when getting a meta-attribute value or creating meta-attributes for direct agent execution.
@nrwahl2 , here's the most significant and useful rules PR |
I do like significance and usefulness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed through "Test: cts-cli: update expected outputs for new schema version". Still need to review the schema changes and, most tediously and usefully, the doc changes.
This drops the pe_get_configured_timeout() function, because it duplicated what other functions already do, but poorly. It did not handle rsc_expression and op_expression in rules, did not default to the minimum-interval monitor timeout for probes, etc. Also, use guint for the timeout, since that's how it's eventually used.
The existing behavior varies greatly depending on numerous criteria. The documented default is 20s, so we should stick with that.
…h op and op_defaults Ref T336
The schema already prevents them, but the code currently processes them if validation is disabled
Contrary to the log message as it was, rules cannot simply be moved from the lifetime element to its parent constraint element. Colocation, ordering, and ticket constraints do not support rules at all, and location constraints support them with different semantics. The feature is undocumented and has been deprecated since 1.0.8, so it should be fine to not recommend any alternative.
Fixes T784 (CLBZ#5415)
… changes The rule 3.9 schema is copied to 3.10. The constraints, nvset, and options schemas are copied and updated to include the new rule schema. The alerts, nodes, and resources schemas are copied and updated to include the new nvset schema.
Latest push addresses most of review so far |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed everything except the final doc commit. I'll do that next. Going ahead and making these comments visible.
@COMPAT: Support for node attribute expressions is deprecated | ||
here. We can just delete this define when we drop support. | ||
--> | ||
<define name="element-nvset.rule"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit message: "Node expressions are allowed only in ... and in meta_attributes in op and op_defaults elements" We're deprecating that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why have the define in the first place? The default element-rule
already allows node attribute expressions, under a COMPAT header.
Note that we didn't redefine element-nvset.rule
in the other meta_attributes blocks in this schema.
Are we wanting to maintain support for node attribute expressions under <op> <meta_attributes>
for longer than we maintain it in the other <meta_attributes>
contexts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit message: "Node expressions are allowed only in ... and in meta_attributes in op and op_defaults elements" We're deprecating that.
Right, the schema as of this commit does allow them. Deprecation is noted in comment.
Why have the define in the first place? The default
element-rule
already allows node attribute expressions, under a COMPAT header.
Just for clarity of intent. element-rule
(as of this commit) is intended for places that do not support node attribute expressions (as of this commit). The other meta-attribute blocks already don't support node attribute expressions, so element-rule
is appropriate there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the schema as of this commit does allow them. Deprecation is noted in comment.
The schema allows them in every single rule regardless of rule context, both before and after this commit. It's no more and no less "allowed" in other contexts than in op
and op_defaults
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the schema as of this commit does allow them. Deprecation is noted in comment.
The schema allows them in every single rule regardless of rule context, both before and after this commit. It's no more and no less "allowed" in other contexts than in
op
andop_defaults
More to my point, compared the other places where it's been deprecated, it's now the same amount of deprecated in op
and op_defaults
... so I don't think we should make a distinction in the deprecation status by saying it's "allowed" in op/op_defaults
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the schema as of this commit does allow them. Deprecation is noted in comment.
The schema allows them in every single rule regardless of rule context, both before and after this commit. It's no more and no less "allowed" in other contexts than in
op
andop_defaults
What I actually meant was "the schema allows them and the code implements them". The other deprecated contexts are allowed but ignored. I'll update the commit message.
Added commit for sed change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pshew
This extends previous work to more exactly specify which rule syntax is allowed in which contexts, specifically: * The lifetime element (within constraints) is deprecated * Node expressions are both allowed and implemented only in instance_attributes within bundle, clone, group, op, primitive, and template elements, and in meta_attributes in op and op_defaults elements * value-source is allowed in expression elements, but only with literal as its value * value is prohibited for defined and not_defined operations, and required for all other operations * The role attribute of rule elements is used only within location constraints * Support for multiple rules within a location constraint is deprecated. To preserve backward compatibility, the invalid syntax is merely marked with comments for now, for easier removal later.
This corrects various errors and omissions in the rules documentation, improves the wording, and converts all tables to list tables. Deprecated syntax is left undocumented.
Updated for remaining review items |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good
updated per most recent review |
missed the bandwidth typo in latest push |
Reword for clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hooray!
This includes a number of fixes in rule processing, deprecates some more rule syntax, prepares the schemas for dropping deprecated syntax, and overhauls the rules documentation.