Repurpose CloudConnectorAnnounceTask for daily Sources registration

[jeremylenz]

Summary

This PR addresses SAT-45966 / SAT-44641 by repurposing the CloudConnectorAnnounceTask from a REX subscription into a daily recurring task with an API endpoint for immediate registration.

Before (Old Flow — Broken with foremanctl)

1. User clicks "Configure Cloud Connector" in the Foreman UI
2. This triggers a Remote Execution (REX) job (ansible_configure_cloud_connector)
3. When the REX job completes → CloudConnectorAnnounceTask fires (subscribed to RunHostsJob)
4. Sources registration happens automatically via CloudPresence.announce_to_sources

With cloud connector setup moving to foremanctl, the REX job never runs, so CloudConnectorAnnounceTask never fires and Sources registration never happens.

After (New Flow — Fixed)

1. User runs foremanctl deploy --add-feature cloud-connector
2. Immediate registration: foremanctl calls POST /api/v2/rh_cloud/announce_to_sources after setting rhc_instance_id
3. Daily self-healing: A recurring task checks if registration is needed for each org, with a staggered 0–3 hour delay
4. Smart skipping: Orgs with recent cloud remediation jobs (past 24h) skip the API call since connectivity is already proven

Changes

• Converts CloudConnectorAnnounceTask from a REX subscription to a daily RecurringAction + one-shot API trigger
• Adds POST /api/v2/rh_cloud/announce_to_sources endpoint for immediate registration during foremanctl deploy
• Adds rhc_connection_exists? check in CloudPresence to avoid duplicate connection errors (Sources API returns 400)
• Skips Sources API call for orgs with recent cloud remediation jobs
• Removes the old REX-based cloud connector flow: CloudConnector service class, enable_cloud_connector endpoints, ansible_configure_cloud_connector REX feature
• Task output distinguishes: Registered / Already registered / Already registered (recent cloud remediation) / Skipped (no manifest) / Failed

Test plan

• Daily scheduled task runs and registers orgs with valid manifests
• POST /api/v2/rh_cloud/announce_to_sources triggers immediate registration (no delay)
• Task skips when rhc_instance_id is not set, in IoP mode, or org has no manifest
• Task skips Sources API call when recent cloud remediation jobs exist
• Task shows warning (not success) when any org fails
• Old REX-based cloud connector functionality is cleanly removed
• 543 tests pass, 0 failures

🤖 Generated with Claude Code

[sourcery-ai]

Hey - I've found 3 issues, and left some high level feedback:

• CloudConnectorAnnounceTask#run logs a per-org info line for every organization skipped due to missing manifest; if you expect many orgs this could be noisy, so consider summarizing skipped orgs at the end (similar to output[:status]) instead of logging each one individually.
• The warning about allow_auto_inventory_upload being disabled in CloudConnectorAnnounceTask#plan will be emitted on every daily run while the setting is off; if that’s an expected long-term configuration for some deployments, consider downgrading to info or adding a guard so the warning is not continuously repeated.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- CloudConnectorAnnounceTask#run logs a per-org info line for every organization skipped due to missing manifest; if you expect many orgs this could be noisy, so consider summarizing skipped orgs at the end (similar to `output[:status]`) instead of logging each one individually.
- The warning about `allow_auto_inventory_upload` being disabled in CloudConnectorAnnounceTask#plan will be emitted on every daily run while the setting is off; if that’s an expected long-term configuration for some deployments, consider downgrading to info or adding a guard so the warning is not continuously repeated.

## Individual Comments

### Comment 1
<location path="lib/insights_cloud/async/cloud_connector_announce_task.rb" line_range="33-46" />
<code_context>
       end

-      def finalize
+      def run
+        announced = []
+        skipped = []
+        failed = []
+
         Organization.unscoped.each do |org|
+          unless cert_auth_available?(org)
+            logger.info("Skipping Sources announcement for organization #{org.name}: no manifest available")
+            skipped << org.name
+            next
+          end
+
           presence = ForemanRhCloud::CloudPresence.new(org, logger)
</code_context>
<issue_to_address>
**suggestion:** Exception handling in `run` drops stack traces, which can complicate debugging

In the `rescue StandardError => ex` block, only `"Failed to announce...: #{ex}"` is logged, so the backtrace is lost. Please also log `ex.full_message` or `ex.message` together with `ex.backtrace.join("\n")` (possibly at debug level or in a separate line) so failures remain diagnosable in production.

```suggestion
        Organization.unscoped.each do |org|
          unless cert_auth_available?(org)
            logger.info("Skipping Sources announcement for organization #{org.name}: no manifest available")
            skipped << org.name
            next
          end

          presence = ForemanRhCloud::CloudPresence.new(org, logger)
          presence.announce_to_sources
          announced << org.name
        rescue StandardError => ex
          logger.warn("Failed to announce to Sources for organization #{org.name}: #{ex}")
          logger.debug(
            "Sources announcement error details for organization #{org.name}: " \
            "#{ex.class}: #{ex.message}\n#{Array(ex.backtrace).join("\n")}"
          )
          failed << org.name
        end
```
</issue_to_address>

### Comment 2
<location path="test/jobs/cloud_connector_announce_task_test.rb" line_range="11-20" />
<code_context>
+    ForemanRhCloud.unstub(:with_iop_smart_proxy?)
+  end
+
+  test 'announces to sources for all organizations when rhc_instance_id is set' do
+    Setting[:rhc_instance_id] = 'test-rhc-id'
+
+    InsightsCloud::Async::CloudConnectorAnnounceTask.any_instance
+      .stubs(:cert_auth_available?).returns(true)

-    @job_invocation = generate_job_invocation(:ansible_configure_cloud_connector)
+    ForemanRhCloud::CloudPresence.any_instance
+      .expects(:announce_to_sources)
+      .times(Organization.unscoped.count)

-    # reset connector feature ID cache
-    InsightsCloud::Async::CloudConnectorAnnounceTask.instance_variable_set(:@connector_feature_id, nil)
+    action = create_and_plan_action(InsightsCloud::Async::CloudConnectorAnnounceTask)
+    run_action(action)
   end

</code_context>
<issue_to_address>
**suggestion (testing):** Add assertions for the task output status to cover the new reporting behavior in `run`

The new `output[:status]` summary (with announced/skipped/failed orgs) isn’t validated by the tests. Please add at least one test that inspects `action.output[:status]` after `run_action(action)` and covers cases like:

- all orgs announced (only `Announced:` present)
- orgs skipped for missing manifest (`Skipped (no manifest):` present)
- orgs failing (`Failed:` present)

This will exercise the new reporting behavior and guard against regressions in the status formatting and content.

Suggested implementation:

```ruby
  test 'announces to sources for all organizations when rhc_instance_id is set' do
    Setting[:rhc_instance_id] = 'test-rhc-id'

    InsightsCloud::Async::CloudConnectorAnnounceTask.any_instance
      .stubs(:cert_auth_available?).returns(true)

    ForemanRhCloud::CloudPresence.any_instance
      .expects(:announce_to_sources)
      .times(Organization.unscoped.count)

    action = create_and_plan_action(InsightsCloud::Async::CloudConnectorAnnounceTask)
    run_action(action)

    # Validate reporting in action output
    assert_equal true, action.output[:success]
    assert_includes action.output[:status], 'Announced:'
    refute_includes action.output[:status], 'Skipped (no manifest):'
    refute_includes action.output[:status], 'Failed:'
  end

```

To fully match the review suggestion, you can add two more tests that exercise the “skipped (no manifest)” and “failed” paths by:
1. Creating organizations that will trigger those branches in `run` (e.g. by stubbing/mocking the parts of `CloudConnectorAnnounceTask` / `CloudPresence` that decide an org is skipped vs failed).
2. Running `run_action(action)` and asserting that `action.output[:status]` includes the respective `Skipped (no manifest):` and `Failed:` fragments (and that the other fragments are absent or have the expected counts).
The exact stubs/exceptions depend on how the implementation distinguishes skipped vs failed orgs in your codebase.
</issue_to_address>

### Comment 3
<location path="test/jobs/cloud_connector_announce_task_test.rb" line_range="59-22" />
<code_context>

-  def read_jsonl(jsonl)
-    jsonl.lines.map { |l| JSON.parse(l) }
+  test 'skips organizations without a manifest' do
+    Setting[:rhc_instance_id] = 'test-rhc-id'
+
+    InsightsCloud::Async::CloudConnectorAnnounceTask.any_instance
+      .stubs(:cert_auth_available?).returns(false)
+
+    ForemanRhCloud::CloudPresence.any_instance
+      .expects(:announce_to_sources)
+      .never
+
+    action = create_and_plan_action(InsightsCloud::Async::CloudConnectorAnnounceTask)
+    run_action(action)
+  end
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider a test that covers behavior when automatic inventory upload is disabled

The `plan` method now warns when `Setting[:allow_auto_inventory_upload]` is false while `rhc_instance_id` is set, but still calls `plan_self`. Current tests don’t distinguish between the true/false cases for `allow_auto_inventory_upload`. Please add a test that sets `Setting[:rhc_instance_id]` with `Setting[:allow_auto_inventory_upload] = false` and verifies the task still runs (e.g., `run_action(action)` leads to `announce_to_sources` being called). This guards against future changes that might skip the task when auto-upload is disabled.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[parthaa]

Recommend adding something like what the flow used to look like and what changes after your PR

What This PR Does:
Before (Old Flow - Broken):

1. User runs Cloud Connector setup via UI
2. This triggers a Remote Execution (REX) job
3. When REX job completes → CloudConnectorAnnounceTask fires
4. Sources registration happens automatically

After (New Flow - Fixed):

1. User runs foremanctl deployment with cloud connector setup
2. Immediate registration: foremanctl can call new API endpoint POST /api/v2/rh_cloud/announce_to_sources
3. Backup registration: Daily recurring task checks if registration is needed
4. Sources registration happens reliably