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
[PATCH 02/13] Add a new git/server role 0002-Add-a-new-git-server-role.patch
[PATCH 03/13] Add a new gitolite/base role 0003-Add-a-new-gitolite-base-role.patch
[PATCH 05/13] Add a new cgit/clean_lock_cron role 0005-Add-a-new-cgit-clean_lock_cron-role.patch
[PATCH 06/13] Add a new clamav role 0006-Add-a-new-clamav-role.patch
[PATCH 07/13] Add a new gitolite/check_fedmsg_hooks role 0007-Add-a-new-gitolite-check_fedmsg_hooks-role.patch
[PATCH 09/13] Add a new git/make_checkout_seed role 0009-Add-a-new-git-make_checkout_seed-role.patch
[PATCH 10/13] Add the confine_ssh task 0010-Add-the-confine_ssh-task.patch
[PATCH 11/13] Add a drbackupkey task 0011-Add-a-drbackupkey-task.patch
[PATCH 13/13] Setup the production and staging Dist Git 0013-Setup-the-production-and-staging-Dist-Git.patch
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
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 - )
[PATCH 01/13] Add a new git/hooks role 0001-Add-a-new-git-hooks-role.patch
Replying to [comment:5 misc]:
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.
Fixed, good catch.
Replying to [comment:6 misc]:
Fixed.
[PATCH 04/13] Add a new cgit/base role 0004-Add-a-new-cgit-base-role.patch
Replying to [comment:4 misc]:
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?
[PATCH 08/13] Add a new cgit/make_pkgs_list role 0008-Add-a-new-cgit-make_pkgs_list-role.patch
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)
[PATCH 12/13] Add a new distgit role 0012-Add-a-new-distgit-role.patch
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]:
No worries, I had enough to do fixing the issues Michael found. :)
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. :)
git/hooks: Add missing package dependency 0001-git-hooks-Add-missing-package-dependency.patch
git/hooks: Express the role dependency properly 0002-git-hooks-Express-the-role-dependency-properly.patch
cgit/make_pkgs_list: Remove useless new lines 0003-cgit-make_pkgs_list-Remove-useless-new-lines.patch
Add support for packaging groups 0004-Add-support-for-packaging-groups.patch
Setup for F21 branching 0005-Setup-for-F21-branching.patch
distgit: Express the role dependency properly 0006-distgit-Express-the-role-dependency-properly.patch
distgit: One action per task 0007-distgit-One-action-per-task.patch
To make things a bit easier, the new patches are:
Applied and pushed
Login to comment on this ticket.