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

Updating Kubernetes tests to properly test missing endpoints code path #1436

Merged
merged 1 commit into from
Apr 28, 2017

Conversation

regner
Copy link
Contributor

@regner regner commented Apr 13, 2017

This fixes #1307

@regner
Copy link
Contributor Author

regner commented Apr 13, 2017

As a heads up when I run the provider tests locally I get the following failed test:

--- FAIL: TestDockerLoadDockerServiceConfig (0.00s)
	docker_test.go:3124: expected map[string]*types.Backend{"backend-foo-service":(*types.Backend)(0xc42062cc90)}, got map[string]*types.Backend{"backend-foo":(*types.Backend)(0xc4206576b0)}

This PR doesn't touch anything relating to that test. Curious if it fails in Travis. Haven't looked into why but I am running from head.

Weight: 1,
},
},
Servers: map[string]types.Server{},
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason we are able to remove / empty a bunch of server definitions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh of course, it's because we now model missingness correctly, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@timoreimann timoreimann left a comment

Choose a reason for hiding this comment

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

In my memory, things seemed much more complicated when we tried to do this the first time back on the v1.2 branch. Must have memorized it incorrectly.

LGTM!

@regner
Copy link
Contributor Author

regner commented Apr 14, 2017

One of the annoying things was at first I just left the Servers: map[string]types.Server{}, line out, and the tests failed but looked 100% identical according to the output but were actually different.

@timoreimann
Copy link
Contributor

@regner 👍

In the referencing issue, you also mention the fact that we keep frontends dangling when no Endpoints can be found. Do you know what the effect of that is? And should we address it in this PR or a different one? (I'm open to both paths.)

@regner
Copy link
Contributor Author

regner commented Apr 18, 2017

I don't know what the effect of that is and am unsure how that should be handled. I think it needs some discussion first so should probably be a different issue and PR.

@timoreimann
Copy link
Contributor

@regner makes sense to me.

@timoreimann
Copy link
Contributor

@regner could you rebase and fix the conflicts? We recently moved all providers into separate sub-packages. Sorry for the inconveniences!

@ldez ldez added this to the 1.3 milestone Apr 25, 2017
@ldez ldez force-pushed the 1307-fix-k8s-tests-missing-endpoints branch from 1385fa3 to cf08bbe Compare April 28, 2017 13:23
@ldez ldez force-pushed the 1307-fix-k8s-tests-missing-endpoints branch from cf08bbe to d24ba90 Compare April 28, 2017 13:25
@ldez ldez self-assigned this Apr 28, 2017
@ldez ldez merged commit 2f1a7cb into traefik:master Apr 28, 2017
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.

Update Kubernetes tests to properly handle endpoints not existing
4 participants