Project

General

Profile

Bug #495

Facts truncation when using storeconfig and sharing a MySQL DB with puppet

Added by Ludovic LANGE over 8 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Facts
Target version:
Difficulty:
Triaged:
No
Bugzilla link:
Pull request:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

Hello,

I have a Puppet 0.25.5 installation (puppet-0.25.5-1.el5.rpm) and a Foreman 0.1.6 installation (foreman-0.1.6-2.rpm).

Puppet being installed with storeconfig and a MySQL DB, I decided to use a shared installation for the facts, as described in http://theforeman.org/projects/foreman/wiki/Puppet_Facts#Using-Puppet-Storeconfigs

While the resulting process (DB update) did work, and foreman is in a usable state, I noticed an impact on puppet : many facts were being stored truncated to 255 characters. (The net effect was that a lot of hosts were being puppet-modified, as I do deploy on them some SSH keys which are more than 255 characters long).

I think the file : http://theforeman.org/projects/foreman/repository/revisions/master/changes/db/migrate/20100325142616_update_fact_names_and_values_to_bin.rb may be the culprit here, as it seems to change the column "fact_values", from TEXT to a varchar(255).

I modified in the DB the column back to TEXT.

I did not have the ability to check what other impact did this migration procedure have, but I think they should be documented somewhere in the Wiki.
Or at least we should emphasize the fact that there are inherent risks to a shared DB configuration. The current text says : "Foreman uses a database, this database can be shared with Puppet store-configs (they are compatible, as Foreman extends the puppet database schema)."

I think we should also check that the migration procedure is updated to be in real sync with the puppet DB schema, so that no other fields are modified.

I put this as a high priority as it has an impact on existing puppet, and may have unintended consequences.

Associated revisions

Revision 79ae7779 (diff)
Added by Ohad Levy over 8 years ago

fixes #495 - Facts truncation when using storeconfig and sharing a MySQL DB with puppet

Revision 78bda549 (diff)
Added by Ohad Levy over 8 years ago

fixes #495 - ensure we handle the correct index if there is more than one

Revision 634023e2 (diff)
Added by Ohad Levy over 8 years ago

fixes #495 - only value field needs to be text, reverting

History

#1 Updated by Ohad Levy over 8 years ago

You Are correct, the reason for the change was that facts were not case sensitive therefore we changed them.

I assume that the right fix would be to enlarge the varchar size to something longer than 256 bytes, what do you think?

#2 Updated by Ludovic LANGE over 8 years ago

Wouldn't it be possible that we use the same column definition as Puppet's (which, I think, is TEXT), and only change the collation ?

I don't know what can be the maximum column size for facts - perhaps it's guessable in puppet's code...

In my DB, the max column length would be 1092 bytes - but it may not be representative of anything.

#3 Updated by Ohad Levy over 8 years ago

@Joschen, do you have any ideas of how does this effect PostgreSQL ?

#4 Updated by Jochen Schalanda over 8 years ago

It should work fine with PostgreSQL. The length of the TEXT data type is unlimited (for later reference: http://www.postgresql.org/docs/9.0/interactive/datatype-character.html).

Collations are MySQL specific and won't influence the PostgreSQL or the SQLite part.

#5 Updated by Ohad Levy over 8 years ago

Ludovic LANGE wrote:

Wouldn't it be possible that we use the same column definition as Puppet's (which, I think, is TEXT), and only change the collation ?

As I'm not a mysql expert, would changing the following will solve the problem?


ALTER TABLE fact_names MODIFY name text COLLATE utf8_bin NOT NULL

#6 Updated by Anonymous over 8 years ago

Guess you've already got this one under your eye. I'd agree to create column parity with Puppet if possible.

#7 Updated by Ohad Levy over 8 years ago

  • Status changed from New to Ready For Testing
  • % Done changed from 0 to 100

#8 Updated by Ohad Levy over 8 years ago

  • Status changed from Ready For Testing to Closed
  • Assignee set to Ohad Levy

Also available in: Atom PDF