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.

Sign up for GitHub

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

Jump to bottom

Use watcher to track unmanaged kube-dns pods in Cilium Operator #11470

Merged
merged 1 commit into from
May 28, 2020
Merged

Use watcher to track unmanaged kube-dns pods in Cilium Operator #11470

merged 1 commit into from
May 28, 2020

Conversation

aanm
Copy link
Member

@aanm aanm commented May 11, 2020 •
edited
Loading

Listing all pods, even if with a filter will load the etcd kvstore as
the filtering is done in kube-apiserver. To avoid doing this operation
every 10 seconds we can have a single watcher that keeps track of all
unmanaged kube-dns pods locally.

Fixes #11466

@aanm aanm added dont-merge/waiting-for-upstream Only merge once upstream library or kernel features have landed release-note/misc This PR makes changes that have no direct user impact. labels May 11, 2020
@aanm
Copy link
Member Author

aanm commented May 11, 2020

test-me-please

@aanm aanm force-pushed the pr/avoid-pod-list branch 3 times, most recently from ad9813c to 842782d Compare May 12, 2020 09:10
@aanm aanm force-pushed the pr/do-not-list-unmanaged-pods branch from 8d7b3c6 to d13f3d3 Compare May 12, 2020 09:11
@aanm
Copy link
Member Author

aanm commented May 12, 2020

test-me-please

@aanm aanm force-pushed the pr/avoid-pod-list branch 5 times, most recently from 199c64c to 29f49b0 Compare May 13, 2020 11:20
@aanm aanm changed the base branch from pr/avoid-pod-list to master May 13, 2020 14:34
@aanm aanm changed the title Use watcher to track unamanged kube-dns pods in Cilium Operator Use watcher to track unmanaged kube-dns pods in Cilium Operator May 13, 2020
@aanm aanm force-pushed the pr/do-not-list-unmanaged-pods branch from d13f3d3 to f960369 Compare May 13, 2020 14:36
@aanm
Copy link
Member Author

aanm commented May 13, 2020

test-me-please

@aanm aanm removed the dont-merge/waiting-for-upstream Only merge once upstream library or kernel features have landed label May 13, 2020
@aanm aanm force-pushed the pr/do-not-list-unmanaged-pods branch 2 times, most recently from b4f2ac2 to b33db47 Compare May 13, 2020 14:44
@aanm aanm marked this pull request as ready for review May 13, 2020 14:44
@aanm aanm requested a review from a team May 13, 2020 14:44
@aanm aanm requested a review from a team as a code owner May 13, 2020 14:44
@aanm
Copy link
Member Author

aanm commented May 13, 2020

will need rebase once #11435 is merged

@coveralls
Copy link

coveralls commented May 13, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.03%) to 36.965% when pulling 66aaee9 on pr/do-not-list-unmanaged-pods into fac1802 on master.

@aanm aanm force-pushed the pr/do-not-list-unmanaged-pods branch from b33db47 to 80aa851 Compare May 14, 2020 07:22
@aanm
Copy link
Member Author

aanm commented May 14, 2020

test-me-please

@aanm aanm force-pushed the pr/do-not-list-unmanaged-pods branch from 80aa851 to 7328d76 Compare May 15, 2020 16:45
@aanm aanm requested a review from a team May 15, 2020 16:45
@aanm
Copy link
Member Author

aanm commented May 15, 2020

test-me-please

1 similar comment
@aanm
Copy link
Member Author

aanm commented May 15, 2020

test-me-please

@aanm aanm force-pushed the pr/do-not-list-unmanaged-pods branch from 7328d76 to 66aaee9 Compare May 15, 2020 16:46
@aanm
Copy link
Member Author

aanm commented May 15, 2020

test-me-please

qmonnet
qmonnet approved these changes May 27, 2020
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@aanm
operator: use watcher to track unmanaged pods
375ccf9 
Listing all pods, even if with a filter will load the etcd kvstore as
the filtering is done in kube-apiserver. To avoid doing this operation
every 10 seconds we can have a single watcher that keeps track of all
unmanaged kube-dns pods locally.

Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm force-pushed the pr/do-not-list-unmanaged-pods branch from 66aaee9 to 375ccf9 Compare May 27, 2020 13:48
@aanm
Copy link
Member Author

aanm commented May 27, 2020 •
edited
Loading

test-me-please (previous test logs were no longer available)

@aanm
Copy link
Member Author

aanm commented May 28, 2020

retest-net-next

@aanm
Copy link
Member Author

aanm commented May 28, 2020

K8s1.17 hit #11314

@aanm aanm merged commit 26e7451 into master May 28, 2020
@aanm aanm deleted the pr/do-not-list-unmanaged-pods branch May 28, 2020 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christarazi christarazi christarazi approved these changes

@qmonnet qmonnet qmonnet approved these changes

Assignees
No one assigned
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not list kube-dns pods in cilium-operator
4 participants
@aanm @coveralls @christarazi @qmonnet