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 panic when pod IP is not yet assigned #507

Merged
merged 2 commits into from
Apr 17, 2020

Conversation

jspdown
Copy link
Contributor

@jspdown jspdown commented Apr 16, 2020

What does this PR do?

Pods with no IP are filtered out in the TopologyBuilder. When collecting the different pods for a TrafficTarget, we are referring to pods which can have been excluded. This is done without checking if the key is in the map, leading to a panic.
This PR intent to be more protective when accessing a map element.
In the same time, log message and errors have been cleaned up so they don't mention two times the same information and use the Key format everywhere instead of "namespace/name".

How to test it

This can be hard to reproduce as it occurs only when the Provider gets triggered and the Pod informers don't already have pods with an IP assigned. The best way to reproduce this bug is to create many pods at the same time.

  • Install Maesh
  • Create many pods
  • There shouldn't have any panic, even when pods don't have an IP assigned yet.
  • Force an error in the topology (mentioning a service which doesn't exist in a TrafficSplit for example) and look at the logs. Log messages should display the resource causing the error using the key format "name@namespace".

Pods with no IP are filtered out in the TopologyBuilder. When collecting
the different pods for a TrafficTarget, we are refering to a pod
which can have been excluded. This is done without checking if the
key is in the map, leading to a panic.
This commit intent to be more protective on map access.
In the same time, log message and errors have been cleaned so they
don't mention two times the same information and use the Key everywhere
instead of the "namespace/name" format.
Copy link
Member

@kevinpollet kevinpollet left a comment

Choose a reason for hiding this comment

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

LGTM 🛥️

@jspdown jspdown merged commit 3852d9e into traefik:master Apr 17, 2020
@kevinpollet kevinpollet added this to the v1.2 milestone Apr 20, 2020
@jspdown jspdown deleted the fix-panic-provider branch August 24, 2020 08:05
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.

None yet

3 participants