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

Add jaeger helm chart #79

Merged
merged 10 commits into from
Jun 24, 2019
Merged

Add jaeger helm chart #79

merged 10 commits into from
Jun 24, 2019

Conversation

mmatur
Copy link
Member

@mmatur mmatur commented Jun 21, 2019

Description

This Pr adds jaeger tracing in the project

@dtomcej
Copy link
Contributor

dtomcej commented Jun 21, 2019

Do we intend to have zipkin running locally?

or can we remove the zipkin tracing stuff if we are going to use jaeger?

Also, is there a particular reason we have the ingress created and the api exposed?

Is it just to be able to view the routes that are being created?

@dtomcej
Copy link
Contributor

dtomcej commented Jun 21, 2019

We should also look at having these things disabled by default when we install the helm chart in the integration tests :P or else our tests are going to take much longer to run 😄

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.

so close!

kind: CustomResourceDefinition
metadata:
name: tlsoptions.traefik.containo.us

Copy link
Contributor

Choose a reason for hiding this comment

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

This needs the CRD Hook annotations :)

@@ -160,8 +160,8 @@ func (w *ClientWrapper) patchCoreConfigMap(coreDeployment *appsv1.Deployment) (b
traefik.mesh.svc.cluster.local:53 {
errors
rewrite continue {
name regex ([a-z]*)\.([a-z]*)\.traefik\.mesh traefik-{1}-{2}.traefik-mesh.svc.cluster.local
answer name traefik-([a-z]*)-([a-z]*)\.traefik-mesh\.svc\.cluster\.local {1}.{2}.traefik.mesh
name regex ([a-z0-9-_]*)\.([a-z0-9-_]*)\.traefik\.mesh traefik-{1}-{2}.traefik-mesh.svc.cluster.local
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we need to add capitals to this? I only ask due to being unsure if case-sensitive DNS is an issue...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I think that we should add capitals.

@mmatur
Copy link
Member Author

mmatur commented Jun 24, 2019

Blocked by traefik/traefik#5007

@dtomcej
Copy link
Contributor

dtomcej commented Jun 24, 2019

traefik/traefik#5007 has been merged. Ready to proceed once the experimental image is built.

@SantoDE
Copy link
Contributor

SantoDE commented Jun 24, 2019

Will test tonight or tomorow and report back if wanted :)

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:

@dtomcej dtomcej merged commit 215e8bf into traefik:master Jun 24, 2019
@mmatur mmatur deleted the feature/add-tracing branch June 24, 2019 15:59
@mmatur mmatur mentioned this pull request Jun 24, 2019
@mmatur mmatur added this to the v0.1.0 milestone Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants