-
Notifications
You must be signed in to change notification settings - Fork 0
in_calyptia_fleet: Fix inability to start up agent when invalid fleet config fetched #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
92d74a7 to
0687187
Compare
a1960d3 to
5d37835
Compare
0687187 to
202e8ec
Compare
5d37835 to
b355c89
Compare
202e8ec to
1d14800
Compare
3612a47 to
bf72ac0
Compare
f938e2d to
1a8e426
Compare
bf72ac0 to
46b6764
Compare
1a8e426 to
d88caa4
Compare
e0dc0cb to
dd75e84
Compare
| if (new_config_path != NULL | ||
| && is_new_fleet_config(ctx, config) == FLB_FALSE) { | ||
| flb_plg_warn(ctx->ins, "deleting uncommitted new config: %s", new_config_path); | ||
| if (calyptia_config_delete_by_ref(ctx, "new") == FLB_FALSE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also rollback old to curr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that's unncessary as we fallback to old in load_fleet_config() if curr doesn't exist
| reload->flb->config->conf_path_file = reload->cfg_path; | ||
|
|
||
| flb_free(reload); | ||
| sleep(5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we sleep?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, and this feels like a Chesterton's fence situation. @pwhelan do you have history on this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I call sleep just to give the actual reload time to be executed. It might be totally unnecessary.
|
|
||
| /** | ||
| * Commits the latest received config as the valid, now-current config. | ||
| * This updates the symlink to the current config file to point to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * This updates the symlink to the current config file to point to the | |
| * This updates the ref file for the current config file to point to the |
Or similar
| flb_sds_t cur_ref_filename = fleet_config_ref_filename(ctx, "cur"); | ||
| if (cur_ref_filename != NULL) { | ||
| unlink(cur_ref_filename); | ||
| if (exists_fleet_config(ctx, "cur") == FLB_TRUE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? shouldn't this be guarded on the ref file existing rather than the referenced config existing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I've updated this function a bit as a result and also tweaked the earlier check around cleaning up uncommitted new config.
d88caa4 to
d5478e8
Compare
dd75e84 to
02e2d1f
Compare
d5478e8 to
cd546f3
Compare
02e2d1f to
7c2a445
Compare
cd546f3 to
f61fd78
Compare
7c2a445 to
1345945
Compare
f61fd78 to
ce07178
Compare
1345945 to
bbb50c9
Compare
ce07178 to
05cc743
Compare
bbb50c9 to
97cebc4
Compare
05cc743 to
a391f55
Compare
97cebc4 to
d9d0604
Compare
a391f55 to
2e540e5
Compare
d9d0604 to
f8329cd
Compare
2e540e5 to
38b11b7
Compare
f8329cd to
6f74614
Compare
38b11b7 to
7883904
Compare
6f74614 to
a64a7c3
Compare
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Alec Holmes <[email protected]>
Signed-off-by: Biao Zhu <[email protected]>
Remove a redundant wget in yum install Signed-off-by: ilove737 <[email protected]>
Signed-off-by: Alec Holmes <[email protected]>
a64a7c3 to
4989c58
Compare
|
PR below this has been merged into OSS so I'm going to close this PR and recreate it in the fluent-bit repo. |
This fixes a fleet bug where fetching an invalid config then shutting down prevented the agent from starting up again.
Problem
The fleet plugin is buggy today in that it will immediately commit newly received config as the current config, implying the new config is valid. This can notably prevent fluent-bit from restarting correctly after receiving an invalid config.
For example:
Change
This PR changes how configs received by the fleet plugin are committed as valid.
When a new config is received:
When a process starts up:
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:
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).Documentation
Backporting
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.