feat(installation): Add task to remove db migrations by nadjaheitmann · Pull Request #89 · ATIX-AG/foreman_ansible_director

nadjaheitmann · 2026-04-24T11:11:39Z

Preparation for plugin deinstallation

Refs similar task in ATIX-AG/foreman_resource_quota@37b7c99

Did not intend to approve

Thorben-D · 2026-04-27T15:52:15Z

+    plugin = Foreman::Plugin.find ForemanAnsibleDirector.name.underscore
+    ActiveRecord::MigrationContext.new(plugin.migrations_paths, ActiveRecord::SchemaMigration).down
+  end
+end


I don't think this will cut it.
Quite a bit more needs to happen for proper cleanup, as Pulp objects also have to be removed.
I see two options:

Query all Pulp Ansible Repositories, CollectionRemotes, GitRemotes, and Distributions where the label creator is foreman_ansible_director

Iterate over ContentUnitVersions and destroy the associated Pulp objects.

I intend to add a consistency check in general, which will do a mix of both, so this problem may already be solved in the future.

nadjaheitmann · 2026-05-08T09:01:37Z

+        def pulp_proxy_url!
+          proxy =
+            ::SmartProxy.unscoped.with_features('Ansible_Director')&.find { |smart_proxy| smart_proxy.url.present? } ||
+            ::SmartProxy.unscoped.find { |smart_proxy| smart_proxy.url.present? }
+          return proxy.url if proxy&.url.present?
+
+          raise Foreman::Exception,
+            'No Smart Proxy with a URL is available for Foreman Ansible Director Pulp cleanup.'
+        end
+


I don't think that we want this part of the code in the way it is written here. It misses the organization filtering.

sbernhard · 2026-05-08T09:21:23Z

+          return proxy.url if proxy&.url.present?
+
+          raise Foreman::Exception,
+            'No Smart Proxy with a URL is available for Foreman Ansible Director Pulp cleanup.'


the pulp_proxy_url is not only related to the pulp cleanup. adjust error message.

sbernhard · 2026-05-08T09:22:10Z

      class << self
+        def pulp_proxy_url!
+          proxy =
+            ::SmartProxy.unscoped.with_features('Ansible_Director')&.find { |smart_proxy| smart_proxy.url.present? } ||


the second find should not be necessary.

sbernhard · 2026-05-08T09:24:15Z

        def pulp3_configuration(config_class)
          config_class.new do |config|
-            uri = URI.parse(::SmartProxy.first.url)
+            uri = URI.parse(pulp_proxy_url!)


would be worth to keep this PR in mind: #95

sbernhard · 2026-05-08T09:28:02Z

+    ::ForemanAnsibleDirector::ContentUnitVersion.find_each do |content_unit_version|
+      ::ForemanAnsibleDirector::TaskHelpers.destroy_content_unit_version_pulp_objects(
+        content_unit_version,
+        deleted_hrefs


what is the purpose of this deleted_hrefs?

sbernhard · 2026-05-08T09:29:07Z

+      role_remote: Set.new,
+    }
+
+    ::ForemanAnsibleDirector::ContentUnitVersion.find_each do |content_unit_version|


It need to cleanup the Execution Environments, too.

sbernhard · 2026-05-08T09:30:36Z

+  module TaskHelpers
+    module_function
+
+    def destroy_pulp_object(deleted_hrefs, key, href, action, input_key)


Is it posible to re-use the introduced labels 1be49d4#diff-bc0ac4dcf2708e3d510050a532b88b8bde8d3e97af27d3b9bde58b62ea777005R13 to find and remove the pulp content / images?

You mean we could have a call to Pulp which simply removes everything with one specific label?

yes. this should be possible - for each remote / distribution / publication / etc.

Thorben-D · 2026-05-08T10:46:14Z


 require 'pulpcore_client'
 module ForemanAnsibleDirector
  module Pulp3


Why is this file being touched at all?
It is an integral requirement of this plugin that Pulp (via Katello) exists on Foreman server.
For this plugin to function at all, the first smart proxy (provisioned on Foreman server) must provide a pulp_ansible API at it's URL.

Use of the Pulp instances of other smart proxies is not and will not be supported.

Actually, I'm unsure if this is a requirement in general or just now that there is a Smart Proxy Ansible Director on the Foreman server. Can you explain why?

Thanks for clarification, I'll remove it. I don't think it is necessary for the issue at hand.

It won't work in the rake context without adjusting the smart proxy urls. I don't completely understand why this is happening but it seems the code is somehow running in another scope during the rake task execution and the proxy does not resolve correctly.

As mentioned before, there is no guarrantee that the Pulp instance used to store Ansible content has the Ansible_Director feature.
This config will break for all cases where the Smart Proxy provisioned on Foreman Server (id: 1, used to store content) does not have the smart_proxy_ansible_director plugin installed.

But we need to change this code in a way that it works in rake scope. I will give it another thought.

nadjaheitmann · 2026-05-13T09:32:31Z

    engine_name 'foreman_ansible_director'

+    initializer 'foreman_ansible_director.cleanup_context' do |app|
+      app.config.x.foreman_ansible_director.running_in_rake = false


About the config.x thing: https://guides.rubyonrails.org/configuring.html#custom-configuration

The idea is that we set a flag here and change it in the Rake task. We can evaluate this in the code and choose a smart proxy accordingly. Turns out this works for the smart proxy base client part but not for the pulp3 base client part.

In case we want to remove the plugin, we need clearly defined names for the settings, so we do not accidently remove some setting from another plugin or core.

Preparation for plugin deinstallation Refs similar task in ATIX-AG/foreman_resource_quota@37b7c99 Also adds task to clean up the created Pulp content.

nadjaheitmann · 2026-05-13T12:15:24Z

        def pulp3_configuration(config_class)
          config_class.new do |config|
-            uri = URI.parse(::SmartProxy.first.url)
+            uri = URI.parse(::SmartProxy.unscoped.first.url)


I don't really like this. But it does not seem to work otherwise, even if I add a Rails flag. Maybe because the task is created in rake env but then scheduled from the Rails application. Not sure what exactly happens here. However, something similar seems to exist in Katello:
https://github.com/Katello/katello/blob/master/app/models/katello/concerns/smart_proxy_extensions.rb#L88

nadjaheitmann · 2026-05-13T12:17:08Z

+    def ensure_ansible_director_proxy!
+      proxies = ansible_director_proxies
+      missing_feature_message =
+        'No Smart Proxy with the Ansible_Director feature is available for Foreman Ansible Director Pulp cleanup.'
+      raise Foreman::Exception, missing_feature_message unless records_exist?(proxies)
+
+      proxy = proxies.first
+      return if proxy&.url.present?
+
+      missing_url_message =
+        'No Smart Proxy with a configured URL is available for Foreman Ansible Director Pulp cleanup.'
+      raise Foreman::Exception, missing_url_message
+    end


I know there is something similar in the proxy base client but I thought it might be a good idea to check on the proxy before even starting a task not when the task is running or whenever this is evaluated.

nadjaheitmann marked this pull request as draft April 24, 2026 11:49

nadjaheitmann force-pushed the deinstallation_task branch from ee1d81d to f7e2c60 Compare April 24, 2026 12:31

nadjaheitmann marked this pull request as ready for review April 24, 2026 12:38

nadjaheitmann requested a review from Thorben-D April 27, 2026 13:36

Thorben-D requested changes Apr 27, 2026

View reviewed changes

nadjaheitmann force-pushed the deinstallation_task branch from f7e2c60 to ad04db3 Compare May 8, 2026 08:57

nadjaheitmann commented May 8, 2026

View reviewed changes

sbernhard reviewed May 8, 2026

View reviewed changes

Thorben-D requested changes May 8, 2026

View reviewed changes

nadjaheitmann mentioned this pull request May 8, 2026

feat(actions/execution_environments): use first valid proxy with Ansi… #95

Open

nadjaheitmann force-pushed the deinstallation_task branch 9 times, most recently from d583b0e to b2ec0e8 Compare May 13, 2026 09:26

nadjaheitmann commented May 13, 2026

View reviewed changes

nadjaheitmann force-pushed the deinstallation_task branch 2 times, most recently from 2fbd6e6 to 7118691 Compare May 13, 2026 12:11

fix(settings): prefix all settings with 'ansible_director_'

c0c77fe

In case we want to remove the plugin, we need clearly defined names for the settings, so we do not accidently remove some setting from another plugin or core.

nadjaheitmann force-pushed the deinstallation_task branch from 7118691 to 4a8f44b Compare May 13, 2026 12:12

feat(installation): Add task to remove db migrations and pulp content

efa13db

Preparation for plugin deinstallation Refs similar task in ATIX-AG/foreman_resource_quota@37b7c99 Also adds task to clean up the created Pulp content.

nadjaheitmann force-pushed the deinstallation_task branch from 4a8f44b to efa13db Compare May 13, 2026 12:13

nadjaheitmann commented May 13, 2026

View reviewed changes

Conversation

nadjaheitmann commented Apr 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbernhard May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nadjaheitmann May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sbernhard May 8, 2026 •

edited

Loading

nadjaheitmann May 13, 2026 •

edited

Loading