-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Ensure config reload on ..data symlink switch for CSI driver support #127628
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
Draft
jfreden
wants to merge
3
commits into
elastic:main
Choose a base branch
from
jfreden:file_watcher_fix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+65
−19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jfreden
commented
May 2, 2025
@@ -498,6 +503,57 @@ public void testSettingsAppliedOnMasterReElection() throws Exception { | |||
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "43mb"); | |||
} | |||
|
|||
public void testSymlinkUpdateTriggerReload() throws Exception { |
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 test fails on linux without the changes to AbstractFileWatchingService
.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
Hi @jfreden, I've created a changelog YAML for you. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
:Core/Infra/Settings
Settings infrastructure and APIs
>enhancement
Team:Core/Infra
Meta label for core/infra team
v9.1.0
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes an issue that was surfaced when using the new CSI driver (not yet in production) used by the Elasticsearch controller to mount config files as secrets. Unlike the current kubelet-based approach, the CSI driver doesn't trigger an
ENTRY_MODIFY
event on theconfig
file watcher when there are symlink updates inconfig/operator
, causing file changes to go unnoticed by the file-watching service—so the fix ensures a full re-read of theconfig/operator
directory when directory symlinks are updated.The controller mounts
settings.json
,project-xyz.json
andproject-xyz.secrets.json
on the Elastcisearch pods in serverless as kubernetes secrets. This is currently done using the kublet that run on each node. The files are are represented by symlinks in theconfig/operator
directory that point to the files in a symlinked directory. For example:In Elasticsearch,
MultiProjectFileSettingsService
(andFileSettingsService
in non-multi-project environments) is listening for file updates in theconfig
andconfig/operator
through theAbstractFileWatchingService
(by registering a WatchService for the config and operator directories and listen forENTRY_MODIFY
,ENTRY_CREATE
andENTRY_DELETE
). Because of the symlink approach, the symlinks for the files that are monitored are not updated, only the..data
symlink is switched to a different timestamped directory, to allow for an atomic update of all files. When running with kublet the..data
symlink is updated (recreated) in theconfig/operator
directory, theAbstractFileWatchingService
gets an update for theconfig
directory (ENTRY_MODIFY
on theconfig
watch key), this in turn trigger a re-register of theconfig/operator
watch key and a re-read of all files in theconfig/operator
, because we don't know if the file name maps to the same native file system file id.With the new csi-driver, the
..data
symlink switch no longer triggersENTRY_MODIFY
on theconfig
directory aftersettings.json
is changed, so we don't re-register theconfig/operator
watch key and the file changes are never picked up. We're not entirely sure what the difference is in the standard kublet flow vs the csi-driver but I think thatENTRY_MODIFY
on the config directory for the..data
symlink switch might have been accidental and that we should always re-read (check if a file has been updated) the fullconfig/operator
directory every time a directory symlink is updated in the settings directory.I've added a
FileSettingsServiceIT
test to reproduce the issue. This only happens on Linux. On Mac (my local) this doesn't happen because theENTRY_MODIFY
event is triggered for theconfig
watcher when updating theconfig/operator/..data
symlink.