Wouldn't this make any subsequent changes in Nix be ignored silently?
cc @johanot

@srhb Probably yes, but that's what you want if you have local changes. It's quite easy to track upstream changes:

1. Save the relevant maps
2. Delete them
3. Let addon-manager recreate them
4. Merge in local changes

I just worry that it's unintuitive from the perspective of NixOS. I'll let @johanot chime in though, since he's actually using Kubernetes in production, and I'm not. :)

@srhb I'm running kubernetes in production, and I spent quite a few hours trying to come up with a sensible and stable workaround (like using dnsmasq on the host(s)) which would not require messing with the defaults. Nothing really worked out. So unfortunately this was the only practical solution left in the end.

@juselius Isn't your usecase an argument for opting out of the kubernetes.dns module completely, i.e. services.kubernetes.addons.dns.enable = false and then deploy the dns service manually or add a custom addon to the addon manager with EnsureExists? I'd argue that Reconcile is the intuitive choice from a declarative point of view.

@johanot Well, yes and no. It's much easier to track nixos when a) all you have to do is delete the deployment and configmap to get the new defaults b) ad you have a patch yaml for local changes. But I see the problem now. Everybody without local modifications end up in dire straits. Would a an option e.g. "allowLocalChanges" make the patch more palatable?

Well yes - especially if the default behavior is preserved.

I think it would be slightly prettier with e.g. mode, type = enum [ "Reconcile" "EnsureExists" ];, default = "Reconcile", but that's just opinion. :)

@johanot Good idea! I'll post a new PR.

Thanks @johanot!

@GrahamcOfBorg eval

@juselius Can you please squash the entire changeset down to one commit?

@johanot done!

Oh.. Sorry I didn't see this before. The commit message should be prefixed with nixos/kubernetes:, instead of kubernetes:, since it involves changes to the NixOS module, not the package.

I find usecases in PR description more compelling than current option description. For a new guy like me, it is not clear why this once setting is exported as NixOS option, but others are not.

Another problem is that only some of "addons" allow this override. That looks a bit ad-hoc. Maybe note about that in option description as well? Otherwise it would require looking into module code to find out what this is for.

@danbst Good suggestion! I have added a line in the description about a use case. My opinion regarding the "ad-hocness" of the override, is that if you need it in the future, do it in the future. There is little point in adding options for things nobody will ever need, just for symmetry's sake.

@johanot Shall we try to get this merged in, before it goes stale again?

@juselius Fine by me, but I don't have merge access :-)
@srhb Do you have any further comments? If not, will you please assist in merging and back-porting?

Thanks @danbst !

@juselius as for me PR description is still more clear about what this option is for. But maybe you k8s guys use some other language, so 🤷‍♂️ problem with me I guess.

backported

First NixOS PR! Time to celebrate :)

@johanot Oh, I thought you had merge access.

@danbst Thanks! Perhaps you are right regarding the description. But the PR description is just one of many uses. This is nonetheless not an option a k8s newbie normally would find useful (I believe).