calyptia: switch to YAML for Fleet config #9698
Conversation
Signed-off-by: Patrick Stephens <[email protected]>
Signed-off-by: Patrick Stephens <[email protected]>
Signed-off-by: Patrick Stephens <[email protected]>
Signed-off-by: Patrick Stephens <[email protected]>
Signed-off-by: Patrick Stephens <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Patrick Stephens <[email protected]>
Signed-off-by: Patrick Stephens <[email protected]>
|
Verified that we can use TOML by default if undefined, also verified we can switch from YAML to TOML even with existing YAML config. |
patrick-stephens
requested review from
pwhelan,
niedbalski,
edsiper,
leonardo-albertovich,
fujimotos and
koleini
as code owners
patrick-stephens
had a problem deploying
to
pr
GitHub Actions
Failure
Signed-off-by: Patrick Stephens <[email protected]>
|
Testing on Windows shows an issue on reloading the first time with the new config so investigating. |
There was a problem hiding this comment.
I've added some minor observations inline in the code.
However, my main suggestion is around test coverage. We should add unit tests for fleet_config_get and get_calyptia_fleet_config functions, similar to how we validate input properties in test_set_fleet_input_properties.
Specifically, I'd like to see tests that (maybe in a similar approach to custom calyptia input tests).
- Verify the generated config content matches expectations given different context configurations test both INI/TOML and YAML output formats
- Validate header inclusion.
Also, I suggest using -DSANITIZE_ADDRESS=On -DSANITIZE_UNDEFINED=On when compiling and run the tests with it.
| TEST_CHECK(value != NULL); | ||
| TEST_MSG("fleet_config_legacy_format changed expected=%s got=%s", expectedValue, value); | ||
| TEST_CHECK(value && strcasecmp(value, expectedValue) == 0); | ||
|
|
There was a problem hiding this comment.
expectedValue doesn't get freed.
| * limitations under the License. | ||
| */ | ||
|
|
||
| #ifndef FLB_IN_CALYPTIA_FLEET |
There was a problem hiding this comment.
This must be FLB_IN_CALYPTIA_FLEET_H as in HEADER convention
| @@ -1687,6 +1655,9 @@ flb_sds_t fleet_config_get(struct flb_in_calyptia_fleet_config *ctx) | |||
| flb_ctx_t *flb = flb_context_get(); | |||
| flb_sds_t fleet_id = NULL; | |||
|
|
|||
| if( !ctx ) { | |||
There was a problem hiding this comment.
missing a whitespace after if.
Updates to using YAML for Fleet configuration now it is completed in 3.2 series.
We have added this as a configurable option now with the default set to the current TOML to keep backwards behaviour - users can opt-in to YAML format and then we can also switch the default in the future.
fleet_config_legacy_format on== current INI/TOML format and the defaultfleet_config_legacy_format off== YAML formatIn each case note the initial configuration will be assumed to be TOML, not changing installation and packaging for that.
Includes changes from cprofiles to resolve macOS failures, these should not cause an issue once that PR is merged: fluent/cprofiles#7
Resolves the usage of
booltype in the plugin and replaces with standardintflag approach.Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Confirmed all fleet config files are YAML:
$ sudo find /tmp/calyptia-fleet/ /tmp/calyptia-fleet/ /tmp/calyptia-fleet/dc86c10731acee376c73985ff88b8ec86888e045d6af2ccf045f94b55e834be4 /tmp/calyptia-fleet/dc86c10731acee376c73985ff88b8ec86888e045d6af2ccf045f94b55e834be4/yaml-fleet /tmp/calyptia-fleet/dc86c10731acee376c73985ff88b8ec86888e045d6af2ccf045f94b55e834be4/yaml-fleet/1733416866 /tmp/calyptia-fleet/dc86c10731acee376c73985ff88b8ec86888e045d6af2ccf045f94b55e834be4/yaml-fleet/cur.yaml /tmp/calyptia-fleet/dc86c10731acee376c73985ff88b8ec86888e045d6af2ccf045f94b55e834be4/yaml-fleet/header.yaml /tmp/calyptia-fleet/dc86c10731acee376c73985ff88b8ec86888e045d6af2ccf045f94b55e834be4/yaml-fleet/1733416866.yaml /tmp/calyptia-fleet/machine-id.conf $ sudo cat /tmp/calyptia-fleet/dc86c10731acee376c73985ff88b8ec86888e045d6af2ccf045f94b55e834be4/yaml-fleet/1733416866.yaml includes: - /tmp/calyptia-fleet/dc86c10731acee376c73985ff88b8ec86888e045d6af2ccf045f94b55e834be4/yaml-fleet/header.yaml pipeline: inputs: - name: dummy dummy: '{"message": "custom dummy"}' outputs: - name: stdout match: '*'If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.