search

traefik / traefik-migration-tool

A migration tool from Traefik v1 to Traefik v2
https://docs.traefik.io/migration/v1-to-v2/
Apache License 2.0
151 stars 35 forks source link

Merge rules in IngressRoute definitions where possible #12

Open hakkibagci opened 4 years ago

hakkibagci commented 4 years ago

instead of creating a route for each path, merge rules that target the same service where possible, which reduces the number of routes created and results a much more readable IngressRoute definition. This is especially useful when there are multiple hosts with the same set of paths, I think which is a common scenario in real life.

ldez commented 4 years ago

Hello,

could you modify existing tests to keep the same behavior instead of applying the merge on it (i.e. modify the host)

ldez commented 4 years ago

after reflection I think there is a problem because you do not take into account the differences which are defined by the annotations.

I think it is better not to try to merge the ingress.

hakkibagci commented 4 years ago

after reflection I think there is a problem because you do not take into account the differences which are defined by the annotations.

I think it is better not to try to merge the ingress.

Hey, thanks for the quick review. I think it takes into account the changes which are defined by annotations as well. Can you please give an example where it should fail, so that I can fix it?

hakkibagci commented 4 years ago

Hello,

could you modify existing tests to keep the same behavior instead of applying the merge on it (i.e. modify the host)

I already modified the existing tests and also add some more examples which covers some edge cases. Can you please elaborate more on this? Thanks

ldez commented 4 years ago

I already modified the existing tests and also add some more examples which covers some edge cases

An example of a change: https://github.com/containous/traefik-migration-tool/pull/12/files?file-filters%5B%5D=.go#diff-19d3a047b1d139a00dfef33903cc8737R11

instead of changing rule, you have to change the host.

hakkibagci commented 4 years ago

I already modified the existing tests and also add some more examples which covers some edge cases

An example of a change: https://github.com/containous/traefik-migration-tool/pull/12/files?file-filters%5B%5D=.go#diff-19d3a047b1d139a00dfef33903cc8737R11

instead of changing rule, you have to change the host.

We are talking about only the test yml files here, right? If so, I still don't get it because in case of there are multiple paths for the same host (for instance in case of ingress.yml) the result will be only 1 rule instead of 2. So the existing test files should be adjusted as well. How can I achieve this by changing the host and not the rule?