Page MenuHomePhabricator

Rename ACAST_PS_ADVERTISE in bird and anycast-healthchecker to BIRD_IP_ADVERTISE
Closed, DeclinedPublic

Description

Parent task with the complete description is: T348041.

The summary is that our current configuration uses ACAST_PS_ADVERTISE everywhere but since we will be advertising unicast IPs from bird as well, that variable name will no longer be correct. We should use the generic BIRD_IP_ADVERTISE instead so that we ascribe the correct usage to it.

The rollout of this change means that anycast-healthchecker.service will be restarted and therefore subsequently bird.service because of the systemd bindings. Thus, we have to be careful in rolling this out, making sure to disable Puppet on all hosts that use bird and roll this out progressively, with -b1 -s30 or something similar in cumin.

sukhe@cumin2002:~$ sudo cumin 'C:bird'
49 hosts will be targeted:
centrallog2002.codfw.wmnet,centrallog1002.eqiad.wmnet,cloudlb[2001-2003]-dev.codfw.wmnet,cloudlb[1001-1002].eqiad.wmnet,cloudservices[2004-2005]-dev.codfw.wmnet,cloudservices[1005-1006].eqiad.wmnet,dns[1004-1006,2004-2006,3003-3004,4003-4004,5003-5004,6001-6002].wikimedia.org,doh[1001-1002,2001-2002,3003-3004,4001-4002,5001-5002,6001-6002].wikimedia.org,durum[2001-2002].codfw.wmnet,durum[6001-6002].drmrs.wmnet,durum[1001-1002].eqiad.wmnet,durum[5001-5002].eqsin.wmnet,durum[3003-3004].esams.wmnet,durum[4001-4002].ulsfo.wmnet

We will have to rename it in the following places:

modules/bird/templates/bird_anycast.conf.erb
function match_route()
{
    return net ~ ACAST_PS_ADVERTISE;
}
<% if @do_ipv6 -%>
function match_route6()
{
    return net ~ ACAST6_PS_ADVERTISE;
}
<% end -%>

And then:

modules/bird/templates/anycast-healthchecker.conf.erb
bird_variable          = ACAST_PS_ADVERTISE
bird6_variable         = ACAST6_PS_ADVERTISE

Event Timeline

Change 963385 had a related patch set uploaded (by Ssingh; author: Ssingh):

[operations/puppet@production] bird: rename ACAST_PS_ADVERTISE to BIRD_IP{4,6}_ADVERTISE

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

Thanks, I think the scope should be larger than just those two variables if we want to remove the term "anycast" as much as possible.
For example class profile::bird::anycast needs to be renamed to something like class profile::hosts_bgp::main (it was a bad choice of me to hardcode the name of the tool in the profile path too).

Note that as those prefixes are advertised from multiple hosts they will become anycasted at the scale of the datacenter or switch fabric, they're just not global (either internally or externally).

Thanks, I think the scope should be larger than just those two variables if we want to remove the term "anycast" as much as possible.
For example class profile::bird::anycast needs to be renamed to something like class profile::hosts_bgp::main (it was a bad choice of me to hardcode the name of the tool in the profile path too).

Note that as those prefixes are advertised from multiple hosts they will become anycasted at the scale of the datacenter or switch fabric, they're just not global (either internally or externally).

Thanks for the feedback. I am a bit split about this but overall I think I am fine with class profile::bird::anycast because even for the unicast stuff, it is anycast-hc that is going to do the healthchecks and the advertisement of the IP does depend on that. Relatedly we also call bird::anycast_healthchecker_check from there and which what sets up the healthchecks, so I think I am fine with leaving that.

I am bit worried about scope creep here as we have discussed in the previous tickets. But more importantly I guess, I am thinking how much we should focus on removing anycast from everywhere vs just from the thing that matters the most, such as the bird prefixes file.

After some consideration about what to rename as part of this effort, I think I am now in favour of not renaming anything for the simple reason that it's hard to define what to rename and renaming just the variables doesn't seem enough. The word anycast is hardcoded in quite a few places and so is the dependence on the tool as well: whether it's an anycast or unicast address, the tool is still the same.

Unless there are strong objections and given the extent of this change for little feature-gain, I am going to skip the renaming part.

Change 963385 abandoned by Ssingh:

[operations/puppet@production] bird: rename ACAST_PS_ADVERTISE to BIRD_IP{4,6}_ADVERTISE

Reason:

as mentioned in the task: abandoning any and all renames for now

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

As mentioned above, abandoning this rename pursuit. There is a lot of stuff to rename and we won't get to it all, so renaming small bits of pieces here and there doesn't seem fruitful. If someone disagrees, please feel free to reopen it and we can discuss.