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

feat: ✨ add support for traefik v3.0.0-beta3 and openTelemetry #872

Merged
merged 18 commits into from
Jul 26, 2023
Merged

feat: ✨ add support for traefik v3.0.0-beta3 and openTelemetry #872

merged 18 commits into from
Jul 26, 2023

Conversation

davhdavh
Copy link
Contributor

What does this PR do?

  • feat: ✨ better support for traefik v3 (beta2 and earlier no longer supported, use beta3+)
  • feat: add support for openTelemetry tracing for traefik v3
  • feat: add support for serverstransporttcps crd in traefik v3
  • fix: no longer listen on obsolete crds for traefik v3

@davhdavh
Copy link
Contributor Author

davhdavh commented Jun 27, 2023 •
edited
Loading

FYI, beta3 complains alot in logs, and thus this fix is pretty critical

	2023-06-27 15:42:28.547	
E0627 08:42:28.546928       1 reflector.go:140] k8s.io/client-go@v0.26.3/tools/cache/reflector.go:169: Failed to watch *v1alpha1.ServersTransportTCP: failed to list *v1alpha1.ServersTransportTCP: serverstransporttcps.traefik.io is forbidden: User "system:serviceaccount:default:traefik" cannot list resource "serverstransporttcps" in API group "traefik.io" at the cluster scope
	
	
2023-06-27 15:42:28.546	
W0627 08:42:28.546904       1 reflector.go:424] k8s.io/client-go@v0.26.3/tools/cache/reflector.go:169: failed to list *v1alpha1.ServersTransportTCP: serverstransporttcps.traefik.io is forbidden: User "system:serviceaccount:default:traefik" cannot list resource "serverstransporttcps" in API group "traefik.io" at the cluster scope
	
	
2023-06-27 15:41:44.721	
E0627 08:41:44.719794       1 reflector.go:140] k8s.io/client-go@v0.26.3/tools/cache/reflector.go:169: Failed to watch *v1alpha1.ServersTransportTCP: failed to list *v1alpha1.ServersTransportTCP: serverstransporttcps.traefik.io is forbidden: User "system:serviceaccount:default:traefik" cannot list resource "serverstransporttcps" in API group "traefik.io" at the cluster scope

mloiseleur
mloiseleur requested changes Jun 27, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
traefik/Changelog.md Outdated Show resolved Hide resolved
traefik/templates/_podtemplate.tpl Outdated Show resolved Hide resolved
@mloiseleur mloiseleur changed the title Fixes for traefik v3 feat: add support for traefik v3.0.0-beta3 Jun 27, 2023
@mloiseleur mloiseleur changed the title feat: add support for traefik v3.0.0-beta3 feat: ✨ add support for traefik v3.0.0-beta3 and openTelemetry Jun 27, 2023
@mloiseleur
Copy link
Contributor

@davhdavh Many thanks for your interest in this new (beta) version and this good PR.

@davhdavh davhdavh requested a review from mloiseleur June 28, 2023 04:38
mloiseleur
mloiseleur reviewed Jun 28, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
mloiseleur
mloiseleur reviewed Jun 28, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
mloiseleur
mloiseleur requested changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

Thanks. Tests looks good.
I left other comments about README and CRDs.

@davhdavh davhdavh mentioned this pull request Jun 28, 2023
Closed
2 tasks
@davhdavh davhdavh requested a review from mloiseleur June 29, 2023 09:51
@davhdavh
Copy link
Contributor Author

davhdavh commented Jul 4, 2023

@mloiseleur Are there other things you need before merging?

@davhdavh
Copy link
Contributor Author

@mloiseleur Did you have time to take a look again?

darkweaver87
darkweaver87 approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@darkweaver87 darkweaver87 left a comment

Choose a reason for hiding this comment

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

LGTM

@darkweaver87
Copy link
Contributor

darkweaver87 commented Jul 12, 2023 •
edited
Loading

@mloiseleur Did you have time to take a look again?

Thanks for contributing @davhdavh. This PR needs @mloiseleur's approval and he'll back on july the 21th :-)

@mloiseleur
Copy link
Contributor

Since Traefik v3 and Traefik v2 does not support the same set of CRDs, I'm not sure if we should go this way. I need to think of it a bit more.

@davhdavh
Copy link
Contributor Author

IMHO that problem belongs upstream and you can just do the best possible and add the CRDs so all versions are supported

mloiseleur
mloiseleur reviewed Jul 24, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
mloiseleur
mloiseleur reviewed Jul 24, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
@mloiseleur
Copy link
Contributor

@davhdavh You can test locally with make test

davhdavh and others added 17 commits July 26, 2023 10:05
@davhdavh @mloiseleur
Fixes for traefik v3
3978569
@davhdavh @mloiseleur
error in example command
f2dce36
@davhdavh @mloiseleur
Better help text for people to understand what updating to v3 does fo…
a2f4407 
…r crds
@davhdavh @mloiseleur
delete changelog per review
30345df
@davhdavh @mloiseleur
Missed update of the role for v3. Added test for all changes
f28ff40
@davhdavh @mloiseleur
Update README.md
14f5c60 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
revert the -f
4356327
@davhdavh @mloiseleur
Missing crd for v3
f91265c
@davhdavh @mloiseleur
Update README.md
47223bf 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
Update README.md
0776a15 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
Update traefik/templates/rbac/clusterrole.yaml
131401a 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
Update traefik/templates/rbac/role.yaml
dabce6f 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
Update traefik/templates/rbac/clusterrole.yaml
81edaa9 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
Update traefik/templates/rbac/role.yaml
b203267 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
Update traefik/tests/rbac-config_test.yaml
3e18e1f 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
Update traefik/tests/rbac-config_test.yaml
785d5ae 
Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com>
@davhdavh @mloiseleur
Remove experimental.v3.enabled and just use semver to autodetect base…
9006ef7 
…d on image.tag
mloiseleur
mloiseleur approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

Tests are fixed with 73bcd18

darkweaver87
darkweaver87 approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@darkweaver87 darkweaver87 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 80b5c06 into traefik:master Jul 26, 2023
1 check passed
@prometheanfire

This comment was marked as off-topic.

Sign in to view
@prometheanfire

This comment was marked as off-topic.

Sign in to view
@ldez ldez mentioned this pull request Aug 13, 2023
Closed
2 tasks
@mloiseleur mloiseleur mentioned this pull request Nov 27, 2023
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darkweaver87 darkweaver87 darkweaver87 approved these changes

@mloiseleur mloiseleur mloiseleur approved these changes

Assignees
No one assigned
Labels
kind/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@davhdavh @mloiseleur @darkweaver87 @prometheanfire @traefiker