gerkElznik commented on PR #1126:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/1126#issuecomment-5248530591
Thanks @gyfora, good question to ask before merging. Short answer: the
override directory keeps working with the old format, and I added a unit test
for it (`FlinkConfigManagerTest#testLegacyFormatConfigOverrides`). Three things
make it work, and the last one is subtle enough that I pinned it in the test:
1. Flink resolves the config file name **per directory**, legacy first:
`GlobalConfiguration.loadConfiguration(dir)` looks for `flink-conf.yaml` and
only falls back to `config.yaml`. This PR only changes the operator's own conf
directory, so an override ConfigMap mounted with a `flink-conf.yaml` key is
still found and parsed by the legacy parser.
2. The merge is programmatic rather than textual. `loadGlobalConfiguration`
parses the override directory into a `Configuration` and passes it in as
`dynamicProperties`, where `addAll` layers it on top of the operator's own
configuration. Two independent parses merged as key/value maps, so there is no
format mixing and no duplicate key hazard, and the overrides still win.
3. `standardYaml` is a process-global in flink-core, set by whichever load
ran last. The override directory is loaded first and the operator's own
directory second, so the effective format is the operator's `config.yaml`. That
ordering is what keeps this safe: if those two calls were ever swapped, a
legacy override directory would quietly put the whole operator back on the
legacy parser. The test asserts `GlobalConfiguration.isStandardYaml()` after
loading so that stays true.
I also ran it in a kind cluster to check operational continuity end to end.
With the operator on `config.yaml` and an external ConfigMap mounted at
`CONF_OVERRIDE_DIR` carrying a `flink-conf.yaml` key:
```
GlobalConfiguration [INFO] Using legacy YAML parser to load flink
configuration file from /opt/flink/conf-override/flink-conf.yaml.
GlobalConfiguration [INFO] Using standard YAML parser to load flink
configuration file from /opt/flink/conf/config.yaml.
GlobalConfiguration [INFO] Loading dynamic configuration property:
kubernetes.operator.reconcile.interval, 42 s
FlinkConfigManager [INFO] Setting default configuration to {...
kubernetes.operator.reconcile.interval=42 s ...}
```
The override was effective and not just loaded: reconcile deltas measured
42.0s, 42.1s, 42.1s, 42.1s against the chart default of 15s.
One nuance worth stating: values coming from a legacy override file are raw
strings that are then read under standard YAML semantics. Scalars are identical
either way. For List and Map typed options flink-core's standard parser tries
YAML first and falls back to the legacy splitter on failure, so legacy forms
like `a;b` and `k1:v1,k2:v2` resolve to the same values. Also worth noting that
the test has to set `FLINK_CONF_DIR`, since without it flink-core returns `new
Configuration(dynamicProperties)` and the format would come from the override
directory instead. The chart sets `FLINK_CONF_DIR` on both containers, so
production always takes the intended path.
On CI: `test_dynamic_flink_conf.sh` was a real failure and is fixed. It
patched the `flink-conf.yaml` key of the operator ConfigMap, which is no longer
mounted, so the patch was invisible and the test waited out its timeout. It now
patches `config.yaml` while still configuring the chart through the legacy
`defaultConfiguration.flink-conf.yaml` values key, so the pair of dynamic
config tests now covers both branches of the resolver. I also noted in the docs
that runtime edits belong to the `config.yaml` key, since that is the one
behavior this PR cannot preserve. The `test_batch_job.sh` failure was
unrelated: it died in the image build when the CRD compatibility checker could
not fetch a released schema from raw.githubusercontent.com, and it needs a
re-run.
@Dennis-Mircea the Flink docs link now points at `flink-docs-master` in both
language versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
