Some notes:

The stale copies of those cookbooks are deployed to the cumin hosts, with the risk of potentially running stale/obsolete code.
There are currently cookbooks in the master branch that are deployed to the cumin hosts that can't be run from there as they will fail because they are supposed to be run from the local environment of a WMCS SRE.

As you mentioned in the chat, only wmcs ever runs those tasks, and we do know which ones should be run locally and which can be run on cumin. The risk you mention imo is very low (and the impact is very little).
Btw. I'd prefer to call them frozen, not stale, as they are actively monitored, maintained and tracked.

As I'm about to remove the deprecated icinga.Icinga class in favor of the icinga.IcingaHosts one, I found that I would need to update both copies of the wmcs and master branches. Apart the double patch (that is not a big problem ofc), this will also create more conflicts when rebasing the wmcs branch on top of the master one.

Removing the tasks will make the rebasing even harder, and the merge later too.

I propose doing the Icinga patch on master, rebase the wmcs branch, and continue with the current setup, keeping the code frozen until the decision to merge or not is taken (as previously discussed). That avoids extra diverging of the branches and minimizes the effort needed to merge in the future.

In T287465#7239650, @dcaro wrote:

Removing the tasks will make the rebasing even harder, and the merge later too.

What do you mean? I don't understand the english, sorry.

In T287465#7239650, @dcaro wrote:

Some notes:

The stale copies of those cookbooks are deployed to the cumin hosts, with the risk of potentially running stale/obsolete code.
There are currently cookbooks in the master branch that are deployed to the cumin hosts that can't be run from there as they will fail because they are supposed to be run from the local environment of a WMCS SRE.

As you mentioned in the chat, only wmcs ever runs those tasks, and we do know which ones should be run locally and which can be run on cumin. The risk you mention imo is very low (and the impact is very little).
Btw. I'd prefer to call them frozen, not stale, as they are actively monitored, maintained and tracked.

I disagree, the cookbooks for which the development has gone forward in the wmcs branch and not reflected in the master branch are effectively stale in the master version of them. If I pick for example the wmcs/openstack/cloudvirt/unset_maintenance.py cookbook between the wmcs version and the master version of it. They are quite different, the wmcs one has got 7 additional commits. It has also an obvious bug at line 90 for example, that was fixed in a later commit in the wmcs branch. How can the master version of it be considered "actively monitored, maintained and tracked" ?

What do you mean? I don't understand the english, sorry.

Rephrasing: Removing the tasks from the master branch will make rebasing the wmcs branch on top of it harder, and merging the wmcs branch onto master at a later time harder too.

How can the master version of it be considered "actively monitored, maintained and tracked" ?

It's frozen, so no changes can go to it (as per our agreement), but it's actively monitored, maintained and tracked in order to not allow any patches in, to rebase on top of any repo-wide refactorings done to any of them. If there's any critical issue with them (ex. breaking CI, breaking other tasks) they will get patched too. The wmcs branch is being also actively developed, that's why there's differences (and we are not allowed to backport any of those changes to master yet, so expect it to keep diverging).

In T287465#7239783, @dcaro wrote:

What do you mean? I don't understand the english, sorry.

Rephrasing: Removing the tasks from the master branch will make rebasing the wmcs branch on top of it harder, and merging the wmcs branch onto master at a later time harder too.

If for tasks you mean the cookbooks, I don't get it. How removing the entire wmcs/ subtree can make any rebase harder given that there is guarantee of no conflicts at all as the branch has a path that is not present in master?

How removing the entire wmcs/ subtree can make any rebase harder given that there is guarantee of no conflicts at all as the branch has a path that is not present in master?

Because the branch was made when the subtree existed, so there's already quite a bit of git history that would be lost.

Because the branch was made when the subtree existed, so there's already quite a bit of git history that would be lost.

Have been following along and appolagise up front iof im just adding noise however I think I'm missing something here. Tthe git history will still be preserved as in, you will still be able to see all the log messages and diff between files. The fact that wmcs is deleted will of course cause a conflict when merging as the files are deleted in one branch but on the other. however i think this is fairly easy to fix e.g.

$ git rm -r cookbooks/wmcs/
rm 'cookbooks/wmcs/__init__.py'
rm 'cookbooks/wmcs/openstack/__init__.py'
rm 'cookbooks/wmcs/openstack/cloudvirt/set_maintenance.py'
rm 'cookbooks/wmcs/openstack/cloudvirt/unset_maintenance.py'
rm 'cookbooks/wmcs/toolforge/__init__.py'
rm 'cookbooks/wmcs/toolforge/add_etcd_node.py'
rm 'cookbooks/wmcs/toolforge/etcd/__init__.py'
rm 'cookbooks/wmcs/toolforge/etcd/add_node_to_cluster.py'
rm 'cookbooks/wmcs/toolforge/etcd/add_node_to_hiera.py'
rm 'cookbooks/wmcs/toolforge/etcd/depool_and_remove_node.py'
rm 'cookbooks/wmcs/toolforge/etcd/remove_node_from_hiera.py'
rm 'cookbooks/wmcs/toolforge/remove_etcd_node.py'
rm 'cookbooks/wmcs/toolforge/start_instance_with_prefix.py'
rm 'cookbooks/wmcs/vps/__init__.py'
rm 'cookbooks/wmcs/vps/refresh_puppet_certs.py'
rm 'cookbooks/wmcs/vps/remove_instance.py'
rm 'cookbooks/wmcs/wikireplicas/__init__.py'
rm 'cookbooks/wmcs/wikireplicas/add_wiki.py'
$ git commit -a -m'remove wmcs directory'                                         
[master 3711dd8] remove wmcs directory
 18 files changed, 1884 deletions(-)
 delete mode 100644 cookbooks/wmcs/__init__.py
 delete mode 100644 cookbooks/wmcs/openstack/__init__.py
 delete mode 100644 cookbooks/wmcs/openstack/cloudvirt/set_maintenance.py
 delete mode 100644 cookbooks/wmcs/openstack/cloudvirt/unset_maintenance.py
 delete mode 100644 cookbooks/wmcs/toolforge/__init__.py
 delete mode 100644 cookbooks/wmcs/toolforge/add_etcd_node.py
 delete mode 100644 cookbooks/wmcs/toolforge/etcd/__init__.py
 delete mode 100644 cookbooks/wmcs/toolforge/etcd/add_node_to_cluster.py
 delete mode 100644 cookbooks/wmcs/toolforge/etcd/add_node_to_hiera.py
 delete mode 100644 cookbooks/wmcs/toolforge/etcd/depool_and_remove_node.py
 delete mode 100644 cookbooks/wmcs/toolforge/etcd/remove_node_from_hiera.py
 delete mode 100644 cookbooks/wmcs/toolforge/remove_etcd_node.py
 delete mode 100644 cookbooks/wmcs/toolforge/start_instance_with_prefix.py
 delete mode 100644 cookbooks/wmcs/vps/__init__.py
 delete mode 100644 cookbooks/wmcs/vps/refresh_puppet_certs.py
 delete mode 100644 cookbooks/wmcs/vps/remove_instance.py
 delete mode 100644 cookbooks/wmcs/wikireplicas/__init__.py
 delete mode 100644 cookbooks/wmcs/wikireplicas/add_wiki.py
$ git merge wmcs                                                          
CONFLICT (modify/delete): cookbooks/wmcs/vps/refresh_puppet_certs.py deleted in HEAD and modified in wmcs. Version wmcs of cookbooks/wmcs/vps/refresh_puppet_certs.py left in tree.
CONFLICT (modify/delete): cookbooks/wmcs/toolforge/start_instance_with_prefix.py deleted in HEAD and modified in wmcs. Version wmcs of cookbooks/wmcs/toolforge/start_instance_with_prefix.py left in tree.
CONFLICT (modify/delete): cookbooks/wmcs/openstack/cloudvirt/unset_maintenance.py deleted in HEAD and modified in wmcs. Version wmcs of cookbooks/wmcs/openstack/cloudvirt/unset_maintenance.py left in tree.
CONFLICT (modify/delete): cookbooks/wmcs/openstack/cloudvirt/set_maintenance.py deleted in HEAD and modified in wmcs. Version wmcs of cookbooks/wmcs/openstack/cloudvirt/set_maintenance.py left in tree.
CONFLICT (modify/delete): cookbooks/wmcs/__init__.py deleted in HEAD and modified in wmcs. Version wmcs of cookbooks/wmcs/__init__.py left in tree.
Automatic merge failed; fix conflicts and then commit the result.
$ git add cookbooks/wmcs/   
$ git merge --continue 
[master 3c25c12] Merge branch 'wmcs'

We can also rebase the wmcs branch after we have removed the wmcs folder (this was a bit more involved but still < 5 minutes work), which would make merging wmcs backing master at a later date a none issue.

Both theses options seem much less painful then having to update two separate branches with the same change.

You'll have to keep applying the changes/refactors to both branches, that will not go away until they are merged (if Icinga gets replaced by IcingaHosts, the change will have to happen also on the wmcs branch).

So there's no benefit there.

In T287465#7240495, @dcaro wrote:

You'll have to keep applying the changes/refactors to both branches, that will not go away until they are merged (if Icinga gets replaced by IcingaHosts, the change will have to happen also on the wmcs branch).

Considering the current case of deprecating icinga.Icinga class from what i can tell in the current set up we would need to:

• create a commit to update everything under sre
• create a commit to update everything under wmcs directory in the master branch
  • of course this could be the same commit as above but the point is its updating an additional set of files
• git checkout wmcs; git rebase -i master
  • not sure if this is done currently however this merge is likely going to be more painful [then the proposal below] as there are modifications in both branches
• then make a commit to change to any additional files under the wmcs directory in the wmcs branch

If we go with proposal from the original post then we would have

• create a commit to update everything under sre
• git checkout wmcs; git rebase -i master
  • this should be painless as, AFAIK, the wmcs branch doesn't change files under sre and the master branch doesn't change files under wmcs
• then a commit make a change to the files under the wmcs directory in the wmcs branch

To me it seems like going with the later proposal i.e. removing the wmcs directory in master then doing a git rebase -i master in the wmcs branch, is a cheep cost to pay now and will make rebasing and a final merge much easier in the future. what are the down sides of doing this? or phrased differently what are the benefits of keeping the wmcs directory in the master branch?

Sure, I'm exhausted, do whatever you feel more comfortable with, let me know if there's anything needed on my side.

I should mention here that the original purpose of the wmcs prefix in the master branch is to enable running of the add_wiki cookbook by those who have admin access to do the manual tasks on the Wikireplicas databases T261145

If you move it elsewhere, please update the sudo definition and perhaps the secure_cookbook wrapper as needed.

In T287465#7244334, @Bstorm wrote:

I should mention here that the original purpose of the wmcs prefix in the master branch is to enable running of the add_wiki cookbook by those who have admin access to do the manual tasks on the Wikireplicas databases T261145

Thanks brooke, the original proposal was to do either of the following:

1. remove all wmcs/** cookbooks from the master branch and all additional code in wmcs/__init__py leaving just the add_wiki cookbook there.
2. move the add_wiki cookbook into the sre/ subtree and remove entirely the wmcs/ subtree from the master branch.

Personally i have a slight presence for option two however i think both are workable if WMCS (or riccardo for that matter :)) would prefer option one? That said i have had to work on some puppetdb issues the last two days and im on vacation next week so will leave this for riccardo to pick up again when he is back in.

If you move it elsewhere, please update the sudo definition and perhaps the secure_cookbook wrapper as needed.

noted

Thank you all for such an active conversation. Myself and Nicholas would like to follow up on this thread and sum up the conversation with next steps that we sign under - please proceed with moving the add_wiki cookbook into the sre/ subtree and remove entirely the wmcs/ subtree from the master branch.

Thanks,
Joanna & Nicholas

Change 714797 had a related patch set uploaded (by Volans; author: Volans):

[operations/cookbooks@master] wmcs.wikireplicas.add_wiki: rename

https://gerrit.wikimedia.org/r/714797

Change 714798 had a related patch set uploaded (by Volans; author: Volans):

[operations/cookbooks@master] wmcs: remove wmcs/ subtree

https://gerrit.wikimedia.org/r/714798

Change 714799 had a related patch set uploaded (by Volans; author: Volans):

[operations/puppet@production] admin: update sudo rule for renamed cookbook

https://gerrit.wikimedia.org/r/714799

Change 714797 merged by Volans:

[operations/cookbooks@master] wmcs.wikireplicas.add_wiki: rename

https://gerrit.wikimedia.org/r/714797

Change 714799 merged by Volans:

[operations/puppet@production] admin: update sudo rule for renamed cookbook

https://gerrit.wikimedia.org/r/714799

Change 714798 merged by jenkins-bot:

[operations/cookbooks@master] wmcs: remove wmcs/ subtree

https://gerrit.wikimedia.org/r/714798

The above patches have all been merged and deployed.
The add_wiki cookbook is now available as sre.wikireplicas.add-wiki. The related documentation at https://wikitech.wikimedia.org/wiki/Add_a_wiki#Best_method has been updated.
@nskaggs @bd808: when you get a chance could you test that you can still run it with the new name? The sudo rule for the secure-cookbook wrapper has been updated too.

Leaving it open for confirmation that all works fine.

@Volans, as far as I can tell, I can no longer use this cookbook. The sudo rule no longer seems to work.

Change 715271 had a related patch set uploaded (by Volans; author: Volans):

[operations/puppet@production] admin: fix typo in sudo rule

https://gerrit.wikimedia.org/r/715271

Change 715271 merged by Volans:

[operations/puppet@production] admin: fix typo in sudo rule

https://gerrit.wikimedia.org/r/715271

@nskaggs sorry for the trouble, there was a typo in the puppet patch, it should be fixed now.