-
Notifications
You must be signed in to change notification settings - Fork 22
make features and flavors configurable #188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,25 @@ For example, pre-pulling images to reduce the core deployment utility runtime. | |
| 6. Run deployment utility | ||
| 7. Post deploy checks | ||
|
|
||
| ### Features and Flavors | ||
|
|
||
| To allow deployments with different sets of functionality enabled, the deployment utility supports features and flavors. | ||
|
|
||
| - A feature is an abstract representation of "the deployed system can now do X", usually implemented by enabling a Foreman/Pulp/Hammer plugin (or a collection of these). | ||
| - A flavor is a set of features that are enabled by default and can not be disabled. This is to allow common deployment types like "vanilla foreman", "katello", "satellite" and similar. | ||
|
Comment on lines
+30
to
+31
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A flavor is something you deploy first and then add features on top of it? If so, I suggest you switch the order in which you list them (here and elsewhere, like in the PR's description or on line 28).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yepp. I mainly listed them here in that order as |
||
|
|
||
| Additionally to the functionality offered by plugins, we define the following "base" features: | ||
| - `foreman` to deploy the main Rails app and make the deployment a "Server" | ||
| - `foreman-proxy` to deploy the Foreman Proxy code | ||
| - `hammer` to deploy the base CLI | ||
|
|
||
| These base features control which plugins are enabled when a feature is requested. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "when a feature is requested" -> Did you mean to say "profile" here? If not, what'd be a valid "profile" at this point?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR only defines one: katello. |
||
| - `foreman` + `remote_execution` = `foreman_remote_execution` | ||
| - `foreman-proxy` + `remote_execution` = `smart_proxy_remote_execution_ssh` | ||
| - `hammer` + `remote_execution` = `hammer_cli_foreman_remote_execution` | ||
|
|
||
| A deployment can have multiple base features enabled. | ||
|
|
||
| ### Authenticated Registry Handling | ||
|
|
||
| In the non-default case where the image sources are supplied from an authenticated location users will need to inject a login step. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| --- | ||
| variables: | ||
| flavor: | ||
| help: Base flavor to use in this deployment. | ||
| features: | ||
| parameter: --add-feature | ||
|
Comment on lines
+5
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also consider adding a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #337 is tracking this, yes |
||
| help: Additional features to enable in this deployment. | ||
| action: append_unique | ||
| remove_features: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a task for listing available features?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also pointed that out in #188 (comment). @evgeni have talked offline and IIRC he wanted to tackle that in a follow up which I'm good with.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, LGTM, @evgeni, Can we create an issue for it so we can track it?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| parameter: --remove-feature | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this going to work? For example I'll run: How does the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Today it will just make that feature "unmanaged", like in the old installer.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can you please create an issue or Jira ticket so we can keep track of it? thx
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| help: Additional features to disable in this deployment. | ||
| action: remove | ||
| dest: features | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| become: true | ||
| vars_files: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| - "../../vars/defaults.yml" | ||
| - "../../vars/flavors/{{ flavor }}.yml" | ||
| - "../../vars/{{ certificate_source }}_certificates.yml" | ||
| - "../../vars/images.yml" | ||
| - "../../vars/database.yml" | ||
|
|
@@ -29,4 +30,10 @@ | |
| - pulp | ||
| - foreman | ||
| - role: systemd_target | ||
| - role: foreman_proxy | ||
| when: | ||
| - "'foreman-proxy' in enabled_features" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In our current installer we have the "unmanaged" state and I think this confuses users. Should we make it explicitly managed and ensure it's stopped/absent otherwise?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is certainly an improvement we can do, but probably not in this PR. |
||
| - role: hammer | ||
| when: | ||
| - "'hammer' in enabled_features" | ||
| - post_install | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,3 +17,4 @@ include: | |
| - _database_mode | ||
| - _database_connection | ||
| - _tuning | ||
| - _flavor_features | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,3 +4,4 @@ help: | | |
|
|
||
| include: | ||
| - _database_mode | ||
| - _flavor_features | ||
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,6 +105,7 @@ | |
| - 'foreman-client-key,type=mount,target=/etc/foreman/client_key.pem' | ||
| env: | ||
| FOREMAN_PUMA_WORKERS: "{{ foreman_puma_workers }}" | ||
| FOREMAN_ENABLED_PLUGINS: "{{ foreman_plugins | join(' ') }}" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious, how this env var for foreman_plugins will be used on the containers ?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The plugins inside the container have the following snippet in their respective bundler files: gem '$PLUGIN' if ENV.fetch('FOREMAN_ENABLED_PLUGINS', '').split.include?('$PLUGIN') || ENV.fetch('FOREMAN_ENABLED_PLUGINS', nil).nil?Which essentially means:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, thanks |
||
| quadlet_options: | ||
| - | | ||
| [Install] | ||
|
|
@@ -135,6 +136,7 @@ | |
| env: | ||
| DYNFLOW_REDIS_URL: "redis://localhost:6379/6" | ||
| REDIS_PROVIDER: "DYNFLOW_REDIS_URL" | ||
| FOREMAN_ENABLED_PLUGINS: "{{ foreman_plugins | join(' ') }}" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And, is this required on Dynflow containers as well? do we enable any plugins on those containers too?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need the same code loaded in the dynflow containers, as otherwise plugin-specific tasks wouldn't work correctly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, thanks |
||
| command: "/usr/libexec/foreman/sidekiq-selinux -e production -r ./extras/dynflow-sidekiq.rb -C /etc/foreman/dynflow/%i.yml" | ||
| quadlet_options: | ||
| - | | ||
|
|
@@ -173,6 +175,8 @@ | |
| - bin/rails db:migrate && bin/rails db:seed | ||
| detach: false | ||
| network: host | ||
| env: | ||
| FOREMAN_ENABLED_PLUGINS: "{{ foreman_plugins | join(' ') }}" | ||
| secrets: | ||
| - 'foreman-database-url,type=env,target=DATABASE_URL' | ||
| - 'foreman-seed-admin-user,type=env,target=SEED_ADMIN_USER' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,7 @@ | ||
| --- | ||
| hammer_foreman_server_url: "https://{{ ansible_facts['fqdn'] }}" | ||
| hammer_ca_certificate: "" | ||
| hammer_packages: | ||
| - hammer-cli-plugin-foreman | ||
| - hammer-cli-plugin-foreman_tasks | ||
| - hammer-cli-plugin-foreman_remote_execution | ||
| - hammer-cli-plugin-katello | ||
| hammer_default_plugins: | ||
| - foreman | ||
| hammer_plugins: [] | ||
| hammer_packages: "{{ (hammer_default_plugins+hammer_plugins) | map('regex_replace', '^', 'hammer-cli-plugin-') }}" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| flavor_features: | ||
| - foreman | ||
| - katello | ||
| - content/container | ||
| - content/rpm |
Uh oh!
There was an error while loading. Please reload this page.