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

k8s integration tests #4569

Merged
merged 5 commits into from
Mar 11, 2019
Merged

k8s integration tests #4569

merged 5 commits into from
Mar 11, 2019

Conversation

juliens
Copy link
Member

@juliens juliens commented Mar 8, 2019 •
edited by ldez

What does this PR do?

Add integration tests on kubernetes.

Motivation

Have tests on kubernetes providers

More

  • Added/updated tests

Additional Notes

For this, we use https://github.com/rancher/k3s.

As k3s generate a kubeconfig.yaml, I add the ability to use the KUBECONFIG env var in Traefik.

@juliens juliens added this to the next milestone Mar 8, 2019
@juliens juliens requested a review from a team as a code owner March 8, 2019 14:55
@ldez ldez requested a review from timoreimann March 8, 2019 15:00
ldez
ldez approved these changes Mar 8, 2019
View reviewed changes
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

emilevauge
emilevauge requested changes Mar 8, 2019
View reviewed changes
Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

Nice job @juliens 👏
Few comments though

provider/kubernetes/kubernetes.go Outdated Show resolved Hide resolved
provider/kubernetes/kubernetes.go Show resolved Hide resolved
integration/k8s_test.go
os.Remove("./resources/compose/output/kubeconfig.yaml")
}

func parseK8sYaml(fileR []byte) []runtime.Object {
Copy link
Member

Choose a reason for hiding this comment

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

OMG this section is absolutely so sexy!
Real question: are we really supposed to do that manually?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to find the best solution and I found this kubernetes/client-go#193

dtomcej
dtomcej suggested changes Mar 8, 2019
View reviewed changes
Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

Awesome work so far! A few little questions!

integration/fixtures/k8s/test.yml Outdated Show resolved Hide resolved
integration/k8s_test.go Outdated Show resolved Hide resolved
integration/k8s_test.go Show resolved Hide resolved
integration/k8s_test.go Show resolved Hide resolved
integration/k8s_test.go Outdated Show resolved Hide resolved
provider/kubernetes/kubernetes.go Show resolved Hide resolved
@ldez ldez self-requested a review March 8, 2019 18:57
@dtomcej dtomcej self-requested a review March 9, 2019 16:52
dtomcej
dtomcej approved these changes Mar 9, 2019
View reviewed changes
Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM
:shipit:

ldez
ldez requested changes Mar 9, 2019
View reviewed changes
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

👀

provider/kubernetes/client.go Outdated Show resolved Hide resolved
provider/kubernetes/client.go Outdated Show resolved Hide resolved
provider/kubernetes/client.go Outdated Show resolved Hide resolved
juliens and others added 3 commits March 9, 2019 18:28
@juliens
fix review.
f39d941
@ldez @juliens
Apply suggestions from code review
072402f 
Co-Authored-By: juliens <julien.salleyron@gmail.com>
@juliens
Fix broken link
2f5ebea
ldez
ldez approved these changes Mar 9, 2019
View reviewed changes
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

timoreimann
timoreimann reviewed Mar 10, 2019
View reviewed changes
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.

Nice one! 👍

Besides k3s, there's also the possibility to use "kind". Mostly FYI though in case we run into troubles with k3s since it's still fairly new.

emilevauge
emilevauge approved these changes Mar 11, 2019
View reviewed changes
Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

LGTM

@traefiker traefiker merged commit 0cfaab0 into traefik:master Mar 11, 2019
@emilevauge
Copy link
Member

Besides k3s, there's also the possibility to use "kind". Mostly FYI though in case we run into troubles with k3s since it's still fairly new.

Indeed, but we were using kind in integration tests on another project and we ended up switching to k3s because:

  • kind is a lot heavier
  • kind does not provide a simple way to launch it using compose

@timoreimann
Copy link
Contributor

@emile that's very interesting to know, thanks. 👍

@juliens juliens deleted the k3s branch September 6, 2019 05:39
@snyk-bot snyk-bot mentioned this pull request Sep 16, 2021
Open
@MaxMood96 MaxMood96 mentioned this pull request Dec 7, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timoreimann timoreimann timoreimann left review comments

@ldez ldez ldez approved these changes

@emilevauge emilevauge emilevauge approved these changes

@dtomcej dtomcej dtomcej approved these changes

Assignees
No one assigned
Labels
area/provider/k8s/ingress kind/enhancement a new or improved feature. size/L
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@juliens @emilevauge @timoreimann @ldez @dtomcej @traefiker