The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Feature #9051

StoreConfig backend is hard-coded; it should be possible to change to another storage engine.

Added by Anonymous over 3 years ago. Updated over 1 year ago.

Status:ClosedStart date:08/17/2011
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:stored configuration
Target version:2.7.4
Affected Puppet version:0.22.1 Branch:https://github.com/daniel-pittman/puppet/commits/feature/2.7.x/9051-storeconfig-backend-should-be-configurable
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

At the moment the StoreConfig backend is hard-coded within Puppet to use an ActiveRecord based database store. It would be great to be able to easily substitute in a new storage engine without having to patch the core, ideally in the same way you can pick another terminus for the node indirection to integrate with your ENC.

This needs to provide feature parity with the current feature.

History

#1 Updated by Anonymous over 3 years ago

List of things and places that interact with the StoreConfig engine directly in the product, to work to build an API around this:

  • :async_storeconfigs, :thin_storeconfigs, in defaults.rb, which enable specific features of the tool. No change expected.
  • :storeconfigs in defaults.rb, which enables the core feature. Triggered to be true by the previous set.
  • lib/puppet/face/node/clean.rb, which interacts directly with the ActiveRecord code to delete / unexport content.
  • lib/puppet/indirector/catalog/compiler.rb, which configures the backend during compilation if :storeconfigs are enabled.
  • same file, update_node_check, which I can’t trace an invocation of…
  • lib/puppet/parser/collector.rb, which does collection from various sources; needs to go through the API rather than direct to Rails.
  • lib/puppet/parser/ast/collexp.rb, which does the syntax transformation to “rails” from the input language.
  • lib/puppet/application/queue.rb, which hard-codes the target when using async storeconfigs.

Defining the API for StoreConfig backends:

  1. add a configuration option for the storeconfig backend / terminus.
  2. update existing code to select a storeconfigs terminus, not active_record, when activating storeconfigs.
    • push down failure reporting to the underlying back-end implementation.
  3. write the storeconfigs terminus for each class so that it references the configuration option, then picks the right terminus to wrap.
  4. ensure that the Rails-specific bits are pushed down into the target, rather than the AST (for collection)
  5. push the query into the API; perhaps “find” from an indirection, to return the data.
    • the resource indirection is the right place for this; it should gain a StoreConfig terminus set.

Then we have an API for the final target:

  • save and find over catalog, facts, and node indirections that works with the storeconfigs terminus.
  • find over the collected resources through a separate indirection.

Alternatively, we could define a sound API over the interface we want – something that supports set and get over catalog, facts, nodes, and collecting resources. I would reach for a face, but the design we received makes it very difficult to do that cleanly; because they are forcibly instances of a class, rather than a class proper, we would need to define a custom API for the back-end, plus replicate all the work the indirector does around configuration, discovery, and loading of terminus instances.

Advice? I don’t like splitting this code into a dozen places, but we can’t pack it sanely into the indirection API without substantial contortion. (eg: a protocol for doing three or four things through a single method, using some dependent dispatch key.)

I lean toward defining a sound API through subclassing, then writing the huge pile of terminus classes to support that, and living with the need to select a backend implementation by hand. (eg: define Puppet::StoreConfigs and Puppet::StoreConfigs::ActiveRecord, then have the parent manage the API so that you get the concrete child instance when you ask for something implementing the API.)

#2 Updated by Anonymous over 3 years ago

  • Branch set to https://github.com/daniel-pittman/puppet/commits/feature/2.7.x/9051-storeconfig-backend-should-be-configurable

https://github.com/daniel-pittman/puppet/commits/feature/2.7.x/9051-storeconfig-backend-should-be-configurable implements the first steps of the process. We have an option for the backend to use, a StoreConfigs terminus for the relevant indirections, it successfully wraps the current backend, and passes testing. (With some order dependent terminus resetting and stuff.)

Now, to create an indirection for the “search” part of the process, and we are at least reasonably close to being able to have a drop-in replacement for StoreConfigs storage engine.

#3 Updated by Anonymous over 3 years ago

The same branch is updated with the latest version of this code (a forced push, because history has been rewritten during development). This is approaching completion, in that it now has all the code in what looks like the right places, and looking to be doing the right thing.

Still broken, though, are some of the collector tests, and by implication the final round of changes are not actually completed yet.

Once they are fixed up, and finished, the collection expressions are still way too intimate with the current ActiveRecord schema; the subsequent change will be to abstract that, and ensure that we have portable references to the data we use, not to the database schema it happens to be implemented with.

#4 Updated by Anonymous over 3 years ago

Just pushed another round of updates to GitHub; now we have working everything, and pass an abstract expression rather than the raw SQL compiled out of the collection expression through to the final destination.

#5 Updated by Anonymous over 3 years ago

  • Status changed from Accepted to In Topic Branch Pending Review

Pull request submitted for the code. I believe this is correct, and it certainly passes all the tests.

#6 Updated by Jeff McCune over 3 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release
  • Target version changed from 2.7.x to 2.7.4

Merged

Merged into 2.7.x as commit:


commit bdad190519e3aae8f76daba41ee7f5eb0fed6867
Merge: 29c7bf2 8700682
Author: Jeff McCune 
Date:   Tue Aug 30 10:32:07 2011 -0700

    Merge branch 'feature/2.7.x/9051-storeconfig-backend-should-be-configurable' into 2.7.x
    
    * feature/2.7.x/9051-storeconfig-backend-should-be-configurable:
      (#9051) de-ActiveRecord-ify Collection expressions.
      (#9051) Port query tests into the indirection.
      (#9051) Implement the `resource` terminus for StoreConfigs.
      (#9051) Make generic tagging imported resource origins.
      (#9051) Whitespace cleanup for puppet/parser/collector
      (#9051) Dead code elimination in the compiler terminus.
      (#9051) Get the compiler out of the ActiveRecord business.
      (#9051) Implement the StoreConfigs indirection itself.
      (#9051) Add configuration around StoreConfigs indirection.
    
    Reviewed-by: Jeff McCune

Code reviewed, rspec tests verify as passing, ticket updated.

#7 Updated by Nick Lewis over 3 years ago

Additional changes merged via pull request #98 in commit:74cffa087aa35d31b331033870b026d41e40cb07.

#8 Updated by Anonymous over 3 years ago

Nick Lewis wrote:

Additional changes merged via pull request #98 in commit:74cffa087aa35d31b331033870b026d41e40cb07.

Specifically, the previous changes missed one limitation of the current StoreConfigs code: previously, the Puppet parser would fail while evaluating the AST when you tried to collect exported resources on more than one condition. This is a fairly arbitrary limit that is really only drawn from the current StoreConfigs implementation. So, the changeset moved the limitation out of the parser, and implemented it instead in the terminus.

That will fail a bit later in the process, but it will still fail cleanly during compilation if the StoreConfig feature is used in ways that are not supported, and the net effect will be pretty much identical.

#9 Updated by Matthaus Owens over 3 years ago

  • Status changed from Merged - Pending Release to Closed

Released in 2.7.4rc1

#10 Updated by Anonymous over 1 year ago

  • Assignee deleted (Anonymous)

Also available in: Atom PDF