Page MenuHomePhabricator

bad failure cases for wmcs custom puppet enc
Open, HighPublic

Description

I just now broke the labspuppet backend API such that it failed to match any host. When that happens I'd like puppet runs to fail, but what happens instead is that puppet runs but assumes a default config. Can get pretty messy.

Event Timeline

As far as I understand the bug that triggered this, from the client side it would have looked exactly the same as an instance with no roles applied. I'm not sure what kind of client validation check could have halted the runs really.

nskaggs triaged this task as High priority.Oct 20 2020, 4:35 PM

Just found a frustrating side effect of this. Hosts that are supposed to not have traffic shaping and NFS do because once that's applied there is no going back without lots of surgery on the VM or simply scrapping it and rebuilding.

On ingress and proxy nodes that makes them potentially problematic.

Just found a frustrating side effect of this. Hosts that are supposed to not have traffic shaping and NFS do because once that's applied there is no going back without lots of surgery on the VM or simply scrapping it and rebuilding.

On ingress and proxy nodes that makes them potentially problematic.

Can we not flip the default so that if the API fails, it defaults to not having NFS (which shouldn't remove anything I'd think) instead of adding NFS?

Can we not flip the default so that if the API fails, it defaults to not having NFS (which shouldn't remove anything I'd think) instead of adding NFS?

We totally could. However, that is a change in behavior for other Cloud tenants, so maybe we'd want to include commits to /labs/private for existing tenants as part of it so their behavior doesn't change unless they want it to. I do think "opt-in" NFS mounts makes much more sense.

Hmm, we don't even need to do that because things don't unmount. It only affects new VMs. Maybe we could flip the default with an announcement instead.

Change 639297 had a related patch set uploaded (by Bstorm; owner: Bstorm):
[operations/puppet@production] cloud-vps: Change NFS mounts to default to false

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

Change 639297 merged by Bstorm:
[operations/puppet@production] cloud-vps: Change NFS mounts to default to false

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

legoktm> could you have a enabled: true hiera key or something that the enc sets and which puppet will refuse to run if not present? to distinguish a barf with no hieradata from instance has no extra hieradata?

legoktm> could you have a enabled: true hiera key or something that the enc sets and which puppet will refuse to run if not present? to distinguish a barf with no hieradata from instance has no extra hieradata?

+1

I'd name it 'safeguard/safecheck/encgenerated/generatedbyenc: "hostname.of.the.enc"/generatedat: "YYYY-mm-dd HH:MM:SS" ...' or something a bit more meaningful, but I think it's a good idea :)