Fixes #22842 - Create .ansible.cfg in /etc/foreman-proxy #415

dLobatog · 2018-03-09T23:24:07Z

Currently we just mention it in the documentation https://github.com/theforeman/theforeman.org/pull/1033/files
but the installer should do this. The directory is used for some Ansible
tmp files

theforeman-bot · 2018-03-09T23:24:13Z

@dLobatog, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

check #22842: Create .ansible in ~foreman-proxy is the correct one
move ticket #22842 from Ansible to the Installer project
or file a new ticket in the Installer project

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).

This message was auto-generated by Foreman's prprocessor

ekohl · 2018-03-09T23:45:51Z

I feel state should not live in /usr. It violates the FHS.

mmoll · 2018-03-10T13:13:11Z

I ran into this problem and I think the only way to work around this differenlty would be to make /usr/share/foreman-proxy writable for the proxy user or to bring the ~/.ansible dir via packaging.

mmoll · 2018-03-10T13:16:08Z

The actual directory is in ${::foreman_proxy::dir}, BTW. It's different on non-Linux OSes.

ekohl · 2018-03-10T13:19:52Z

In vanilla foreman we create files in /var and place a symlink in /usr pointing there. I'd prefer that over writing in /usr. We probably will need to consider a migration but packaging could take care of that.

mmoll · 2018-03-10T13:31:56Z

@dLobatog could you check the vanilla ansible.cfg for file that are put into ~/.ansible per default and add configuration to put these into e.g ./var/lib/foreman-proxy/ansible. That should fix the original problem differently.

mmoll · 2018-03-10T13:35:41Z

Thinking about it again, having ~foreman-proxy/.ansible linked to e.g. /var/lib/foreman-proxy/ansible makes indeed more sense...

ekohl

I still think we should never write in /usr and including it in the installer is wrong.

ekohl · 2018-03-12T11:09:46Z

manifests/plugin/ansible.pp

@@ -38,6 +38,12 @@
    template_path => 'foreman_proxy/plugin/ansible.yml.erb',
  }

+  file { '/usr/share/foreman-proxy/.ansible':


Like @mmoll said, this should be "${::foreman_proxy::dir}

as @lpramuk mentioned we also need .ansible_galaxy for jobs that install roles from galaxy

~foreman-proxy/.ansible ~foreman-proxy/.ansible_galaxy

we need both, the latter is for galaxy jobs

theforeman-bot · 2018-03-12T11:23:42Z

@dLobatog, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

check #22842: Create .ansible in ~foreman-proxy is the correct one
move ticket #22842 from Ansible to the Installer project
or file a new ticket in the Installer project

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).

This message was auto-generated by Foreman's prprocessor

theforeman-bot · 2018-03-12T11:23:43Z

@dLobatog, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

check #22842: Create .ansible in ~foreman-proxy is the correct one
move ticket #22842 from Ansible to the Installer project
or file a new ticket in the Installer project

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).

This message was auto-generated by Foreman's prprocessor

dLobatog

I've updated this to create /var/lib/foreman-proxy/ansible, /var/lib/foreman-proxy/ansible_galaxy and /var/lib/foreman-proxy/ansible.cfg. These are visible files - the links in /usr/share/foreman-proxy/.ansible (etc..) are invisible.

cc @ares @lpramuk @ekohl @mmoll

ekohl · 2018-03-13T16:00:57Z

manifests/params.pp

-      $shell = '/bin/false'
-      $user  = 'foreman-proxy'
+      $dir     = '/usr/share/foreman-proxy'
+      $var_dir = '/var/lib/foreman-proxy'


This should be set for every OS.

ekohl · 2018-03-13T16:01:55Z

manifests/plugin/ansible.pp

@@ -19,13 +19,17 @@
  Stdlib::Absolutepath $ansible_dir = $::foreman_proxy::plugin::ansible::params::ansible_dir,
  Optional[Stdlib::Absolutepath] $working_dir = $::foreman_proxy::plugin::ansible::params::working_dir,
 ) inherits foreman_proxy::plugin::ansible::params {
-  file {"${::foreman_proxy::dir}/.ansible.cfg":
+  file {"${::foreman_proxy::var_dir}/ansible.cfg":


Should this be in the etc dir instead since it's config, not state?

dLobatog

@ekohl Thanks for the review, I have set $var_dir for all OSs now, and also moved ansible.cfg to live in /etc/foreman-proxy/ansible.cfg as it's config. Once this is merged I'll have to update the docs to reflect the changes.

ekohl

We should probably also fix these things in packaging.

ekohl · 2018-03-14T09:51:56Z

manifests/plugin/ansible.pp

    ensure  => file,
    content => template('foreman_proxy/plugin/ansible.cfg.erb'),
    owner   => 'root',
    group   => $::foreman_proxy::user,
    mode    => '0640',
  }
+  ~> file { "${::foreman_proxy::dir}/.ansible.cfg":
+    ensure => link,
+    target => "${::foreman_proxy::etc}/ansible.cfg",


This should include /foreman-proxy like on line 22.

ekohl · 2018-03-14T09:53:43Z

manifests/plugin/ansible.pp

@@ -38,6 +42,26 @@
    template_path => 'foreman_proxy/plugin/ansible.yml.erb',
  }

+  file { "${::foreman_proxy::var_dir}/ansible":


The var_dir isn't created by packaging. It may be best to ensure that in foreman_proxy::config so all plugins can rely on it.

dLobatog · 2018-03-14T10:17:48Z

Updated to make sure var_dir is automatically created & the ansible.cfg file is linked properly.

@ekohl What exactly do you have in mind to fix in packaging?

ekohl · 2018-03-14T10:23:44Z

@dLobatog I think ensuring those directories and symlinks exist is something we should fix in packaging. It's also a better place to migrate any existing data. We'll still need to modify the etc directory here so that still makes sense.

dLobatog

@ekohl Is there anything missing here? The var_dir is created in foreman_proxy::config , however if packaging creates it, the installer would just skip the creation and it's just a check that the file exists and has the correct owner.

dLobatog

Tests are green now

sean797 · 2018-03-21T23:24:52Z

Do we need to migrate any existing data? This could be done in foreman-installer as a pre hook, similar to https://github.com/Katello/katello-installer/blob/master/hooks/pre/30-upgrade.rb

ares · 2018-03-22T06:40:21Z

I think the opposite aproach was requested

sean797 · 2018-03-22T09:23:21Z

Right, sorry - somehow I missed they are symlinks last night 👍

dLobatog

Anything else missing here? @ekohl

ekohl · 2018-04-04T16:44:31Z

theforeman/foreman-packaging#2348 should simplify this. With that you only need to change manifests/plugin/ansible.pp to place the file in /etc instead of /usr. All the other changes can be left out, but ensuring the symlink is a good thing because that means we don't rely on packaging which ensures this is a backwards compatible change.

I'll also have a look at a comparable Debian packaging change.

dLobatog

@ekohl Alrighty, I've moved it to include only the /etc/ change. Everything else is saved in a branch 22842-backup in case we need to use it at some point.

theforeman-bot added the Waiting on contributor label Mar 9, 2018

theforeman-bot added Needs testing Not yet reviewed labels Mar 9, 2018

dLobatog mentioned this pull request Mar 12, 2018

How to set up Ansible SSH keys theforeman/theforeman.org#1033

Closed

ekohl requested changes Mar 12, 2018

View reviewed changes

theforeman-bot added the Waiting on contributor label Mar 12, 2018

theforeman-bot added Waiting on contributor and removed Not yet reviewed labels Mar 12, 2018

dLobatog force-pushed the 22842 branch from 3019233 to 8e89747 Compare March 13, 2018 15:56

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Mar 13, 2018

dLobatog commented Mar 13, 2018

View reviewed changes

ekohl reviewed Mar 13, 2018

View reviewed changes

dLobatog force-pushed the 22842 branch 2 times, most recently from 1cdf94f to c6518ec Compare March 14, 2018 08:27

dLobatog commented Mar 14, 2018

View reviewed changes

ekohl requested changes Mar 14, 2018

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels Mar 14, 2018

dLobatog force-pushed the 22842 branch from c6518ec to 146221a Compare March 14, 2018 10:14

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Mar 14, 2018

dLobatog force-pushed the 22842 branch from 146221a to 95593a9 Compare March 14, 2018 10:17

dLobatog force-pushed the 22842 branch from 95593a9 to ca1be52 Compare March 20, 2018 19:12

dLobatog commented Mar 20, 2018

View reviewed changes

dLobatog mentioned this pull request Mar 20, 2018

Create proxy etc and var directories theforeman/foreman-packaging#2295

Closed

dLobatog force-pushed the 22842 branch 3 times, most recently from 4245535 to f4f9da3 Compare March 21, 2018 20:05

dLobatog commented Mar 21, 2018

View reviewed changes

dLobatog commented Apr 2, 2018

View reviewed changes

dLobatog force-pushed the 22842 branch from f4f9da3 to 23c4e5b Compare April 4, 2018 19:29

dLobatog commented Apr 4, 2018

View reviewed changes

dLobatog changed the title ~~Fixes #22842 - Create .ansible in ~foreman-proxy~~ Fixes #22842 - Create .ansible.cfg in /etc/foreman-proxy Apr 4, 2018

Fixes #22842 - Create .ansible.cfg in /etc/foreman-proxy

1329c8b

dLobatog force-pushed the 22842 branch from 23c4e5b to 1329c8b Compare April 5, 2018 08:09

ekohl approved these changes Apr 5, 2018

View reviewed changes

theforeman-bot removed the Needs re-review label Apr 5, 2018

ekohl merged commit 07d8b52 into theforeman:master Apr 5, 2018

dLobatog deleted the 22842 branch April 5, 2018 11:53

ekohl added the Enhancement label May 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #22842 - Create .ansible.cfg in /etc/foreman-proxy #415

Fixes #22842 - Create .ansible.cfg in /etc/foreman-proxy #415

dLobatog commented Mar 9, 2018

theforeman-bot commented Mar 9, 2018

ekohl commented Mar 9, 2018

mmoll commented Mar 10, 2018

mmoll commented Mar 10, 2018 •

edited

Loading

ekohl commented Mar 10, 2018

mmoll commented Mar 10, 2018

mmoll commented Mar 10, 2018

ekohl left a comment

ekohl Mar 12, 2018

ares Mar 13, 2018

lpramuk Mar 13, 2018

theforeman-bot commented Mar 12, 2018

theforeman-bot commented Mar 12, 2018

dLobatog left a comment

ekohl Mar 13, 2018

ekohl Mar 13, 2018

dLobatog left a comment

ekohl left a comment

ekohl Mar 14, 2018

ekohl Mar 14, 2018

dLobatog commented Mar 14, 2018

ekohl commented Mar 14, 2018

dLobatog left a comment

dLobatog left a comment

sean797 commented Mar 21, 2018

ares commented Mar 22, 2018

sean797 commented Mar 22, 2018

dLobatog left a comment

ekohl commented Apr 4, 2018

dLobatog left a comment

Fixes #22842 - Create .ansible.cfg in /etc/foreman-proxy #415

Fixes #22842 - Create .ansible.cfg in /etc/foreman-proxy #415

Conversation

dLobatog commented Mar 9, 2018

theforeman-bot commented Mar 9, 2018

ekohl commented Mar 9, 2018

mmoll commented Mar 10, 2018

mmoll commented Mar 10, 2018 • edited Loading

ekohl commented Mar 10, 2018

mmoll commented Mar 10, 2018

mmoll commented Mar 10, 2018

ekohl left a comment

Choose a reason for hiding this comment

ekohl Mar 12, 2018

Choose a reason for hiding this comment

ares Mar 13, 2018

Choose a reason for hiding this comment

lpramuk Mar 13, 2018

Choose a reason for hiding this comment

theforeman-bot commented Mar 12, 2018

theforeman-bot commented Mar 12, 2018

dLobatog left a comment

Choose a reason for hiding this comment

ekohl Mar 13, 2018

Choose a reason for hiding this comment

ekohl Mar 13, 2018

Choose a reason for hiding this comment

dLobatog left a comment

Choose a reason for hiding this comment

ekohl left a comment

Choose a reason for hiding this comment

ekohl Mar 14, 2018

Choose a reason for hiding this comment

ekohl Mar 14, 2018

Choose a reason for hiding this comment

dLobatog commented Mar 14, 2018

ekohl commented Mar 14, 2018

dLobatog left a comment

Choose a reason for hiding this comment

dLobatog left a comment

Choose a reason for hiding this comment

sean797 commented Mar 21, 2018

ares commented Mar 22, 2018

sean797 commented Mar 22, 2018

dLobatog left a comment

Choose a reason for hiding this comment

ekohl commented Apr 4, 2018

dLobatog left a comment

Choose a reason for hiding this comment

mmoll commented Mar 10, 2018 •

edited

Loading