Skip to content
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

Adding rule property to crm location #310

Closed
wants to merge 4 commits into from
Closed

Adding rule property to crm location #310

wants to merge 4 commits into from

Conversation

smoeding
Copy link
Contributor

Please check the following items before submitting a PR -- thank you!

Note that this project is released with a Contributor Code of Conduct.
By participating in this project you agree to abide by its terms.
Contributor Code of Conduct.

  • There is no existing PR that addresses this problem
  • Mentioned any existing issues in your commit so they get linked and
    closed once this PR gets merged, i.e: Closes #1554 in the body of a commit
  • Followed the instructions in the Contributing document
  • Ran the unit/spec tests and ensured they still pass
  • Added tests to cover the new behaviour
  • Updated the documentation to match the changes
  • When possible, add an entry to the CHANGELOG file
  • Squashed your PR down to a single commit. You may forego this if the PR
    tries to address multiple issues. Though we prefer one PR per feature/fix,
    sometimes that's not feasible. In that case ensure that a single feature/fix
    and associated tests and documentation is bundled up in one commit

Optional, but extra points:

  • Added tests to ensure the old behaviour cannot accidentally be
    reintroduced

There is no real progress at PR #132 currently. I am submitting this PR as a different implementation for the crm provider. My implementation adds the following enhancements:

  • The rule parameter has been renamed to rules as it will take an array with multiple rules.
  • Each rule is a hash of the rule name and the rule parameters.
  • The possible parameters are
    • score for a direct score
    • score-attribute to use the attribute value as score
    • expression containing an array of expressions to evaluate. Each array element is also a hash containing the attribute, operation and possibly the value of the expression.
    • boolean-op which can be and (default) or or to indicate how the expressions are joined

The README has been updated to show some of the possibilities.

I have included an updated version of the acceptance test written by @GiooDev but unfortunately I have no environment to run acceptance tests, so this might be an issue...

@smoeding
Copy link
Contributor Author

I've updated the branch with some (squashed) commits. If I'm not mistaken, the remaining failures are caused by the (currently unimplemented) pcs-provider on RedHat based systems.

@roidelapluie
Copy link
Member

Thanks. I will not be able to review this until next week.

@@ -17,6 +17,108 @@
# Decided to just go with relative.
commands crm: 'crm'

mk_resource_methods

# The node2hash method maps resource locations from XML into hashes. The
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move node2hash to cib_helper?

@roidelapluie
Copy link
Member

Is it possible to support lib/puppet/provider/cs_location/pcs.rb ?

@roidelapluie roidelapluie added needs-feedback Further information is requested needs-work not ready to merge just yet tests-fail x.y.z: Y version bump labels Aug 19, 2016
@roidelapluie roidelapluie added this to the 5.x milestone Aug 19, 2016
@roidelapluie
Copy link
Member

How can we mix this and #132?

@roidelapluie roidelapluie removed this from the 5.x milestone Aug 31, 2016
@roidelapluie roidelapluie added needs-rebase enhancement New feature or request labels Sep 1, 2016
@smoeding
Copy link
Contributor Author

smoeding commented Sep 6, 2016

I am sorry but I am giving up on this one. I have a working implementation on Debian that does all I currently need. I don't have a RedHat machine for tests and fiddling with Beaker/Rubocop/... simply takes more time than I am willing to spend on this topic. If anyone is brave enough to work in this: feel free to use any of my code as you like.

@roidelapluie
Copy link
Member

Thanks. Will do in #356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-feedback Further information is requested needs-rebase needs-work not ready to merge just yet tests-fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants