Bug #17335

puppet_fact_parser.rb does not correctly add hosts facts whos minor number does not exist or is empty

Added by Noh Wayh 8 months ago. Updated 8 months ago.

Status:Closed
Priority:High
Assigned To:Dominic Cleal
Category:Importers
Target version:-
Difficulty: Bugzilla link:
Found in release:1.13.1 Pull request:https://github.com/theforeman/foreman/pull/4025
Story points-
Velocity based estimate-
Release1.13.2Release relationshipAuto

Description

foreman/app/services/puppet_fact_parser.rb

"Fixes #14545, #13104 - Correctly parse y.z minor OS versions" change causes Solaris 10 hosts to not have their facts stored due to

os = Operatingsystem.find_or_initialize_by(args)

since the args then become name: Solaris, major: 10, minor: ''
the "operatingsystemrelease" fact looks like "10_u10" (Solaris 10 update 10 - ie 10.10), so it does not get split (and only the first host who's fact is uploaded does not get an error (the first time)
, rest of the time foreman spits out "error 500, internal server error" pointing to line 47 of this file ).

Solution to this is either to add a code like the following (ugly hack but works for Solaris 10):

   elsif os_name[/Solaris/i]
        orel.gsub!(/_u/, '.')

or a nicer way is to expand the orel.split with a regex that incorporates the "_u" and possibly even the FreeBSD regex
The "elsif" with Solaris works with Solaris 10 though, not so sure Solaris 11 keeps the same naming convention or uses "." notation so a orel.split with regex is preferrable.

solaris-facter-json.json - Solaris 10 facter json output (2.79 KB) Noh Wayh, 11/15/2016 09:43 AM


Related issues

Related to Foreman - Bug #13104: CentOS minor version numbering issue - preventing build Closed 01/10/2016
Related to Foreman - Refactor #17354: Test for idempotent operating system creation in Puppet f... Closed 11/16/2016

Associated revisions

Revision 13875289
Added by Dominic Cleal 8 months ago

fixes #17335 - parse Solaris update as minor version

Also fixes idempotency of OSes with only a major version (as Solaris
was prior to this change), where `minor` was nil instead of "", causing
the OS to be recreated with a validation error.

History

#1 Updated by Michael Moll 8 months ago

  • Release deleted (1.13.2)

Could you upload the output of "facter --json" of such a Solaris 10 machine? That would make it much easier to write tests for that case.

#2 Updated by Dominic Cleal 8 months ago

  • Related to Bug #13104: CentOS minor version numbering issue - preventing build added

#3 Updated by Noh Wayh 8 months ago

Uploaded facter facts in json format for Solaris 10

#4 Updated by Dominic Cleal 8 months ago

  • Category set to Importers
  • Status changed from New to Assigned
  • Assigned To set to Dominic Cleal
  • Priority changed from Urgent to High

After the first successful fact import, subsequent imports fail with:

2016-11-15T15:11:28 36ca7449 [app] [I] Import facts for 'cobalt.example.com' completed. Added: 0, Updated: 0, Deleted 0 facts
2016-11-15T15:11:28 36ca7449 [sql] [D]    (0.1ms)  begin transaction
2016-11-15T15:11:28 36ca7449 [sql] [D]   Operatingsystem Load (0.1ms)  SELECT  "operatingsystems".* FROM "operatingsystems" WHERE "operatingsystems"."id" = ?  ORDER BY "operatingsystems"."title" ASC LIMIT 1  [["id", 23]]
2016-11-15T15:11:28 36ca7449 [sql] [D]   Nic::Base Load (0.2ms)  SELECT "nics".* FROM "nics" WHERE "nics"."host_id" = ?  ORDER BY "nics"."identifier" ASC  [["host_id", 109]]
2016-11-15T15:11:28 36ca7449 [sql] [D]   Domain Load (0.1ms)  SELECT  "domains".* FROM "domains" WHERE "domains"."id" = ?  ORDER BY domains.name LIMIT 1  [["id", 4]]
2016-11-15T15:11:28 36ca7449 [sql] [D]    (0.0ms)  commit transaction
2016-11-15T15:11:28 36ca7449 [sql] [D]   Model Load (0.1ms)  SELECT  "models".* FROM "models" WHERE "models"."name" = ?  ORDER BY models.name LIMIT 1  [["name", "VMware Virtual Platform"]]
2016-11-15T15:11:28 36ca7449 [sql] [D]   Domain Load (0.1ms)  SELECT  "domains".* FROM "domains" WHERE "domains"."name" = ?  ORDER BY domains.name LIMIT 1  [["name", "localdomain"]]
2016-11-15T15:11:28 36ca7449 [sql] [D]   Architecture Load (0.1ms)  SELECT  "architectures".* FROM "architectures" WHERE "architectures"."name" = ?  ORDER BY "architectures"."id" ASC LIMIT 1  [["name", "i386"]]
2016-11-15T15:11:28 36ca7449 [sql] [D]   Operatingsystem Load (0.2ms)  SELECT  "operatingsystems".* FROM "operatingsystems" WHERE "operatingsystems"."name" = ? AND "operatingsystems"."major" = ? AND "operatingsystems"."minor" IS NULL  ORDER BY "operatingsystems"."title" A
SC LIMIT 1  [["name", "Solaris"], ["major", "109"]]
2016-11-15T15:11:28 36ca7449 [sql] [D]    (0.0ms)  begin transaction
2016-11-15T15:11:28 36ca7449 [sql] [D]   Operatingsystem Exists (0.1ms)  SELECT  1 AS one FROM "operatingsystems" WHERE ("operatingsystems"."name" = 'Solaris' AND "operatingsystems"."major" = '109' AND "operatingsystems"."minor" = '') LIMIT 1
2016-11-15T15:11:28 36ca7449 [sql] [D]   Operatingsystem Exists (0.1ms)  SELECT  1 AS one FROM "operatingsystems" WHERE "operatingsystems"."title" = 'Solaris 109' LIMIT 1
2016-11-15T15:11:28 36ca7449 [sql] [D]    (0.1ms)  rollback transaction
2016-11-15T15:11:28 36ca7449 [app] [W] Action failed
 | ActiveRecord::RecordInvalid: Validation failed: »Name« Operating system version already exists, »Title« has already been taken
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/validations.rb:79:in `raise_record_invalid'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/validations.rb:43:in `save!'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/attribute_methods/dirty.rb:29:in `save!'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/transactions.rb:291:in `block in save!'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/transactions.rb:351:in `block in with_transaction_returning_status'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `block in transaction'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/connection_adapters/abstract/transaction.rb:184:in `within_new_transaction'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `transaction'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/transactions.rb:220:in `transaction'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/transactions.rb:348:in `with_transaction_returning_status'
 | /home/dcleal/.rvm/gems/ruby-2.3.0@foreman/gems/activerecord-4.2.7.1/lib/active_record/transactions.rb:291:in `save!'
 | /home/dcleal/code/foreman/foreman/app/services/puppet_fact_parser.rb:47:in `operatingsystem'
 | /home/dcleal/code/foreman/foreman/app/models/host/base.rb:163:in `block in set_non_empty_values'
 | /home/dcleal/code/foreman/foreman/app/models/host/base.rb:162:in `each'
 | /home/dcleal/code/foreman/foreman/app/models/host/base.rb:162:in `set_non_empty_values'
 | /home/dcleal/code/foreman/foreman/app/models/host/base.rb:155:in `populate_fields_from_facts'
 | /home/dcleal/code/foreman/foreman/app/models/host/managed.rb:470:in `populate_fields_from_facts'
 | /home/dcleal/code/foreman/foreman/app/models/host/base.rb:133:in `import_facts'
 | /home/dcleal/code/foreman/foreman/app/models/host/managed.rb:307:in `import_facts'
 | /home/dcleal/code/foreman/foreman/app/controllers/api/v2/hosts_controller.rb:260:in `facts'

The operating system created was:

=> {"id"=>23, "major"=>"109", "name"=>"Solaris", "minor"=>"", "nameindicator"=>nil, "created_at"=>Tue, 15 Nov 2016 15:11:06 UTC +00:00, "updated_at"=>Tue, 15 Nov 2016 15:11:06 UTC +00:00, "release_name"=>nil, "type"=>"Solaris", "description"=>nil, "password_hash"=>"SHA256", "title"=>"Solaris 109"}

#5 Updated by The Foreman Bot 8 months ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/4025 added

#6 Updated by Dominic Cleal 8 months ago

  • Release set to 1.13.2

#7 Updated by Dominic Cleal 8 months ago

  • Related to Refactor #17354: Test for idempotent operating system creation in Puppet fact parser added

#8 Updated by Dominic Cleal 8 months ago

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

Also available in: Atom PDF