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

Skipper Ingress Controller support #670

Merged
merged 5 commits into from
Aug 19, 2020
Merged

Conversation

universam1
Copy link
Contributor

@universam1 universam1 commented Aug 14, 2020

fix: #452

✨ zalan.do/Skipper Router Implementation

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://github.com/zalando/skipper/

  • The concept is to define routes with specific weights via the skipper specific annotation predicate of zalando.org/backend-weights.
  • apex Ingress is immutable
  • A new "canary ingress" is created with to paths for primary and canary service that has higher "weight" hence receiving all traffic, which distributes progressively
  • After the canary process is finished, this ingress is disabled via the False() annotation predicate to route traffic again back to the apex Ingress.

Skipper Principles:

There are certain Skipper principles which are taken into account:

  • if only one backend has a weight, only one backend will get 100% traffic
  • if two of three or more backends have a weight, only those two should get traffic.
  • if two backends don't have any weight, it's undefined and right now they get equal amount of traffic.
  • weights can be int or float, but always treated as a ratio.

📈 Prometheus Metrics Observer

  • "request-success-rate" and "request-duration" queries are implemented and tested that provide those oberservations from Skipper metrics
  • Takes into account how Skipper renders the paths accordingly and reformats the quieries.

Tests

  • adding CircleCI tests
  • 👷 Add high-level E2E test steps for Skipper
    • install Skipper ingress with Kustomize
    • load Flagger image onto the local cluster
    • install Flagger and Prometheus in the flagger-system namespace
  • local testing - could be also saved as file
#!/usr/bin/env bash
REPO_ROOT=$(git rev-parse --show-toplevel)
cd $REPO_ROOT

make test
make build
docker tag weaveworks/flagger:latest test/flagger:latest
make loadtester-build
(kind get clusters && kubectl delete ns/test --force) || kind create cluster --wait 5m --image kindest/node:v1.16.9
./test/e2e-skipper.sh
# port forward prometheus UI to localhost:9090
kubectl port-forward $(kubectl get pods -l=app=flagger-prometheus -o name -n flagger-system | head -n 1) 9090:9090 -n flagger-system &

./test/e2e-skipper-tests.sh

Proof

Successful progress

image

Failed checks

{"level":"info","ts":"2020-08-14T10:07:36.357Z","caller":"controller/events.go:28","msg":"Halt podinfo.test advancement request duration 915ms > 500ms","canary":"podinfo.test"}                                                                                                                                    
{"level":"debug","ts":"2020-08-14T10:07:36.358Z","logger":"event-broadcaster","caller":"record/event.go:278","msg":"Event(v1.ObjectReference{Kind:\"Canary\", Namespace:\"test\", Name:\"podinfo\", UID:\"c1162c0e-409a-4825-9dac-a0a6c3418e72\", APIVersion:\"flagger.app/v1beta1\", ResourceVersion:\"3704\", FieldPath:\"\"}): type: 'Warning' reason: 'Synced' Halt podinfo.test advancement request duration 915ms > 500ms"}                                             
{"level":"debug","ts":"2020-08-14T10:07:51.341Z","caller":"router/skipper.go:155","msg":"GetRoutes primaryWeight: 60, canaryWeight: 40","GetRoutes":"podinfo.test"}                                                                                                                                                 
{"level":"info","ts":"2020-08-14T10:07:51.348Z","caller":"controller/events.go:28","msg":"Rolling back podinfo.test failed checks threshold reached 5","canary":"podinfo.test"}

Prometheus graphs

http://localhost:9090/graph?g0.range_input=5m&g0.moment_input=2020-08-13%2004%3A25%3A27&g0.expr=sum(rate(skipper_response_duration_seconds_bucket%7Broute%3D~%22kube(ew)%3F_test__podinfo_ingress_canary__.*__podinfo_service_canary(_%5B0-9%5D%2B)%3F%22%2Ccode!~%225..%22%2Cle%3D%22%2BInf%22%7D%5B15s%5D))%20%2F%20%0Asum(rate(skipper_response_duration_seconds_bucket%7Broute%3D~%22kube(ew)%3F_test__podinfo_ingress_canary__.*__podinfo_service_canary(_%5B0-9%5D%2B)%3F%22%2Cle%3D%22%2BInf%22%7D%5B15s%5D))%20*%20100%0A&g0.tab=0&g1.range_input=5m&g1.expr=sum(rate(skipper_serve_route_duration_seconds_sum%7Broute%3D~%22kube(ew)%3F_test__podinfo_ingress_canary__.*__podinfo_service_canary(_%5B0-9%5D%2B)%3F%22%7D%5B15s%5D))%20%2F%20%0Asum(rate(skipper_serve_route_duration_seconds_count%7Broute%3D~%22kube(ew)%3F_test__podinfo_ingress_canary__.*__podinfo_service_canary(_%5B0-9%5D%2B)%3F%22%7D%5B15s%5D))%20*%201000&g1.tab=0

@universam1
Copy link
Contributor Author

FYI @webframp @szuecs

dhohengassner added a commit to o11n/flagger that referenced this pull request Aug 14, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
dhohengassner added a commit to o11n/flagger that referenced this pull request Aug 14, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
dhohengassner added a commit to o11n/flagger that referenced this pull request Aug 14, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
@universam1 universam1 force-pushed the feature-Skipper branch 6 times, most recently from e19fbd1 to 831d4a5 Compare August 14, 2020 15:24
dhohengassner added a commit to o11n/flagger that referenced this pull request Aug 14, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
dhohengassner added a commit to o11n/flagger that referenced this pull request Aug 14, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
Copy link

@szuecs szuecs left a comment

Choose a reason for hiding this comment

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

From my side it looks good, but of course I have not much idea about flagger.
I am not 100% sure if the Marshal/Unmarshal with string() will work in Go1.15. Please check that.
I Just added some comments, which are no blockers.

pkg/router/skipper.go Show resolved Hide resolved
pkg/router/skipper.go Show resolved Hide resolved
pkg/router/skipper.go Show resolved Hide resolved
test/e2e-skipper.sh Outdated Show resolved Hide resolved
@universam1
Copy link
Contributor Author

universam1 commented Aug 16, 2020

I am not 100% sure if the Marshal/Unmarshal with string() will work in Go1.15.

@szuecs thanks for reviewing!
Looked up the go 1.15 changes like the ascii-character-number-to-string conversion deprecation which does not apply here - other than that I could not find anything in this regard!? Cannot find any other, more cannonical way to convert a []byte to string though. Anything else you have in mind?

@szuecs
Copy link

szuecs commented Aug 16, 2020

@universam1 it’s fine if you checked. I just remembered “something changed in that area”, so please ignore my comment. 😊

Router implementation for zalan.do/Skipper Ingress -
An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress

https://github.com/zalando/skipper/

* The concept is to define routes with specific weights via the skipper specific annotation predicate of "zalando.org/backend-weights".
* A new "canary ingress" is created that has higher "weight" thus receiving all traffic, which distributes progressively
* After the canary process is finished, this ingress is disabled via the "False()" annotation predicate to route traffic again back to the apex Ingress.
There are certain Skipper principles which are taken into account:

```
Skipper Principles:
* if only one backend has a weight, only one backend will get 100% traffic
* if two of three or more backends have a weight, only those two should get traffic.
* if two backends don't have any weight, it's undefined and right now they get equal amount of traffic.
* weights can be int or float, but always treated as a ratio.

Implementation:
* apex Ingress is immutable
* new canary Ingress contains two paths for primary and canary service
* canary Ingress manages weights on primary & canary service, hence no traffic to apex service
```
Te be able to distinct Skipper routes we need to combine the Canary data to generate the Skipper metric label.

"request-success-rate" and  "request-duration" queries are implemented and tested that provide those obersvations from Skipper metrics

* Takes into account how Skipper renders the paths accordingly and reformats the quieries.
universam1 pushed a commit to o11n/flagger that referenced this pull request Aug 17, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
universam1 pushed a commit to o11n/flagger that referenced this pull request Aug 17, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
@universam1
Copy link
Contributor Author

@stefanprodan I think this PR is now ready for merge. Please note the added tutorial for Skipper

universam1 pushed a commit to o11n/flagger that referenced this pull request Aug 17, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
docs/gitbook/tutorials/skipper-progressive-delivery.md Outdated Show resolved Hide resolved
kustomize/skipper/kustomization.yaml Outdated Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
docs/gitbook/tutorials/skipper-progressive-delivery.md Outdated Show resolved Hide resolved
dhohengassner added a commit to o11n/flagger that referenced this pull request Aug 18, 2020
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
universam1 and others added 2 commits August 18, 2020 17:02
Add e2e-skipper* files for test setup

It does the following things:
* install Skipper ingress with Kustomize
* load Flagger image onto the local cluster
* install Flagger and Prometheus in the flagger-system namespace
Skipper Ingress Controller support is added with
fluxcd#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Awesome work Samuel and David 🏅

Looking forward to the A/B testing implementation if you want to tackle that.

@stefanprodan stefanprodan merged commit 11b82db into fluxcd:master Aug 19, 2020
adam-thorpe pushed a commit to adam-thorpe/flagger-helm that referenced this pull request Sep 3, 2024
Skipper Ingress Controller support is added with
fluxcd/flagger#670.

This commit add the documentation and links to mention
Skipper is now an available option.

Currently only Canary deployments are supported.
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.

Skipper support possible or in development?
4 participants