Fixes #38856 - chain composite CV publishes to wait on children (#11621)

* Fixes #38856 - chain composite CV publishes to wait on children

* Refs #38856 - rubocop + unneeded rescue

* Refs #38856 - skip auto publish request if already scheduled

* Refs #38856 - fail CCV publish if already scheduled

* Refs #38856 - use ForemanTasks chain method

* Refs #38856 - bump dynflow and foreman tasks reqs

* Refs #38856 - test scheduled_composite_publish?

* Refs #38856 - bump ruby requirement to 3.0 to match Dynflow

* Refs #38856 - halt manual composite publish if children are publishing

* Refs #38856 - fix rubocop errors from Ruby 3 upgrade

Author: ianballou

.github/workflows/ruby.yml

@@ -18,6 +18,7 @@ jobs:
     uses: theforeman/actions/.github/workflows/rubocop.yml@v0
     with:
       command: bundle exec rubocop --parallel --format github
+      ruby: '3.0'
 
   test:
     name: Ruby

.rubocop.yml

@@ -27,7 +27,7 @@ AllCops:
     - '**/*.rb'
     - app/views/**/*.rabl
     - '**/*.rake'
-  TargetRubyVersion: 2.7
+  TargetRubyVersion: 3.0
 
 Metrics/MethodLength:
   Description: 'Avoid methods longer than 30 lines of code.'

CLAUDE.md

@@ -353,6 +353,27 @@ const filtered = array.filter(cb => cb.disabled);        // Not a promise!
 const firstCheckbox = checkboxes.find(cb => !cb.disabled);
 ```
 
+### Test Writing Best Practices
+
+**Avoid Non-Determinism:**
+- **Never use `SecureRandom.uuid`** or other random values in tests
+- Use fixed strings like `'test-task-id-123'` instead
+- This ensures tests are reproducible and debuggable
+
+**Example - Bad:**
+```ruby
+task = ForemanTasks::Task.create!(
+  external_id: SecureRandom.uuid  # DON'T DO THIS
+)
+```
+
+**Example - Good:**
+```ruby
+task = ForemanTasks::Task.create!(
+  external_id: 'test-scheduled-publish-123'  # Deterministic
+)
+```
+
 ### TDD Workflow
 1. Write failing test
 2. Run test to confirm failure

app/lib/katello/util/package.rb

@@ -1,10 +1,10 @@
 module Katello
   module Util
     module Package
-      SUFFIX_RE = /\.(rpm)$/.freeze
-      ARCH_RE = /\.([^.\-]*)$/.freeze
-      NVRE_RE = /^(.*)-(?:([0-9]+):)?([^-]*)-([^-]*)$/.freeze
-      EVR_RE = /^(?:([0-9]+):)?(.*?)(?:-([^-]*))?$/.freeze
+      SUFFIX_RE = /\.(rpm)$/
+      ARCH_RE = /\.([^.\-]*)$/
+      NVRE_RE = /^(.*)-(?:([0-9]+):)?([^-]*)-([^-]*)$/
+      EVR_RE = /^(?:([0-9]+):)?(.*?)(?:-([^-]*))?$/
       SUPPORTED_ARCHS = %w(noarch i386 i686 ppc64 s390x x86_64 ia64).freeze
 
       # is able to take both nvre and nvrea and parse it correctly

app/lib/katello/util/path_with_substitutions.rb

@@ -5,7 +5,7 @@ class PathWithSubstitutions
 
       attr_reader :base_path, :path, :substitutions
 
-      SUBSTITUTABLE_REGEX = /^(.*?)\$([^\/]*)/.freeze
+      SUBSTITUTABLE_REGEX = /^(.*?)\$([^\/]*)/
 
       #path /content/rhel/server/$arch/$releasever/os
       #substitutions  {$arch => 'x86_64'}

app/models/katello/content_view.rb

@@ -662,6 +662,8 @@ def check_ready_to_publish!(importing: false, syncable: false)
         check_composite_action_allowed!(organization.library)
         check_docker_repository_names!([organization.library])
         check_orphaned_content_facets!(environments: self.environments)
+        check_scheduled_publish!
+        check_component_publishes!
       end
 
       true
@@ -677,6 +679,22 @@ def check_repositories_blocking_publish!
       end
     end
 
+    def check_scheduled_publish!
+      return unless composite?
+
+      if ::Katello::ContentViewManager.scheduled_composite_publish?(self)
+        fail ::Katello::Errors::ConflictException, _("A publish is already scheduled for this content view. Please wait for the scheduled publish to complete.")
+      end
+    end
+
+    def check_component_publishes!
+      return unless composite?
+
+      if ::Katello::ContentViewManager.running_component_publish_tasks(self).any?
+        fail ::Katello::Errors::ConflictException, _("Cannot publish composite content view while its content views are being published. Please wait for component publishes to complete.")
+      end
+    end
+
     def check_docker_repository_names!(environments)
       environments.each do |environment|
         repositories = []

app/services/katello/content_view_manager.rb

@@ -22,6 +22,11 @@ def self.create_candlepin_environment(content_view_environment:)
     end
 
     def self.request_auto_publish(content_view:, content_view_version:)
+      if scheduled_composite_publish?(content_view)
+        auto_publish_log(nil, "composite publish already scheduled for ID #{content_view.id}, skipping")
+        return
+      end
+
       request = content_view.create_auto_publish_request!(
         content_view_version: content_view_version
       )
@@ -46,14 +51,34 @@ def self.content_view_locks(content_view:)
 
     def self.trigger_auto_publish!(request:)
       request.with_lock do
-        if content_view_locks(content_view: request.content_view).any?
+        composite_cv = request.content_view
+
+        if content_view_locks(content_view: composite_cv).any?
           auto_publish_log(request, "locks found")
           return
         end
 
         description = _("Auto Publish - Triggered by '%s'") % request.content_view_version.name
-        ForemanTasks.async_task(Actions::Katello::ContentView::Publish, request.content_view, description, auto_published: true, triggered_by_id: request.content_view_version_id)
-        auto_publish_log(request, "task triggered")
+
+        # Find running component CV publish tasks for chaining
+        sibling_tasks = running_component_publish_tasks(composite_cv)
+
+        if sibling_tasks.any?
+          # Chain composite publish to wait for running component CVs
+          ForemanTasks.chain(
+            sibling_tasks,
+            Actions::Katello::ContentView::Publish,
+            composite_cv,
+            description,
+            auto_published: true,
+            triggered_by_id: request.content_view_version_id
+          )
+          auto_publish_log(request, "task chained to #{sibling_tasks.size} component tasks")
+        else
+          # No component CVs running, publish immediately
+          ForemanTasks.async_task(Actions::Katello::ContentView::Publish, composite_cv, description, auto_published: true, triggered_by_id: request.content_view_version_id)
+          auto_publish_log(request, "task triggered")
+        end
         request.destroy!
       rescue ForemanTasks::Lock::LockConflict => e
         auto_publish_log(request, e)
@@ -62,5 +87,29 @@ def self.trigger_auto_publish!(request:)
     rescue ActiveRecord::RecordNotFound
       auto_publish_log(request, "request gone")
     end
+
+    def self.scheduled_composite_publish?(composite_cv)
+      ForemanTasks::Task::DynflowTask
+        .for_action(::Actions::Katello::ContentView::Publish)
+        .where(state: 'scheduled')
+        .any? do |task|
+          delayed_plan = ForemanTasks.dynflow.world.persistence.load_delayed_plan(task.external_id)
+          args = delayed_plan.args
+          args.first.is_a?(::Katello::ContentView) && args.first.id == composite_cv.id
+        end
+    end
+
+    def self.running_component_publish_tasks(composite_cv)
+      component_cv_ids = composite_cv.components.pluck(:content_view_id)
+      return [] if component_cv_ids.empty?
+
+      ForemanTasks::Task::DynflowTask
+        .for_action(::Actions::Katello::ContentView::Publish)
+        .where(state: ['planning', 'planned', 'running'])
+        .select do |task|
+          task_input = task.input
+          task_input && component_cv_ids.include?(task_input.dig('content_view', 'id'))
+        end
+    end
   end
 end

katello.gemspec

@@ -13,7 +13,7 @@ Gem::Specification.new do |gem|
   gem.homepage    = "http://www.katello.org"
   gem.summary     = "Content and Subscription Management plugin for Foreman"
   gem.description = "Katello adds Content and Subscription Management to Foreman. For this it relies on Candlepin and Pulp."
-  gem.required_ruby_version = '>= 2.7', '< 4'
+  gem.required_ruby_version = '>= 3.0', '< 4'
 
   gem.files = Dir["{app,webpack,vendor,lib,db,ca,config,locale}/**/*"] +
     Dir['LICENSE.txt', 'README.md', 'package.json']
@@ -33,9 +33,9 @@ Gem::Specification.new do |gem|
   gem.add_dependency "rest-client"
 
   gem.add_dependency "rabl"
-  gem.add_dependency "foreman-tasks", ">= 9.1"
+  gem.add_dependency "foreman-tasks", ">= 12.0.0"
   gem.add_dependency "foreman_remote_execution", ">= 7.1.0"
-  gem.add_dependency "dynflow", ">= 1.6.1"
+  gem.add_dependency "dynflow", ">= 2.0.0"
   gem.add_dependency "activerecord-import"
   gem.add_dependency "stomp"
   gem.add_dependency "scoped_search", ">= 4.1.9"

test/factories/foreman_task.rb

@@ -7,7 +7,7 @@
     result { 'success' }
     sequence(:started_at) { |n| "2016-#{(n / 30) + 1}-#{(n % 30) + 1} 11:15:00" }
     after(:build) do |task|
-      task.ended_at = task.started_at.change(:sec => 32)
+      task.ended_at = task.started_at.change(:sec => 32) if task.started_at
     end
 
     trait :running do
@@ -21,6 +21,13 @@
       result { 'error' }
       ended_at { nil }
     end
+
+    trait :scheduled do
+      state { 'scheduled' }
+      result { 'pending' }
+      started_at { nil }
+      ended_at { nil }
+    end
   end
 
   factory :dynflow_task, :parent => :foreman_task, :class => ForemanTasks::Task::DynflowTask do

test/models/content_view_test.rb

@@ -853,5 +853,38 @@ def test_ignore_generated
       ignore_generated_include_library = Katello::ContentView.ignore_generated(include_library_generated: true).count
       assert_equal (ignore_generated - ignore_generated_include_library), 2
     end
+
+    def test_check_scheduled_publish_raises_when_scheduled_publish_exists
+      composite_cv = katello_content_views(:composite_view)
+
+      Katello::ContentViewManager.stubs(:scheduled_composite_publish?).with(composite_cv).returns(true)
+
+      error = assert_raises(Katello::Errors::ConflictException) do
+        composite_cv.check_scheduled_publish!
+      end
+
+      assert_match(/publish is already scheduled/, error.message)
+    end
+
+    def test_check_component_publishes_raises_when_component_publishes_running
+      composite_cv = katello_content_views(:composite_view)
+      running_task = mock('task')
+
+      Katello::ContentViewManager.stubs(:running_component_publish_tasks).with(composite_cv).returns([running_task])
+
+      error = assert_raises(Katello::Errors::ConflictException) do
+        composite_cv.check_component_publishes!
+      end
+
+      assert_match(/its content views are being published/, error.message)
+    end
+
+    def test_check_component_publishes_passes_when_no_component_publishes
+      composite_cv = katello_content_views(:composite_view)
+
+      Katello::ContentViewManager.stubs(:running_component_publish_tasks).with(composite_cv).returns([])
+
+      composite_cv.check_component_publishes!
+    end
   end
 end