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

xds-server: Create informers before starting informer factory #446

Merged
merged 1 commit into from
Nov 29, 2021

Conversation

cheahjs
Copy link
Contributor

@cheahjs cheahjs commented Nov 26, 2021

What type of PR is this?
/kind bug

What this PR does / Why we need it:

Fixes the Kubernetes controller implementation of the xds management server. Without the fix, the controller isn't able to list any GameServers or Pods in the cluster, and doesn't update connected Quilkin instances with endpoints.

The informer factory only starts informers that were already requested when Start() is called. The informers are requested when getting the Lister for a specific type. As we are calling Lister() after Start(), the informer never watches for the resources, and List() will always return the empty list.

Which issue(s) this PR fixes:

Special notes for your reviewer:

@quilkin-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: 763ab32b-929f-4455-bd85-11d7bbfcaecc

To build this version:

git fetch [email protected]:googleforgames/quilkin.git pull/446/head:pr_446 && git checkout pr_446
cargo build

Copy link
Collaborator

@XAMPPRocky XAMPPRocky left a comment

Choose a reason for hiding this comment

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

Thank you for your PR! LGTM, but I'll wait for @markmandel || @iffyio to have a look before merging.

Copy link
Collaborator

@iffyio iffyio left a comment

Choose a reason for hiding this comment

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

LGTM! The order of things got shifted wrong in the pr it seems, Thanks for finding and fixing it 🎉

@iffyio iffyio merged commit 29b7350 into googleforgames:main Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants