|operations/puppet||production||+1 -0||ores: Notify ores services when the config changes|
I 've left comments in the change, but to answer the question of the strategy, keep in mind this moves orchestration of restarts to puppet meaning puppet will be restarting services if the config changes. Depending on the timing of the puppet agents runs (some might coincide, and there is also the chance of a force puppet run by SRE) it might cause issues. We 've been approaching this up to now on a case by case basis, moving away from puppet restarting services when we want more control over the change deployment process. For ORES specifically up to now we seldom had to do so, with the notable exception of the migration of settings to celery version 4.
Overall, I am fine with moving forward with this, assuming the puppet aspect is done better.
My biggest problem is that it's not documented that config changes in puppet doesn't cause the service to restart. If I knew that. I would do it gradually. My (wrong) assumption was that changing /etc/ores/*.yaml would trigger a restart. If it's clear somewhere (like ores documentation). I'm fine with just abandoning my patch and calling this invalid as current method gives more power to the deployer to test changes in a more careful manner.