Pods stuck waiting for ConfigMap updates due to missing ca-file key

[rbtcollins]

Not sure what let to this situation; Background: We updated the operator for 0.4 to 0.13, and we'd had tls on and then disabled it due to expired certs and too many things to chase at once.

The stateless pods were not getting cluster updates because the kubelet could't project the ConfigMap to disk. This is because there was a reference to the key ca-file in the pod definition, but the ConfigMap no longer had a key ca-file.

The pod definition

``` - configMap: defaultMode: 420 items: - key: fdbmonitor-conf-stateless path: fdbmonitor.conf - key: cluster-file path: fdb.cluster - key: ca-file path: ca.pem name: fdb-test-cluster-config name: config-map ```

The config map

``` apiVersion: v1 data: cluster-file: fdb_test_cluster:i4PTILcQyoeJ9mYxUxLKRASCoZI0zTuL@10.244.18.22:4501,10.244.17.22:4501,10.244.25.22:4501,10.244.23.18:4501,10.244.26.20:4501 fdbmonitor-conf-log: |- [general] kill_on_configuration_change = false restart_delay = 60 [fdbserver.1] command = $BINARY_DIR/fdbserver cluster_file = /var/fdb/data/fdb.cluster seed_cluster_file = /var/dynamic-conf/fdb.cluster public_address = $FDB_PUBLIC_IP:4501 class = log datadir = /var/fdb/data logdir = /var/log/fdb-trace-logs loggroup = fdb-test-cluster locality_instance_id = $FDB_INSTANCE_ID locality_machineid = $FDB_MACHINE_ID locality_zoneid = $FDB_ZONE_ID knob_relocation_parallelism_per_source_server=20 knob_fetch_keys_parallelism_bytes=100000000 knob_available_space_ratio_cutoff=0.03 knob_min_available_space_ratio=0.03 fdbmonitor-conf-stateless: |- [general] kill_on_configuration_change = false restart_delay = 60 [fdbserver.1] command = $BINARY_DIR/fdbserver cluster_file = /var/fdb/data/fdb.cluster seed_cluster_file = /var/dynamic-conf/fdb.cluster public_address = $FDB_PUBLIC_IP:4501 class = stateless datadir = /var/fdb/data logdir = /var/log/fdb-trace-logs loggroup = fdb-test-cluster locality_instance_id = $FDB_INSTANCE_ID locality_machineid = $FDB_MACHINE_ID locality_zoneid = $FDB_ZONE_ID knob_relocation_parallelism_per_source_server=20 knob_fetch_keys_parallelism_bytes=100000000 knob_available_space_ratio_cutoff=0.03 knob_min_available_space_ratio=0.03 fdbmonitor-conf-storage: |- [general] kill_on_configuration_change = false restart_delay = 60 [fdbserver.1] command = $BINARY_DIR/fdbserver cluster_file = /var/fdb/data/fdb.cluster seed_cluster_file = /var/dynamic-conf/fdb.cluster public_address = $FDB_PUBLIC_IP:4501 class = storage datadir = /var/fdb/data logdir = /var/log/fdb-trace-logs loggroup = fdb-test-cluster locality_instance_id = $FDB_INSTANCE_ID locality_machineid = $FDB_MACHINE_ID locality_zoneid = $FDB_ZONE_ID knob_relocation_parallelism_per_source_server=20 knob_fetch_keys_parallelism_bytes=100000000 knob_available_space_ratio_cutoff=0.03 knob_min_available_space_ratio=0.03 running-version: 6.2.15 sidecar-conf: '{"ADDITIONAL_SUBSTITUTIONS":null,"COPY_BINARIES":["fdbserver","fdbcli"],"COPY_FILES":["fdb.cluster"],"COPY_LIBRARIES":[],"INPUT_MONITOR_CONF":"fdbmonitor.conf"}' kind: ConfigMap metadata: creationTimestamp: "2020-06-12T09:18:37Z" labels: fdb-cluster-name: fdb-test-cluster name: fdb-test-cluster-config namespace: fdb-test ownerReferences: - apiVersion: apps.foundationdb.org/v1beta1 controller: true kind: FoundationDBCluster name: fdb-test-cluster uid: a3ecd3a9-5ae3-4d19-b209-1b73b39142be resourceVersion: "80768592" selfLink: /api/v1/namespaces/fdb-test/configmaps/fdb-test-cluster-config uid: b918e22a-aed8-4d16-b352-b5015d5988ba ```

I think to guard against this, always putting the ca-file key in the ConfigMap would be a sensible precaution; then it can be projected as an empty file during transitions.

[brownleej]

I think this is a problem with transitioning from an older version of the operator. We used to put an empty file in the config map, and set it as the CA file through the pod's environment variables, but this caused a problem when the FDB TLS code was refactored in a way that made it reject an empty CA file. We definitely could have handled this transition more smoothly. We've been working to establish development practices to make those kind of changes controllable through settings in the spec, with defaults that preserve the old version. That will enable us to do a sweeping change in defaults as part of the major version upgrade, while allowing anyone to opt out of those changes by setting explicit values in the spec.

There also may be a problem in the transition between TLS and non-TLS. I haven't tested this, but I suspect you might run into problems if you are setting the CA file and enabling TLS in a single update to the spec. Setting the CA file will require a rolling bounce, because we have to both change the config map volume definition and add environment variables to the pod. This rolling bounce might get blocked by needing to change the coordinators to use TLS, which would hit problems if the processes didn't have valid TLS configuration. The workaround for this is to add all of the TLS certificate configuration, wait for that to fully reconcile, and then enable TLS. At a minimum we should test and document this. I don't know how much we can make it fundamentally better, since we don't know what values someone should be using for their TLS certs. Maybe we could also inspect the pod config and throw an error if someone tries to enable TLS without having the certs configured on all the pods.

[rbtcollins]

I think ideally we wouldn't do TLS at all and let folk do it via mTLS in service meshes; but thats a different discussion.

In this particular case, if the configmap had an empty key, but new pods didn't project that key, the code in FDB that checks for empty TLS files would not get triggered, and everything should work smoothly.

Anyhow - we're past this now, but the concern is whether we can trigger it again in future.

[brownleej]

Yeah, I think that defining the key but not projecting it would be a good way to avoid brittle coupling between config map keys and pod updates. It would still require a rolling bounce to add the CA file to the pods, which would potentially cause problems if we did it at the same time as we enabled TLS.

[johscheuer]

We should verify if that is still possible in the current version of thee operator, if not we can close this issue.

[johscheuer]

I'm going to close this issue since wee haven't observed this issue. Feel free to reopen the issue or create a new one. Thanks!