Katello nightly pipeline failed:
https://ci.theforeman.org/job/katello-nightly-rpm-pipeline/1220/
foreman-pipeline-katello-nightly-centos7-upgrade (passed) (remote job)
foreman-pipeline-katello-nightly-centos7-install (passed) (remote job)
foreman-pipeline-katello-nightly-centos8-stream-install (passed) (remote job)
foreman-pipeline-katello-nightly-centos8-stream-upgrade (failed) (remote job)
I wonder why Puppet fails in the CentOS 8 Stream upgrade but not in the install, nor in the CentOS 7 upgrade.
I do see this in the logs:
2022-01-27T18:21:08 [W|app|4a3dc185] Could not find a provider for pipe-up-katello-nightly-centos8-stream.n34.example.com. Providers returned {"Katello::ManagedContentMediumProvider"=>["Kickstart repository was not set for host 'pipe-up-katello-nightly-centos8-stream.n34.example.com'", "Content source was not set for host 'pipe-up-katello-nightly-centos8-stream.n34.example.com'"], "MediumProviders::Default"=>["CentOS_Stream 8 medium was not set for host 'pipe-up-katello-nightly-centos8-stream.n34.example.com'", "Invalid medium '' for 'CentOS_Stream 8'"]}
2022-01-27T18:21:08 [W|app|4a3dc185] Could not find a provider for pipe-up-katello-nightly-centos8-stream.n34.example.com. Providers returned {"Katello::ManagedContentMediumProvider"=>["Kickstart repository was not set for host 'pipe-up-katello-nightly-centos8-stream.n34.example.com'", "Content source was not set for host 'pipe-up-katello-nightly-centos8-stream.n34.example.com'"], "MediumProviders::Default"=>["CentOS_Stream 8 medium was not set for host 'pipe-up-katello-nightly-centos8-stream.n34.example.com'", "Invalid medium '' for 'CentOS_Stream 8'"]}
And a bit earlier there’s:
2022-01-27T18:12:47 [I|app|8595644e] Started POST "/api/hosts/facts" for 192.168.121.17 at 2022-01-27 18:12:47 +0000
2022-01-27T18:12:47 [I|app|8595644e] Processing by Api::V2::HostsController#facts as JSON
2022-01-27T18:12:47 [I|app|8595644e] Parameters: {"facts"=>"[FILTERED]", "name"=>"pipe-up-katello-nightly-centos8-stream.n34.example.com", "certname"=>"pipe-up-katello-nightly-centos8-stream.n34.example.com", "apiv"=>"v2", "host"=>{"certname"=>"pipe-up-katello-nightly-centos8-stream.n34.example.com", "name"=>"pipe-up-katello-nightly-centos8-stream.n34.example.com"}}
2022-01-27T18:12:47 [I|app|8595644e] Import facts for 'pipe-up-katello-nightly-centos8-stream.n34.example.com' completed. Added: 7, Updated: 7, Deleted 0 facts
2022-01-27T18:12:47 [W|app|8595644e] Action failed
2022-01-27T18:12:47 [I|app|8595644e] Backtrace for 'Action failed' error (ActiveRecord::RecordInvalid): Validation failed: Description has already been taken, Title has already been taken
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/validations.rb:80:in `raise_validation_error'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/validations.rb:53:in `save!'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/transactions.rb:318:in `block in save!'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/transactions.rb:375:in `block in with_transaction_returning_status'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/connection_adapters/abstract/database_statements.rb:280:in `block in transaction'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/connection_adapters/abstract/transaction.rb:280:in `block in within_new_transaction'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/connection_adapters/abstract/transaction.rb:278:in `within_new_transaction'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/connection_adapters/abstract/database_statements.rb:280:in `transaction'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/transactions.rb:212:in `transaction'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/transactions.rb:366:in `with_transaction_returning_status'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/transactions.rb:318:in `save!'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/suppressor.rb:48:in `save!'
8595644e | /usr/share/foreman/app/services/puppet_fact_parser.rb:38:in `operatingsystem'
8595644e | /usr/share/foreman/app/models/host/base.rb:161:in `block in set_non_empty_values'
8595644e | /usr/share/foreman/app/models/host/base.rb:160:in `each'
8595644e | /usr/share/foreman/app/models/host/base.rb:160:in `set_non_empty_values'
8595644e | /usr/share/foreman/app/models/host/base.rb:154:in `populate_fields_from_facts'
8595644e | /usr/share/foreman/app/models/host/managed.rb:489:in `populate_fields_from_facts'
8595644e | /usr/share/foreman/app/services/host_fact_importer.rb:50:in `block (2 levels) in parse_facts'
8595644e | /usr/share/foreman/app/services/foreman/telemetry_helper.rb:28:in `telemetry_duration_histogram'
8595644e | /usr/share/foreman/app/services/host_fact_importer.rb:49:in `block in parse_facts'
8595644e | /usr/share/foreman/app/services/host_fact_importer.rb:90:in `block in skipping_orchestration'
8595644e | /usr/share/foreman/app/models/concerns/orchestration.rb:124:in `without_orchestration'
8595644e | /usr/share/foreman/app/services/host_fact_importer.rb:89:in `skipping_orchestration'
8595644e | /usr/share/foreman/app/services/host_fact_importer.rb:45:in `parse_facts'
8595644e | /usr/share/foreman/app/services/host_fact_importer.rb:34:in `import_facts'
8595644e | /usr/share/foreman/app/controllers/api/v2/hosts_controller.rb:313:in `facts'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/abstract_controller/base.rb:195:in `process_action'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_controller/metal/rendering.rb:30:in `process_action'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/abstract_controller/callbacks.rb:42:in `block in process_action'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/callbacks.rb:112:in `block in run_callbacks'
8595644e | /usr/share/foreman/app/controllers/concerns/foreman/controller/timezone.rb:10:in `set_timezone'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/callbacks.rb:121:in `block in run_callbacks'
8595644e | /usr/share/foreman/app/models/concerns/foreman/thread_session.rb:32:in `clear_thread'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/callbacks.rb:121:in `block in run_callbacks'
8595644e | /usr/share/foreman/app/controllers/concerns/foreman/controller/topbar_sweeper.rb:12:in `set_topbar_sweeper_controller'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/callbacks.rb:121:in `block in run_callbacks'
8595644e | /usr/share/gems/gems/audited-4.9.0/lib/audited/sweeper.rb:14:in `around'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/callbacks.rb:121:in `block in run_callbacks'
8595644e | /usr/share/gems/gems/audited-4.9.0/lib/audited/sweeper.rb:14:in `around'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/callbacks.rb:121:in `block in run_callbacks'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/callbacks.rb:139:in `run_callbacks'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/abstract_controller/callbacks.rb:41:in `process_action'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_controller/metal/rescue.rb:22:in `process_action'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_controller/metal/instrumentation.rb:33:in `block in process_action'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/notifications.rb:180:in `block in instrument'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/notifications.rb:180:in `instrument'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_controller/metal/instrumentation.rb:32:in `process_action'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_controller/metal/params_wrapper.rb:245:in `process_action'
8595644e | /usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/railties/controller_runtime.rb:27:in `process_action'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/abstract_controller/base.rb:136:in `process'
8595644e | /usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/rendering.rb:39:in `process'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_controller/metal.rb:190:in `dispatch'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_controller/metal.rb:254:in `dispatch'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/routing/route_set.rb:50:in `dispatch'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/routing/route_set.rb:33:in `serve'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/routing/mapper.rb:18:in `block in <class:Constraints>'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/routing/mapper.rb:48:in `serve'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/journey/router.rb:49:in `block in serve'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/journey/router.rb:32:in `each'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/journey/router.rb:32:in `serve'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/routing/route_set.rb:834:in `call'
8595644e | /usr/share/gems/gems/katello-4.4.0.pre.master/lib/katello/middleware/organization_created_enforcer.rb:18:in `call'
8595644e | /usr/share/gems/gems/katello-4.4.0.pre.master/lib/katello/middleware/event_daemon.rb:10:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/static.rb:126:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/static.rb:126:in `call'
8595644e | /usr/share/gems/gems/apipie-dsl-2.4.0/lib/apipie_dsl/static_dispatcher.rb:67:in `call'
8595644e | /usr/share/gems/gems/apipie-rails-0.5.17/lib/apipie/static_dispatcher.rb:66:in `call'
8595644e | /usr/share/gems/gems/apipie-rails-0.5.17/lib/apipie/extractor/recorder.rb:137:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/static.rb:126:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/static.rb:126:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/static.rb:126:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/static.rb:126:in `call'
8595644e | /usr/share/foreman/lib/foreman/middleware/libvirt_connection_cleaner.rb:9:in `call'
8595644e | /usr/share/foreman/lib/foreman/middleware/telemetry.rb:10:in `call'
8595644e | /usr/share/gems/gems/apipie-rails-0.5.17/lib/apipie/middleware/checksum_in_headers.rb:27:in `call'
8595644e | /usr/share/foreman/lib/foreman/middleware/catch_json_parse_errors.rb:9:in `call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/tempfile_reaper.rb:15:in `call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/etag.rb:27:in `call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/conditional_get.rb:40:in `call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/head.rb:12:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/http/content_security_policy.rb:18:in `call'
8595644e | /usr/share/foreman/lib/foreman/middleware/logging_context_session.rb:22:in `call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/session/abstract/id.rb:266:in `context'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/session/abstract/id.rb:260:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/cookies.rb:648:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/callbacks.rb:101:in `run_callbacks'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/callbacks.rb:26:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/actionable_exceptions.rb:18:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/debug_exceptions.rb:32:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
8595644e | /usr/share/gems/gems/railties-6.0.3.7/lib/rails/rack/logger.rb:37:in `call_app'
8595644e | /usr/share/gems/gems/railties-6.0.3.7/lib/rails/rack/logger.rb:28:in `call'
8595644e | /usr/share/gems/gems/sprockets-rails-3.2.1/lib/sprockets/rails/quiet_assets.rb:13:in `call'
8595644e | /usr/share/foreman/lib/foreman/middleware/logging_context_request.rb:11:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/request_id.rb:27:in `call'
8595644e | /usr/share/gems/gems/katello-4.4.0.pre.master/lib/katello/prevent_json_parsing.rb:12:in `call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/method_override.rb:24:in `call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/runtime.rb:22:in `call'
8595644e | /usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/executor.rb:14:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/static.rb:126:in `call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/sendfile.rb:110:in `call'
8595644e | /usr/share/gems/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/host_authorization.rb:76:in `call'
8595644e | /usr/share/gems/gems/secure_headers-6.3.0/lib/secure_headers/middleware.rb:11:in `call'
8595644e | /usr/share/gems/gems/railties-6.0.3.7/lib/rails/engine.rb:527:in `call'
8595644e | /usr/share/gems/gems/railties-6.0.3.7/lib/rails/railtie.rb:190:in `public_send'
8595644e | /usr/share/gems/gems/railties-6.0.3.7/lib/rails/railtie.rb:190:in `method_missing'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/urlmap.rb:74:in `block in call'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/urlmap.rb:58:in `each'
8595644e | /usr/share/gems/gems/rack-2.2.3/lib/rack/urlmap.rb:58:in `call'
8595644e | /usr/share/gems/gems/puma-5.5.2/lib/puma/configuration.rb:249:in `call'
8595644e | /usr/share/gems/gems/puma-5.5.2/lib/puma/request.rb:77:in `block in handle_request'
8595644e | /usr/share/gems/gems/puma-5.5.2/lib/puma/thread_pool.rb:340:in `with_force_shutdown'
8595644e | /usr/share/gems/gems/puma-5.5.2/lib/puma/request.rb:76:in `handle_request'
8595644e | /usr/share/gems/gems/puma-5.5.2/lib/puma/server.rb:447:in `process_client'
8595644e | /usr/share/gems/gems/puma-5.5.2/lib/puma/thread_pool.rb:147:in `block in spawn_thread'
8595644e | /usr/share/gems/gems/logging-2.3.0/lib/logging/diagnostic_context.rb:474:in `block in create_with_logging_context'
2022-01-27T18:12:47 [I|app|8595644e] Rendering api/v2/errors/standard_error.json.rabl within api/v2/layouts/error_layout
2022-01-27T18:12:47 [I|app|8595644e] Rendered api/v2/errors/standard_error.json.rabl within api/v2/layouts/error_layout (Duration: 6.2ms | Allocations: 5409)
2022-01-27T18:12:47 [I|app|8595644e] Completed 500 Internal Server Error in 264ms (Views: 12.7ms | ActiveRecord: 45.5ms | Allocations: 53345)
This is most probably this PR:
That seems to indicate, we’re trying to save the same OS for the second time. Could you please point us to the test definition, that’s failing? I looked at the puppet fact parser code and it seems the problem is something else.
This is what happens in the console after I created the OS once already
2022-01-28T14:35:06 [D|sql|] Operatingsystem Load (0.7ms) SELECT "operatingsystems".* FROM "operatingsystems" WHERE "operatingsystems"."id" = $1 ORDER BY "operatingsystems"."title" ASC LIMIT $2 [["id", 15], ["LIMIT", 1]]
=> #<Redhat:0x000055d5ff28c0a0
id: 15,
major: "8",
name: "CentOS_Stream",
minor: "",
nameindicator: nil,
created_at: Fri, 28 Jan 2022 13:34:11 UTC +00:00,
updated_at: Fri, 28 Jan 2022 13:34:11 UTC +00:00,
release_name: nil,
type: "Redhat",
description: nil,
password_hash: [FILTERED],
title: "CentOS_Stream 8">
[12] pry(main)> Operatingsystem.find_or_initialize_by :name => 'CentOS_Stream', :major => 8, :minor => nil
2022-01-28T14:35:12 [D|sql|] Operatingsystem Load (1.1ms) SELECT "operatingsystems".* FROM "operatingsystems" WHERE "operatingsystems"."name" = $1 AND "operatingsystems"."major" = $2 AND "operatingsystems"."minor" IS NULL ORDER BY "operatingsystems"."title" ASC LIMIT $3 [["name", "CentOS_Stream"], ["major", "8"], ["LIMIT", 1]]
=> #<Operatingsystem:0x000055d5ff2f0d70
id: nil,
major: "8",
name: "CentOS_Stream",
minor: nil,
nameindicator: nil,
created_at: nil,
updated_at: nil,
release_name: nil,
type: nil,
description: nil,
password_hash: [FILTERED],
title: nil>
why that happens? The minor of the OS is “” instead of nil. IMHO it’s not necessarily caused by the linked patch, it’s just the first OS with empty minor. We need to drop the :minor => nil for such OSes or we need to make sure the minor is always NULL in the DB.
I don’t have time to finish the full patch, but I guess this would help
diff --git a/app/services/puppet_fact_parser.rb b/app/services/puppet_fact_parser.rb
index f8a6a8af7..b09cbf696 100644
--- a/app/services/puppet_fact_parser.rb
+++ b/app/services/puppet_fact_parser.rb
@@ -8,7 +8,8 @@ class PuppetFactParser < FactParser
major, minor = orel.split('.', 2)
major = major.to_s.gsub(/\D/, '')
minor = minor.to_s.gsub(/[^\d\.]/, '')
- args = {:name => os_name, :major => major, :minor => minor}
+ args = {:name => os_name, :major => major}
+ args = args.merge({ :minor => minor }) if minor.present?
os = Operatingsystem.find_or_initialize_by(args)
if os_name[/debian|ubuntu/i] || os.family == 'Debian'
if distro_codename
Run puppet agent -t on a CentOS Stream 8 system.
I know that in Django they recommended to always make string columns NOT NULL in order to avoid these differences. I’m leaning to the same. We can even add a DB default of an empty string to provide maximum compatibility.
I guess it typically is "" because the value usually comes from the form. When it’s created by facts/API, it can be nil. This is not the only place like this in Foreman. Perhaps this should become some sort of a policy. But to be crystal clear, I’m not opening such PR, the diff above would work for me to unblock the current issue.
I had a super-busy day and I need to wrap it up now, but I will take a look tomorrow.
NULL is a valid field state in SQL, getting rid of it for everything is not good approach. This is a problem with Rails validators and how we implemented it rather than with SQL. Hopefully the fix works, I also added a migration to clean up the DB too.
I looked up the Django recommendation:
https://docs.djangoproject.com/en/4.0/ref/models/fields/#null
Quoting it:
Avoid using
nullon string-based fields such asCharFieldandTextField. If a string-based field hasnull=True, that means it has two possible values for “no data”:NULL, and the empty string. In most cases, it’s redundant to have two possible values for “no data;” the Django convention is to use the empty string, notNULL. One exception is when aCharFieldhas bothunique=Trueandblank=Trueset. In this situation,null=Trueis required to avoid unique constraint violations when saving multiple objects with blank values.
This is precisely what we ran into and I think it’s good advice.
I misread your statement, you said “for string columns”. Generally that is a good practice, yeah.
What I meant was getting rid of them for everything, that is indeed a terrible idea.
I vaguely remember there was a reason why we kept the nil option in minor and that there was a patch that tried to exploit this. I think it was me who wrote it…
I looked at the PR and from reading that it should already only store it as a string when parsing facts. So I really struggle to find out what caused this. The last 2 runs of katello-nightly-rpm-pipeline [Jenkins] have passed. I wonder if it’s really this commit that fixed it:
Note that the error was talking about medium and Katello does override some things with that.
However, right now I can’t find a reproducer which makes this hard to investigate.
It really looks like nil value was not culprit of the problem. Looks like we do not allow nil in OS versions for quite some time:
I am closing the PR then.
The only idea I have for that particular pipeline failure was two concurrent requests trying to create OS at the same time?!
I think you may have misunderstood the actual cause. The OS gets save with "" as minor. The Fact parser ends up with minor set to nil and searches for the OS with minor being NULL. That is not found, therefore it initializes the new object (id being nil) and then when it’s saved, it fails hard because the OS with the same name exists. We should not add the minor key to find_or_initialize args if the value of it is nil, since such OS can never exist.
FTR such issue should only happen if facts for the same OS are uploaded twice. Perhaps the tests ran in a different order?
How? Looking at this code:
Here it calls minor.to_s so minor should never be nil, always "" since nil.to_s is "".
I also thought it may be RSHM uploading facts first, but that also has minor || '':
So I can’t quite figure out where it would be nil.
Ok, now I’m out of ideas. When I added pry there, I can no longer reproduce the nil. I can confirm all OSes in the DB have empty string as minor. I checked the ansible parser, which also sets the empty value to “”.
Could it really be that the test ran two imports at the same time?
I doubt it. It also showed up in multiple test runs so it wasn’t just a one-off, but we only saw it on Katello. I wonder if it’s somehow hidden in there. Possibly with the media relation?
My final conclusion: we need to rename OS name from CentOS to CentOS_Stream for all OSes that have “CentOS Stream” or “CentOS_Stream” in title or description.
The reason is stated in the migration itself:
# When redhat-lsb-core package is installed, puppet creates
# description/title "CentOS Stream 8", however, when the package is not
# present description is unset and title is set to "CentOS_Stream 8" from
# the OS name and major by the ActiveRecord callback. Let's migrate both
# cases.
This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.