#4452 Migrate Dist Git from Puppet to Ansible
Closed: Fixed None Opened 9 years ago by bochecha.

SSIA.


I've been working on this for some time now, and I think I have something ready to be tested.

I'll be attaching individual patches to this ticket, but for a more graphical overview, see:

https://github.com/bochecha/fedora-infra-ansible/compare/master...pkgs

Just a note: this is the very first time I ever am in contact with Puppet and Ansible, I never used either of them before, never wrote any module for either of them, etc...

I totally expect that I missed some things, made stupid mistakes, and generally got stuff wrong. :)

So, commenting on every patch is gonna take time, so i will just look at them in order :)

On the first patch, a comment say that this part requires fedmsg/base. I think such dependencies should be expressed using a role dependencies :

http://docs.ansible.com/playbooks_roles.html#role-dependencies

And on roles/git/hooks/files/gnome-post-receive-email , there is a module called kitchen. I do not see it being required or installed, so I suspect this would be missing on a new installation.

Patch 2

I think we need to make sure that xinetd is started and enabled ( see the service module :

http://docs.ansible.com/service_module.html )

And in fact, I would put a specific role for xinetd, so you could just have the git-server role requires the xinetd role, who would start xinetd, install it and have a proper handler ( since currently, the notify is not going anywhere IMHO ). See
http://docs.ansible.com/playbooks_intro.html for how handlers work.

patch 3, I am not sure why there is a specific owner change on gitolite directory, shouldn't it be fixed in the package directly ?

On patch 4, I think the yaml around line 10 to be incorrect. IE, you need to have 1 name for each module :
{{{
- name: foo
file: src=foo dest=/etc/foo

  • name: foo 2
    file: src=bar dest=/etc/bar
    }}}

rather than placing the 2 file and copy under the same item ( ie, each time you do a action, you need to have a new yaml element, ie start by - )

Replying to [comment:5 misc]:

patch 3, I am not sure why there is a specific owner change on gitolite directory, shouldn't it be fixed in the package directly ?

Because:

{{{
$ repoquery -l gitolite | grep etc
$
}}}

AIUI, {{{/etc/gitolite}}} is something that is used in the Fedora Dist Git deployment, but it's not where gitolite configuration is necessarily hosted. (for example, the gitolire rc file usuqlly lives in the gitolite user's home directory)

So it's not just an owner change, it's actually creating the directory.

Replying to [comment:3 misc]:

On the first patch, a comment say that this part requires fedmsg/base. I think such dependencies should be expressed using a role dependencies :

http://docs.ansible.com/playbooks_roles.html#role-dependencies

Fixed, didn't know about that, thanks.

Note that I've done that quite often, so I'll go ahead and fix my other patches for this. Do let me know if you find other instances, though.

And on roles/git/hooks/files/gnome-post-receive-email , there is a module called kitchen. I do not see it being required or installed, so I suspect this would be missing on a new installation.

Fixed, good catch.

Replying to [comment:6 misc]:

rather than placing the 2 file and copy under the same item ( ie, each time you do a action, you need to have a new yaml element, ie start by - )

Fixed.

Replying to [comment:4 misc]:

And in fact, I would put a specific role for xinetd, so you could just have the git-server role requires the xinetd role, who would start xinetd, install it and have a proper handler ( since currently, the notify is not going anywhere IMHO ). See
http://docs.ansible.com/playbooks_intro.html for how handlers work.

This one I don't really understand what you mean. I basically just redid the same thing as what's done in other roles/playbooks with regards to services. :-/

There is a handler for xinetd in {{{handlers/restart_services.yml}}}, and it is included in the playbook, like other playbooks (I took example on the Bodhi one).

Should I instead include the handler in the role?

Just fixed patch 8, it was adding lots of empty lines at the end of a file, for no reason (not sure what I did there)

Just fixed patch 12, which contained some of the same mistakes you had found:
not using role dependencies
too many tasks in one

I didn't get to this this week, sorry... ;(

We can look at it at flock if we have time, or just after. ;)

Replying to [comment:13 kevin]:

I didn't get to this this week, sorry... ;(

No worries, I had enough to do fixing the issues Michael found. :)

We can look at it at flock if we have time, or just after. ;)

Unfortunately I'm not going to Flock. :(

ok. I have commited this to ansible git, done some cleanups and tweaks and am trying to build a new pkgs01.stg. ;)

Will let you know if I run into any issues. Once it's up we will want to test it out quite a lot...

Reopening, as I found some stuff passed through the cracks (e.g stuff I corrected in new patch versions, but the old ones were applied, or things that got done in puppet after the patches were made/applied).

I'll attach the new patch series. :)

Login to comment on this ticket.

Metadata