🌱 Force Cache Refresh when Server was not found by name.#84
Merged
Conversation
guettli
force-pushed
the
tg/refresh-stale-cache
branch
from
fdc3ca4 to
3742073
Compare
This was referenced Mar 16, 2026
guettli
force-pushed
the
tg/refresh-stale-cache
branch
from
2fa3608 to
9a905a4
Compare
guettli
force-pushed
the
tg/refresh-stale-cache
branch
from
9a905a4 to
a8f2d65
Compare
guettli
force-pushed
the
tg/refresh-stale-cache
branch
from
5679a23 to
02bd962
Compare
guettli
force-pushed
the
tg/refresh-stale-cache
branch
from
02bd962 to
1d9d7da
Compare
janiskemper
requested changes
Mar 16, 2026
// Remember this node name, so that it does not trigger a cache refresh again. c.NodeTriggeredForcedRefresh(string(node.Name))
janiskemper
requested changes
Mar 17, 2026
| } | ||
| } | ||
|
|
||
| func TestInstances_InstanceExistsRobotServerRepeatedMissingNameSkipsForceRefresh(t *testing.T) { |
Author
There was a problem hiding this comment.
I added this comment:
// If a node name is not in the cache, then only on the time the cache should be refreshed. A
// second time (during the time of CACHE_TIMEOUT), the unknown node name should not trigger a
// cache refresh again.There was a problem hiding this comment.
you just check twice the same thing. How does that fit to the description? I still don't understand it.
Author
There was a problem hiding this comment.
@janiskemper I updated the test and added coments: 4cfba46
| return c.ServerGetList() | ||
| } | ||
|
|
||
| c.m = nil |
There was a problem hiding this comment.
can you add a comment that you do this in order to delete the cache and to call the actual API?
|
|
||
| // nodeTriggeredForcedRefresh records that nodeName already triggered a forced | ||
| // refresh. | ||
| func (c *cacheRobotClient) nodeTriggeredForcedRefresh(nodeName string) { |
There was a problem hiding this comment.
can you inline this function? I find it confusing that it is not. AFAIK it's only used in one place.
janiskemper
approved these changes
Mar 19, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Refresh the Robot server cache once when a bare-metal node lookup by name misses in the cached list.
This fixes the case where:
InstanceExists()looks up the server by name,false.Fix: Deleting node because it does not exist in the cloud provider
Without this fix new Nodes get deleted if the baremetal node is not in the cache yet.
Changes
ServerGetListForceRefresh()to the Robot client interfacegetRobotServerByName()retry once with a forced refresh after a cache missbm-existingbm-newcreated afterwardsInstanceExists(bm-new)must returntrueUpstream HCloud CCM
Related PR in upstream ccm: fix robot name lookup after stale cache miss by guettli · Pull Request #1204 · hetznercloud/hcloud-cloud-controller-manager