fix: missing fields in destinationrule #1253
Conversation
huikang
force-pushed
the
1238-DestinationRule-unstrctured
branch
2 times, most recently
from
22d64b1 to
9368587
Compare
- Add Extra to DestinationRule to hold any other fields from istio's destinationRule - handle any extra fields that could be introduced to the subsets in istio's destinationRule fields - unit test Signed-off-by: Hui Kang <[email protected]>
huikang
force-pushed
the
1238-DestinationRule-unstrctured
branch
from
9368587 to
24e8350
Compare
|
@jessesuen , please take a look at this PR when you have time. The approach is tedious as we can not predict the key names of the additional fields in the subets. Do you have any better idea? Thanks. |
| apiVersion: networking.istio.io/v1alpha3 | ||
| kind: DestinationRule | ||
| metadata: | ||
| name: istio-destrule | ||
| namespace: default | ||
| spec: | ||
| subsets: | ||
| - name: stable | ||
| labels: | ||
| version: v3 | ||
| - name: canary | ||
| trafficPolicy: | ||
| loadBalancer: | ||
| simple: ROUND_ROBIN | ||
| `) |
There was a problem hiding this comment.
Can we improve the test to verify we also preserve fields at the .spec.XXX level? For example, this example from istio docs which has spec.host and spec.trafficPolicy:
apiVersion: networking.istio.io/v1beta1
kind: DestinationRule
metadata:
name: bookinfo-ratings
spec:
host: ratings.prod.svc.cluster.local
trafficPolicy:
loadBalancer:
simple: LEAST_CONN
subsets:
- name: testversion
labels:
version: v3
trafficPolicy:
loadBalancer:
simple: ROUND_ROBINThere was a problem hiding this comment.
Sure. Let me update the PR to generalize the method to the .spec.
There was a problem hiding this comment.
@jessesuen , I think the current implement has been able to preserve the fields of .spec.XXX. Following is a snippet of a destination rule after an update to a rollout.
spec:
host: istio-rollout <--------- Preserved
subsets:
- labels:
app: istio-rollout
rollouts-pod-template-hash: 6646844db8
name: canary
- labels:
app: istio-rollout
rollouts-pod-template-hash: cf89dfb88
name: stable
trafficPolicy: <--------- Preserved by the PR
loadBalancer:
simple: ROUND_ROBIN
trafficPolicy: <--------- Preserved
loadBalancer:
simple: LEAST_CONN
However, I could not find the relevant part in the code. Do you have any idea?
There was a problem hiding this comment.
So to verify we preserve the fields under .spec, I added a string comparison between the expected result and the unstructured object.
Signed-off-by: Hui Kang <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1253 +/- ##
==========================================
- Coverage 81.40% 81.36% -0.04%
==========================================
Files 106 106
Lines 9527 9599 +72
==========================================
+ Hits 7755 7810 +55
- Misses 1251 1260 +9
- Partials 521 529 +8
Continue to review full report at Codecov.
|
Signed-off-by: Hui Kang <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
|
Thank you! |
…oproj#1253) Signed-off-by: Hui Kang <[email protected]> Signed-off-by: caoyang001 <[email protected]>
…oproj#1253) Signed-off-by: Hui Kang <[email protected]> Signed-off-by: Andrii Perenesenko <[email protected]>
destinationRule
in istio's destinationRule fields
Signed-off-by: Hui Kang [email protected]
close #1238
Checklist:
"fix(controller): Updates such and such. Fixes #1234".