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

Fix flake in LimitRange e2e #21788

Merged
merged 1 commit into from
Feb 25, 2016

Conversation

derekwaynecarr
Copy link
Member

Fixes #21234

The LimitRange TTL cache was being populated when the LimitRange itself was being created which meant the cache was getting set to empty by default.

The fix is that LimitRange should ignore things that are not pods prior to building its live look-up cache.

@derekwaynecarr derekwaynecarr added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Feb 23, 2016
@derekwaynecarr
Copy link
Member Author

/cc @ncdc

@k8s-github-robot
Copy link

Labelling this PR as size/XS

@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 23, 2016
@derekwaynecarr
Copy link
Member Author

To elaborate:

  • the admission controller normally works against an index.
  • the flake happens when that index is not yet populated
  • @deads2k added a live lookup cache to do a live lookup when the index was not populated, and hold onto that for 30s as an effort to reduce flake
  • but limit ranger admission code intercepts all create operations for any resource
  • so when the limit range itself is first created, admission controller intercepts that, and it did a live lookup, and said 'oh there are actually no limit ranges, let me remember that in my expiry cache`
  • then we go to create a pod, and the index in admission is still not updated, so it falls back to its live lookup cache and sees no limit ranges for the past action and so no resources are applied

The fix is to not populate a live lookup cache until we see our first pod so live lookup cache is not prematurely updated when the limit range itself is created.

@ncdc ncdc assigned ncdc and unassigned thockin Feb 23, 2016
@ncdc ncdc added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 23, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 23, 2016

GCE e2e test build/test passed for commit 4858b48.

@deads2k
Copy link
Contributor

deads2k commented Feb 24, 2016

@derekwaynecarr good catch. Is this already in the origin pick list?

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Feb 24, 2016

GCE e2e test build/test passed for commit 4858b48.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@derekwaynecarr
Copy link
Member Author

@deads2k - I added it to the issue list.

@k8s-bot
Copy link

k8s-bot commented Feb 24, 2016

GCE e2e build/test failed for commit 4858b48.

@ncdc
Copy link
Member

ncdc commented Feb 24, 2016

@k8s-bot e2e test this github flake: #21798

@k8s-bot
Copy link

k8s-bot commented Feb 24, 2016

GCE e2e test build/test passed for commit 4858b48.

a-robinson added a commit that referenced this pull request Feb 25, 2016
@a-robinson a-robinson merged commit 054d98f into kubernetes:master Feb 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

e2e flake: LimitRange [It] should create a LimitRange with defaults and ensure pod has those defaults applied.
9 participants