While re-imaging worker nodes as part of the ServiceOps vlan migration work T421711: ServiceOps: Re-IP eqiad private baremetal hosts to new per-rack vlans/subnets, we realized that the sre.hosts.reimage cookbook does not support drain and cordon logic needed for k8s worker nodes (by design), since we do worker node reimages in batches using the sre.k8s.roll-reimage-nodescookbook in [0]
As such, we could either implement support --move-vlan in sre.k8s.roll-reimage-nodes or decide on some workaround if we choose not to introduce necessarily interactive changes (out of cookbook homer diffs for example) to the current non interactive workflow in sre.k8s.roll-reimage-nodes.
We could also consider an automated workflow for vlan migrations on k8s worker nodes separate to the sre.k8s.roll-reimage-nodes cookbook altogether. A temporary workaround may be to call the pool-depool-node cookbook [1], reimage with sre.hosts.reimage + --move-vlan, then pool again with pool-depool-node.
Conclusion:
Following some discussion with ServiceOps, we've decided to reuse the current renumber cookbook for vlan migrations on worker nodes, with some changes to the deployment host puppet runs. We considered potentially calling those runs only when the --rename flag is called, however some context in sre.k8s.renumber-node: Run puppet on deploy servers [2] highlights that we run puppet on the deployment hosts to avoid scap failures due to the reimages as well as circumvent the need to sleep while waiting for BGP sessions to establish. Suggesting that we should likely keep these runs as default cookbook behaviour when renumbering nodes with it nonetheless.
We've uploaded a patch [3] to increase the timeout and retry attempts on the deployment host puppet runs. Additionally, sampling through some of the deploy puppet runs on the puppet-board it appears that the average runtime for a host can be anything between 130s to 210s. In conjunction with the retry attempts, this should give us enough room to not fail on those runs consistency.
Action Taken:
The above change has been tested on a series of nodes and merged.
[0] https://gerrit.wikimedia.org/r/plugins/gitiles/operations/cookbooks/+/refs/heads/master/cookbooks/sre/k8s/roll-reimage-nodes.py
[1] https://gerrit.wikimedia.org/r/plugins/gitiles/operations/cookbooks/+/refs/heads/master/cookbooks/sre/k8s/pool-depool-node.py
[2] - https://gerrit.wikimedia.org/r/c/operations/cookbooks/+/1070903
[3] - https://gerrit.wikimedia.org/r/c/operations/cookbooks/+/1307872