MAISTRA-1575 create new SMCP structure for Istio 1.6

[rcernich]

New SMCP structure that better organizes (I hope) configuration settings.

[rcernich]

The first cut at a new structure for SMCP is complete. Still to do:

• Verify installation still works with 1.0/1.1
• Integrate istio 1.6 charts
• Write conversion code from v1 to v2
• Add conversion webhook

[lgtm-com]

This pull request introduces 2 alerts and fixes 2 when merging c4a3dbc into 32740c9 - view on LGTM.com

new alerts:

• 2 for Useless assignment to local variable

fixed alerts:

• 2 for Size computation for allocation may overflow

[lgtm-com]

This pull request fixes 2 alerts when merging d87387b into 32740c9 - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[rcernich]

@dgn, here's a link to some example yaml for the new structure: https://github.com/maistra/istio-operator/blob/6121c576086fbe56edb04f957f8fff24193d657c/pkg/apis/maistra/v2/smcp_new.yaml

I've also sprinkled the code with XXX comments where I'm unsure about things or need to come back and fix/decide what to do (e.g. maybe some settings don't map to values.yaml, but should be supported).

[lgtm-com]

This pull request fixes 2 alerts when merging 6121c57 into 32740c9 - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[rcernich]

I'm currently using enabled = !null in a lot of places, and I'm not sure whether or not I should include an explicit "enabled" field.

[lgtm-com]

This pull request fixes 2 alerts when merging 87e755d into 32740c9 - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[rcernich]

SMCP reconciler has been converted to use v2 resource and all unit tests are passing. I think there are some differences in the way things are defaulted, e.g. gateways.ingress and gateways.egress must be specified, as should policy, telemetry, and the various addons, which I think were all enabled by default. (See my comment above about having a separate enabled field, as opposed to using enabled = field != nil.)

[lgtm-com]

This pull request fixes 2 alerts when merging 8221b4a into 32740c9 - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[lgtm-com]

This pull request fixes 2 alerts when merging 63bb20a into 32740c9 - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[lgtm-com]

This pull request fixes 2 alerts when merging 8a091b0 into 32740c9 - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[lgtm-com]

This pull request fixes 2 alerts when merging e5b6857 into 32740c9 - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[lgtm-com]

This pull request fixes 2 alerts when merging 112af55 into 32740c9 - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[lgtm-com]

This pull request fixes 2 alerts when merging b60115d into ee739ef - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[lgtm-com]

This pull request fixes 2 alerts when merging c981148 into ee739ef - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[rcernich]

This is working for v1.1 installs.

[lgtm-com]

This pull request fixes 2 alerts when merging 0363dca into ee739ef - view on LGTM.com

fixed alerts:

• 2 for Size computation for allocation may overflow

[brian-avery]

Are you planning on removing hte TODO before merge?

[rcernich]

I anticipate a couple of rounds of cleanup, so not at this point. I need to get this in so we can start getting 2.0 installations going. This is a pretty big change and needs a lot of eyeballs and a proper design review. I don't anticipate this being the final revision, but a starting point to get it dialed in.

[dgn]

Thanks for starting this, Rob. I added a few comments but they don't have to be addressed in this PR. Generally, I think we should finalize the spec asap and then start working on documentation - because that'll become really important with this change

[dgn]

what's this used for? upstream 1.5+ does not have this anymore, and we never shipped it

[rcernich]

I was just trying to cover the full range of items and wasn't sure if this was something special for gateways. It can be removed.

[dgn]

how do you trigger installation? just by settings any values under install?

[rcernich]

Yes, if install is not null, maistra will create a CR with the settings. If a CR already exists, no action will be taken.

[dgn]

I don't think we should nest the options underneath install, because every addon setting we provide will be an installation setting. maybe install should just be the bool that determines whether the addon is managed?

[rcernich]

I'm trying to preserve the existing behavior, where the maistra operator installs and manages these things. The difference is, once installed, maistra will only manage certain fields in the CR. For example, in the case of Kiali, the settings for grafana, prometheus, and jaeger, and possibly the version (to ensure it is compatible with the control plane version).