-
Notifications
You must be signed in to change notification settings - Fork 727
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
Flagger omits TrafficSplit
backend service weight if weight is 0 due to omitempty
option
#930
Comments
Can you please post here |
|
Thanks @stefanprodan for working with me on this. We (the OSM team) are trying to run a successful end-to-end demo for integrating OSM with Flagger. After our successful demo, we plan to contribute to the Flagger repo (for the OSM custom metrics + kustomization files required + other changes necessary in the Flagger codebase) to have support for OSM <> Flagger. I think this is the only thing blocking our E2E demo (TrafficSplits not being created successfully). Appreciate the help @stefanprodan. ref: openservicemesh/osm#1700 |
Looks like you have |
Also please rename the metric templates and add |
Ok thanks @stefanprodan for pointing out that the SMI versions were mismatched. I'll try it again and let you know. |
Tried it with the correct version and yet it still isn't working - I'll paste the diagnostics. |
The TrafficSplit versions are both identical (v1alpha2).
|
@michelleN and I were able to figure out why the error message In the Flagger codebase, the flagger/pkg/apis/smi/v1alpha2/traffic_split.go Lines 34 to 38 in 4b084cf
According to Golang's JSON Marshall docs at https://golang.org/pkg/encoding/json/#Marshal, The Take note the TrafficSplit JSON struct is created here at this line. Note that the TrafficSplit backend service weight spec for canary is 0 and created here. flagger/pkg/router/smi_v1alpha2.go Lines 58 to 62 in 4b084cf
The error is being emitted at this line when the smi v1alpha2 router attempts to create the TrafficSplit custom resource. JSON marshalling of the struct results in the canary service weight being omitted (due to flagger/pkg/router/smi_v1alpha2.go Lines 89 to 92 in 4b084cf
As can be seen in the smi go sdk repo, both |
TrafficSplit
backend service weight if weight is 0 due to omitempty
option
Removed After deploying the canary deployment (
So now we've resolved the invalid TrafficSplit smi spec issue. However, after triggering a canary deployment by updating the container image, we get another issue:
|
Tried disabling pre-rollout webhooks and performing canary analysis. This time it went without any issues. The issues only pop up when pre-rollout webhook checks are enabled. See https://docs.flagger.app/usage/webhooks @michelleN, it seems like my osm/k8s cluster has traffic policy issues preventing the flagger loadtester from being successfully reached? I installed osm with
|
Please open a PR for the Like with AppMesh we could tell people how to allow traffic between the load tester and the app. |
@johnsonshi Flagger 1.12.0 has been released and contains your fix, please give it a try. |
Describe the bug
Since OSM is supported (SMI support added in #896), I did the following to create a canary deploy using OSM and Flagger.
As recommended in #896, I used the
MetricsTemplate
CRDs to create the required Prometheus custom metrics (request-success-rate
andrequest-duration
).I then created a canary custom resource for podinfo deployment, however it does not succeed. It says that the canary custom resource cannot create a TrafficSplit resource for the canary deployment.
Output excerpt of
kubectl describe -f ./podinfo-canary.yaml
:To Reproduce
./kustomize/osm/kustomization.yaml
:./kustomize/osm/patch.yaml
:Used
MetricTemplate
CRD to implement required custom metric (recommended in #896) -request-success-rate.yaml
:Used
MetricTemplate
CRD to implement required custom metric (recommended in #896) -request-duration.yaml
:podinfo-canary.yaml
:Output excerpt of
kubectl describe -f ./podinfo-canary.yaml
:Full output of
kubectl describe -f ./podinfo-canary.yaml
: https://pastebin.ubuntu.com/p/kB9qtPxZvr/Expected behavior
A clear and concise description of what you expected to happen.
Additional context
1.11.0
1.19.11
smi
(throughosm
)The text was updated successfully, but these errors were encountered: