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

Jump to bottom

Skipper support possible or in development? #452

Closed
webframp opened this issue Feb 21, 2020 · 4 comments · Fixed by #670
Closed

Skipper support possible or in development? #452

webframp opened this issue Feb 21, 2020 · 4 comments · Fixed by #670
Labels
help wanted Extra attention is needed kind/feature Feature request

Comments

@webframp
Copy link

I haven't looked much into how flagger provides support for the different ingress options, but I'm wondering where to start with adding support for skipper.

It supports some concepts that seems like it would be possible to integrate with although it's behavior is different than an envoy based ingress. Since nginx ingress is supported, the Filters and Predicates in skipper seem like concepts that flagger could use to implement progress delivery while bringing the automated progressive delivery and canary CRD of flagger to support another ingress.

Where do you suggest starting to attempt?
@stefanprodan
Copy link
Member

Flagger could patch the ingressRef object and add annotations: weight for canaries and header filters for A/B testing.

To add Skipper support you have to:

  • implement the router interface
  • add the skipper provider to CRD
  • implement the metrics observer interface
  • add unit tests for router and observer
  • add e2e tests for Skipper

@stefanprodan stefanprodan added help wanted Extra attention is needed kind/feature Feature request labels Feb 22, 2020
@dhohengassner dhohengassner mentioned this issue Aug 3, 2020
Merged
This was referenced Aug 4, 2020
Merged
Merged
@universam1
Copy link
Contributor

@stefanprodan @szuecs
WIP in

early review comments very welcome!

@stefanprodan
Copy link
Member

@universam1 this looks great 💯
When e2e are working please open a PR with the whole integration and I'll review it.

@szuecs
Copy link

szuecs commented Aug 4, 2020

@universam1 From skipper side looks great, but please add some comments in skipper source code about the _ and the route matching with prometheus queries. We don't want to break this, but it was never part of the interface so we should at least be aware that we would need to create an issue or PR here, in case the unlikely happening has to happen. There is no reason that this will ever happen, but better be prepared than sorry. ;)

I added some comments in (IIRC) all PRs.

@universam1 universam1 mentioned this issue Aug 14, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Extra attention is needed kind/feature Feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Skipper Ingress Controller support o11n/flagger
4 participants
@szuecs @webframp @stefanprodan @universam1