fix cluster scoped istio CR #391
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #391 +/- ##
==========================================
- Coverage 65.87% 65.14% -0.73%
==========================================
Files 35 35
Lines 3795 3796 +1
==========================================
- Hits 2500 2473 -27
- Misses 1112 1128 +16
- Partials 183 195 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
|
controllers/kuadrant_controller.go
Outdated
| @@ -380,6 +381,10 @@ func controlPlaneProviderName() string { | |||
| return env.GetString("ISTIOOPERATOR_NAME", "istiocontrolplane") | |||
| } | |||
|
|
|||
| func istioCRName() string { | |||
| return env.GetString("ISTIO_NAME", "default") | |||
There was a problem hiding this comment.
So we need two environment variables, one for specifying the name of the IstioOperator CR and another one for specifying the name of the Istio CR in case the IstioOperator API is not installed?
There was a problem hiding this comment.
I'm investigating this as well as part of #392. It appears that currently the gateway is only reconciled correctly if the Istio CR is named default, in which case the associated configmap ends up being named istio. If the Istio CR is named anything else the configmap ends up being called istio-<istio-cr-name> and (currently in this case) the gateway is not being reconciled correctly.
There was a problem hiding this comment.
I've reached out to the sail team and it appears that the istio CR must be named default to process GW API resources (at least for now). This is due to the way upstream istio handles revisions (they are not fully supported yet) and it may change in the future.
Perhaps to avoid confusion for now we could remove the configuration through an environment variable and instead have it always search for default - I'll push a commit for this - I've also checked the docs and there's no mention of the previous name as they rely on the quickstart script or the Makefile which both use this yaml.
Co-authored-by: Adam Cattermole <[email protected]>
Co-authored-by: Adam Cattermole <[email protected]>
🤷