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 skipper observer to flagger #2

Merged
merged 1 commit into from Aug 4, 2020
Merged

✨ add skipper observer to flagger #2

merged 1 commit into from Aug 4, 2020

Conversation

dhohengassner
Copy link

@dhohengassner dhohengassner commented Jul 30, 2020

✨ add skipper observer to flagger

To add Skipper support we need an oberserver to watch how the canary
rollout went.

This commit implements point 3 from
fluxcd#452 (comment)

Te be able to distinct Skipper routes we need to combine the Canary
data to generate the Skipper metric label.

@dhohengassner dhohengassner changed the title WIP: ✨ add skipper observer to flagger ✨ add skipper observer to flagger Aug 3, 2020
@dhohengassner
Copy link
Author

@universam1 please review

cc @icereed @webframp

pkg/metrics/observers/skipper.go Outdated Show resolved Hide resolved
@dhohengassner dhohengassner changed the base branch from master to Skipper August 4, 2020 08:39
@universam1 universam1 changed the base branch from Skipper to feature-Skipper August 4, 2020 09:36
To add Skipper support we need an oberserver to watch how the canary
rollout went.

This commit implements point 3 from
fluxcd#452 (comment)

Te be able to distinct Skipper routes we need to combine the Canary
data to generate the Skipper metric label.
@universam1
Copy link

nice job!

LGTM

@universam1 universam1 merged commit 1c97357 into o11n:feature-Skipper Aug 4, 2020

// encodeModelForSkipper replaces non word character in model with underscore to match route names
// https://github.com/zalando/skipper/blob/dd70bd65e7f99cfb5dd6b6f71885d9fe3b2707f6/dataclients/kubernetes/ingress.go#L101
func encodeModelForSkipper(model flaggerv1.MetricTemplateModel) flaggerv1.MetricTemplateModel {
Copy link

Choose a reason for hiding this comment

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

@aryszka here also the conversion to _ needs to be a > patch version change (unlikely that this changes anyways).

universam1 pushed a commit that referenced this pull request Aug 5, 2020
according to review #2 (comment) the East-West and the annotations need to be honored here as well
@universam1 universam1 mentioned this pull request Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants