Skip to content
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

always attempt a live pod get on miss to confirm its really not there #1332

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

dougbtv
Copy link
Member

@dougbtv dougbtv commented Sep 11, 2024

From openshift pull request:

This makes sure that stale caches never result in NotFound errors.

It was explained to me that informers are almost always are more efficient, and in most cases will work, but a live lookup is appropriate after a number of failures.

This happens only on the retry portion, so we're still getting the benefits of informers, but, on a retry situation, we don't get a cache miss.

@coveralls
Copy link

coveralls commented Sep 11, 2024

Coverage Status

coverage: 56.369% (-0.01%) from 56.381%
when pulling fb03b0f on dougbtv:getlivepod
into 5338017 on k8snetworkplumbingwg:master.

pkg/k8sclient/k8sclient.go Outdated Show resolved Hide resolved
Copy link

This pull request is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 12, 2024
@dougbtv dougbtv force-pushed the getlivepod branch 2 times, most recently from 79dc92a to 1f4b061 Compare December 19, 2024 16:33
@bpickard22
Copy link
Collaborator

/lgtm

@bpickard22
Copy link
Collaborator

bump to remove stale

It was explained to me that informers are almost always are more efficient, and in most cases will work, but a live lookup is appropriate after a number of failures.

This happens only on the retry portion, so we're still getting the benefits of informers, but, on a retry situation, we don't get a cache miss.

Additionally, changes out use of cache get on this, since it already bails out before it on CNI DEL.
@bpickard22
Copy link
Collaborator

name change looks good

@bpickard22
Copy link
Collaborator

/lgtm

@dougbtv dougbtv merged commit fc72ddb into k8snetworkplumbingwg:master Dec 19, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants