diff --git a/lib/ldclient-rb/impl/file_data/watcher.rb b/lib/ldclient-rb/impl/file_data/watcher.rb index b1d5a474..e1f7d079 100644 --- a/lib/ldclient-rb/impl/file_data/watcher.rb +++ b/lib/ldclient-rb/impl/file_data/watcher.rb @@ -28,10 +28,15 @@ module FileData # watches the directory of the link's target for the target's name, so that an edit to the # target is reported. The links are resolved again after every reported change, after a retry # sets up watches, and when a directory that holds a target is deleted, as happens when a - # Kubernetes ConfigMap volume is updated. So a link that comes to point elsewhere is followed. - # A link that cannot be resolved, for example because its target's directory does not exist - # yet or was deleted, is resolved again once per second until its target is watched, and the - # callback then runs once. + # Kubernetes ConfigMap volume is updated. A link that cannot be resolved, for example because + # its target's directory does not exist yet or was deleted, is resolved again once per second + # until its target is watched, and the callback then runs once. + # + # A change that none of those events reports is not followed: a link further along the chain + # that is pointed elsewhere while its old target stays, a configured directory that is itself + # a link and is pointed elsewhere, or a configured directory that is renamed and replaced. + # Deployments that switch files that way, such as a `current` link to a release directory, + # should use polling, which reads every configured path on each interval. # # @private # @@ -274,7 +279,9 @@ def stop # The targets of links that changed are dropped, and a watch that served only such targets # is removed. A link that cannot be resolved, for example because its target's directory # does not exist yet, or whose target's directory cannot be watched, keeps the retry - # running, which resolves the links again. Returns true if it added a watch. + # running, which resolves the links again. Returns true if it added a watch or if a link + # now leads to a target it did not lead to before, which can be in a directory that is + # already watched. # private def refresh_links desired = {} @@ -292,6 +299,13 @@ def stop @lock.synchronize do return false if @stopped.value + # A new target in a directory that is already watched is a change. A directory that is + # not watched yet counts once its watch is added below, so a watch that keeps failing + # does not report a change on every retry. + desired.each do |real_directory, names| + entry = @watched[real_directory] + added = true if entry && !(names - entry.targets).empty? + end @watched.each { |real_directory, entry| entry.targets = desired[real_directory] || Set.new } @watched.delete_if do |_real_directory, entry| next false unless entry.directories.empty? && entry.targets.empty? diff --git a/spec/impl/file_data/watcher_spec.rb b/spec/impl/file_data/watcher_spec.rb index 181fbe57..07e11885 100644 --- a/spec/impl/file_data/watcher_spec.rb +++ b/spec/impl/file_data/watcher_spec.rb @@ -214,6 +214,50 @@ def wait_for_next_file_time(link) end end + it "invokes the callback when a link resolves to a target in a directory that is already watched" do + Dir.mkdir(path("config")) + Dir.mkdir(path("other")) + File.write(path("other/other.json"), "{}") + # The link leads through data, which does not exist yet, to a directory that the watcher + # already watches because another configured file is in it. + make_symlink(path("data/flags.json"), path("config/flags.json")) + with_watcher([path("config/flags.json"), path("other/other.json")]) do |_watcher, calls| + sleep 0.3 + File.write(path("other/flags.json"), "{}") + before = settled(calls) + + make_symlink(path("other"), path("data")) + # The retry that resolves the link runs the callback once. + expect(wait_for { calls.value > before }).to be true + end + end + + it "does not invoke the callback on every retry while the directory of a link's target cannot be watched" do + Dir.mkdir(path("config")) + Dir.mkdir(path("data")) + File.write(path("data/flags.json"), "{}") + make_symlink(path("data/flags.json"), path("config/flags.json")) + data_directory = File.realpath(path("data")) + failing = Concurrent::AtomicBoolean.new(true) + allow_any_instance_of(Watcher).to receive(:add_watch).and_wrap_original do |original, real_directory| + raise "cannot watch #{real_directory}" if failing.true? && real_directory == data_directory + + original.call(real_directory) + end + with_watcher([path("config/flags.json")]) do |_watcher, calls| + # The retry runs once a second. While the watch keeps failing, nothing has changed. + sleep 2.5 + expect(calls.value).to eq 0 + + failing.make_false + # The retry that watches the target's directory runs the callback once. + expect(wait_for { calls.value >= 1 }).to be true + before = settled(calls) + File.write(path("data/flags.json"), '{"flagValues": {}}') + expect(wait_for { calls.value > before }).to be true + end + end + it "watches the target of a symbolic link once the target's directory appears" do Dir.mkdir(path("config")) make_symlink(path("data/flags.json"), path("config/flags.json"))