Add migration subcommand by archanaserver · Pull Request #446 · theforeman/foremanctl

archanaserver · 2026-04-08T08:50:10Z

Fixes #52

Create foremanctl migration subcommand to convert foreman-installer answer files to foremanctl YAML configuration.

Mappings include:

Database configuration (host, port, credentials, mode)
Foreman admin credentials
Certificate paths

Implements SAT-43246

ehelms · 2026-04-08T14:48:12Z

Good to reference the repository issue -- #52

ehelms · 2026-04-08T14:48:59Z

@@ -0,0 +1,128 @@
+# Migrate Command


I'd consider making this guide about migrating from foreman-installer to foremanctl rather than the command itself.

ehelms · 2026-04-08T14:50:07Z

+
+### Command Options
+
+- `--answer-file PATH` - Path to the foreman-installer answer file (default: `/etc/foreman-installer/scenarios.d/satellite.yaml`)


Suggested change

- `--answer-file PATH` - Path to the foreman-installer answer file (default: `/etc/foreman-installer/scenarios.d/satellite.yaml`)

- `--answer-file PATH` - Path to the foreman-installer answer file (default: `/etc/foreman-installer/scenarios.d/foreman-answers.yaml`)

ehelms · 2026-04-08T14:51:38Z

+
+- `--answer-file PATH` - Path to the foreman-installer answer file (default: `/etc/foreman-installer/scenarios.d/satellite.yaml`)
+- `--output PATH` - Path for the migrated configuration (default: stdout)
+- `--root PATH` - Root directory for finding the answer file, useful for migrations from backups (default: /)


Is the expectation when using --root that there is a directory that has this /backup/foreman-answers.yaml ? If so, would it be better to make the user be explicit here and just have the --answer-file parameter?

Hmm I see the --root parameter adds little complexity but my intent was to simplify backup/restore scenarios where you have a full system backup mounted (e.g., at /backup) - using --root /backup is shorter than typing the full path. However, I agree the explicit --answer-file approach is clearer. Does foreman-installer have a similar --root parameter that users might expect? If not, I'm happy to remove it and keep things simple - just using --answer-file makes the code easier to understand. What's your preference?

ex:

With --root: foremanctl migrate --root /backup --output config.yaml

Without --root: foremanctl migrate --answer-file /backup/etc/foreman-installer/scenarios.d/last_scenario.yaml --output config.yaml

I think using --root here and pointing it at a backup gives a false idea of the migration concept/flow. A backup (as it's taken today) does not contain the answer file directly -- you'd need to unpack config_files.tar.gz and fetch the relevant yamls from there. And while at it, also grab the certs, as you'll need them for the migration too.

Ideally foremanctl will be able to do all that: point it at a valid backup and it will restore it including the migration from packages/puppet to containers/ansible. But I'd not put it here in this PR and focus solely on providing an entrypoint to start the "simple" "in-place" migration.

ehelms · 2026-04-08T14:52:26Z

+foremanctl migrate --output /tmp/new-config.yaml
+
+# Migrate from custom location
+foremanctl migrate --answer-file /path/to/answers.yaml --output /tmp/config.yaml


My first thought is what do I as a user do with this file once I generate it?

Good catch! Updated doc now makes this clear upfront.

ehelms · 2026-04-08T14:56:20Z

+help: |
+  Migrate foreman-installer answer file to foremanctl configuration format.
+
+  This command reads the old Satellite 6.18 answer file and converts it to the


This is overly Satellite specific.

evgeni · 2026-04-15T13:46:25Z

+
+    - name: Run migration
+      migrate_answers:
+        answer_file: "{{ answer_file | default(resolved_answer_file) }}"


I would have done answer_file | default(omit) and do the "load last_scenario and find answer file there" logic in the migrate_answers module instead.

not blocking, just an opinion :)

yeah, sounds clean moving all the logic in the module. on it

Gauravtalreja1 · 2026-04-15T10:55:28Z

+
+## Overview
+
+When upgrading from foreman-installer to foremanctl, the `foremanctl migrate` command helps convert your existing configuration to the new format.


Do you think it would be good to have steps here to install foremanctl on foreman-installer based setups?

not sure about it, but my thinking is installation steps are probably better suited for the main README or deployment docs rather than the migration guide docs or can add a simple "prerequisites" section with a link to installation docs. WDYT?

@ehelms @evgeni thoughts?

I think since this guide is about how to upgrade from foreman-installer to foremanctl, that it should cover those steps for now. Like, prerequisites (e.g. foreman-installer installed, foremanctl present on the system) then run the steps. Or point to docs and say what steps should be run.

Gauravtalreja1 · 2026-04-15T11:04:38Z

+  answer_file:
+    help: Path to the foreman-installer answer file to migrate. If not specified, attempts to read from the default location.
+  output:
+    help: Path where the migrated configuration should be written. If not specified, outputs to stdout.


If not specified, it outputs to stdout currently, so just a thought should we also put those into default config /var/lib/foremanctl/parameters.yaml as well?

I kept stdout as the default for the safer side, so that users can review output before it overwrites existing config. However, I believe we could document the recommended command: foremanctl migrate --output /var/lib/foremanctl/parameters.yaml. What's your preference?

That logic makes sense to me.

Gauravtalreja1 · 2026-04-15T14:05:29Z

+        - name: Read scenario file
+          ansible.builtin.slurp:
+            src: "{{ default_scenario_file }}"
+          register: scenario_content
+


Currently, if I touch any random file and run the migration by passing that file with --answer-file, it passes with Migration completed successfully!, so I suggest adding a YAML validation check before reading it, to check if its valid yaml and valid answers file

Gauravtalreja1 · 2026-04-15T14:15:22Z

+        - name: Parse scenario file
+          ansible.builtin.set_fact:
+            scenario_data: "{{ scenario_content['content'] | b64decode | from_yaml }}"
+
+        - name: Extract answer file path from scenario
+          ansible.builtin.set_fact:
+            resolved_answer_file: "{{ scenario_data[':answer_file'] | default('') }}"
+


Can we combine this both set_fact tasks, as scenario_data seems redundant, can you try something like
(scenario_content.content | b64decode | from_yaml).get(':answer_file', '')?

not blocking, just an opinion to optimize?

already addressed! I moved all scenario file reading logic to the module

Gauravtalreja1 · 2026-04-15T14:28:26Z

+    - name: Run migration
+      migrate_answers:
+        answer_file: "{{ answer_file | default(resolved_answer_file) }}"
+        output: "{{ output | default(omit) }}"


if I pass --output diff.yaml , where file doesn't exist previously, and the file gets created under /usr/share/foremanctl/src/playbooks/migrate/diff.yaml instead of present working directory, is this expected?

no it was not expected, i see a bug now, let me fix that and address other comments as well

ehelms · 2026-04-15T14:46:18Z

@@ -0,0 +1,129 @@
+# Migrating from foreman-installer to foremanctl


Given the title and focus of the doc, I think the filename could use an update to reflect this.

ehelms · 2026-04-15T14:48:33Z

+- `--answer-file PATH` - Path to the foreman-installer answer file. If not specified, reads the currently active scenario and extracts the answer file path from it.
+- `--output PATH` - Path for the migrated configuration (default: stdout)
+
+## Parameter Mappings


We have an overlap now with parameters.md. I think either combine these two, or link out and capture this information in there.

i've linked it, looks better?

evgeni · 2026-04-17T10:02:54Z

+        if isinstance(value, dict):
+            items.extend(flatten_nested_dict(value, key).items())


This removes the "root" key from the result, which makes it harder to detect enabled plugins (you can detect them by being "non False", so an empty dict is an enabled plugin.

Suggested change

if isinstance(value, dict):

items.extend(flatten_nested_dict(value, key).items())

if isinstance(value, dict):

items.append(key, True)

items.extend(flatten_nested_dict(value, key).items())

This would add key with a True value to the list and we could then later process it.

At the same time, this is nothing that we need in the initial "here is the command", so feel free to say we add it later.

Hmm I was checking that earlier, it's small change but gonna differ here, and needs testing and more. but let's add this enhancement in follow-up to keep this PR focused on the migration functionality for now.

I've added this to my list of things that we need to implement for the final version.

You mean in that prepare work epic related to this one, right?

evgeni · 2026-04-17T11:35:52Z

+
+Before migrating, ensure the following:
+
+1. **foreman-installer is installed** - You should have an existing foreman-installer setup with answer files to migrate from.


I'd phrase that a bit differently, as the main point is not that foreman-installler is installed, but you have a Foreman installed by foreman-installer, so how about:

Suggested change

1. **foreman-installer is installed** - You should have an existing foreman-installer setup with answer files to migrate from.

1. **Foreman deployment using foreman-installer** - You should have an existing Foreman deployment has been installed using foreman-installer and has an answers file to migrate from.

evgeni · 2026-04-17T11:43:59Z

+```yaml
+database_host: database.example.com
+database_port: 5432
+database_mode: internal


Suggested change

database_mode: internal

database_mode: external

not that it technically matters, but your example uses a db host, so this is external db :)

evgeni · 2026-04-17T11:44:54Z

+Warning: The following parameters could not be mapped:
+  - katello::enable_ostree
+  - foreman::some_other_param
+```


the warning format changed in one of the recent pushes, so this probably deserves an update to match code

evgeni · 2026-04-17T11:46:24Z

There is a failing test, and I posted a few nitpicks, but I think this reflects what we wanted to expose as the first version of the migrate command (read: ACK once my comments are addressed ;) )

Implements [SAT-43246](https://redhat.atlassian.net/browse/SAT-43246): Create foremanctl migration subcommand to convert foreman-installer answer files to foremanctl YAML configuration. Mappings include: - Database configuration (host, port, credentials, mode) - Foreman admin credentials - Certificate paths

evgeni · 2026-04-21T13:06:10Z

+    # Certificate configuration
+    ('foreman', 'server_ssl_cert'): 'server_certificate',
+    ('foreman', 'server_ssl_key'): 'server_key',
+    ('foreman', 'server_ssl_ca'): 'ca_certificate',


I think this mapping is wrong, we should rather map the data from certs (so ('certs', 'server_cert'), (…, 'server_key') and (…, 'server_ca_cert') to the custom cert variables introduced in #421, but we don't need to wait on that and can update the mapping later, the current setup does not break anything

ACK, I'll update the certificate mappings in a follow-up PR.

ehelms · 2026-04-22T14:18:37Z

+
+When the migration completes, you may see warnings like:
+
+> [!WARNING]  


This is going to be tricky in practice. Users will need guidance on parameters that could not be mapped. Perhaps for now we can include as part of the warning output the high-level possible reasons they could not be mapped?

For example:

There was an error trying to map it

The value has no equivalent parameter / is unsupported

other reasons?

I think that's something that we'll have to learn (and document) as we go.

Yeah, that is something we can focus on follow up work as well. Thanks for pointing it out.

archanaserver · 2026-04-23T10:42:50Z

All green now, can we get this in, or is there anything else this needs to be covered? @evgeni @ehelms

pr-processor Bot added the Not yet reviewed label Apr 8, 2026

ehelms reviewed Apr 8, 2026

View reviewed changes

pr-processor Bot removed the Not yet reviewed label Apr 8, 2026

ehelms reviewed Apr 8, 2026

View reviewed changes

Comment thread docs/migrate-command.md Outdated

ehelms reviewed Apr 8, 2026

View reviewed changes

Comment thread docs/migrate-command.md Outdated

ehelms reviewed Apr 8, 2026

View reviewed changes

Comment thread docs/migrate-command.md Outdated

ehelms reviewed Apr 8, 2026

View reviewed changes

Comment thread src/playbooks/migrate/migrate.yaml Outdated

archanaserver force-pushed the create-migration-subcommand branch from 89cac1f to 10c4843 Compare April 14, 2026 10:07

archanaserver requested review from ehelms and evgeni April 14, 2026 10:22