istio / enhancements

Enhancement Tracking and Backlog Repo for Istio Releases
13 stars 41 forks source link

Deprecate In-Cluster IstioOperator #166

Open whitneygriffith opened 10 months ago

whitneygriffith commented 10 months ago

Part of #156

TOC agrees that the In-Cluster IstioOperator is a good candidate for deprecation.

whitneygriffith commented 10 months ago

#44814

whitneygriffith commented 10 months ago

#47838

ericvn commented 6 months ago

Do we have any consensus on this being announced at KubeCon and/or someone planning to do the work? It seems that we are only talking deprecation of the "Istio In-Cluster Operator" and not necessarily its removal in https://github.com/istio/istio/issues/44814. My main concern is what the timeline will look like for when the depreciation will be completed and the in-cluster operator is removed from the code.

whitneygriffith commented 6 months ago

Do we have any consensus on this being announced at KubeCon and/or someone planning to do the work? It seems that we are only talking deprecation of the "Istio In-Cluster Operator" and not necessarily its removal in istio/istio#44814. My main concern is what the timeline will look like for when the depreciation will be completed and the in-cluster operator is removed from the code.

We currently do not have an owner for the work and therefore will revist our deprecation announcement and timeline in the Enhancements Subgroup meeting today.

howardjohn commented 2 months ago

IMO we should do the following for Istio 1.23:

istioctl operator remove --purge # Remove the install, so it will not reconcile anything
kubectl get istiooperator -oyaml > iop.yaml
istioctl install -f iop.yaml

We will mention Helm and Sail operator as alternative paths with no smooth migration path.

In Istio 1.24:

whitneygriffith commented 2 months ago
istioctl operator remove --purge # Remove the install, so it will not reconcile anything
kubectl get istiooperator -oyaml > iop.yaml
istioctl install -f iop.yaml

@howardjohn What needs to be done for the istioctl migration story beyond testing the migration path?

howardjohn commented 2 months ago

I suppose just docs + test

zirain commented 2 months ago

I'm sure some users will still want to use it, maybe we can move it out of istio(under Istio-ecosystem/istio-operator or istio-ecosystem/istio-legacy-operator) and people who really want to use it can send pr there?

craigbox commented 2 months ago

@zirain The Red Hat team are working on sail-operator, a continuation of their Maistra Operator, which I expect learns from the Istio operaotr. I understand we intend to suggest it the path forward for people who want to use an operator, while pointing out that it is "not supported by the project".

(We could move https://github.com/istio/istio/tree/master/operator to istio-ecosystem and archive it, or it could just live on in the git history. See how https://github.com/istio/operator remains archived.)

craigbox commented 2 months ago

I understand this is a lot to ask, but is it possible to consider a migration path for the resource called IstioOperator? Its existence (and continued usage) suggests the existence of an operator and will doubtlessly cause confusion.

I don't know if conversion webhooks would solve this, and I doubt we want to go full Ahmet. If there is a path to renaming this which doesn't involve proposing support for renaming CRDs into Kubernetes and waiting two years, I'd love to see it.

howardjohn commented 2 months ago

@craigbox you mean as an input to istioctl?

We could make istioctl easily accept alternative names since it's just a CLI. I had even considered just accepted one without kind at all

craigbox commented 2 months ago

I mean, this sentence:

The istioctl command supports the full IstioOperator API via command-line options for individual settings or for passing a yaml file containing an IstioOperator custom resource (CR).

For use with istioctl, it might be easier if the API was called IstioConfiguration or IstioValues etc.

howardjohn commented 2 months ago

Yeah I agree. It's easy to change from a technical standpoint. Only problem is explaining to users that it sort of changed but doesn't matter, bike shredding the name, etc

linsun commented 2 months ago

IMO we should do the following for Istio 1.23:

* Formally announce deprecation, with removal for Istio 1.24.

* Provide a documentation for migration. This will be, roughly:
istioctl operator remove --purge # Remove the install, so it will not reconcile anything
kubectl get istiooperator -oyaml > iop.yaml
istioctl install -f iop.yaml

We will mention Helm and Sail operator as alternative paths with no smooth migration path.

In Istio 1.24:

* Remove operator.

This timeline makes sense to me. cc @rcernich @jwendell from redhat team to see if sail operator could serve as an alternative path at the 1.23/24 time frame.

craigbox commented 2 months ago

I see that our feature stages promise is only 3 months for a Beta feature, but for the fact that people upgrade slowly, and this is one of the first ways people interact with Istio, I might recommend at least 6 months before removing.

(I would like to relate this to @therealmitchconnors's research on upgrading and hear his POV on the timeframe)

howardjohn commented 2 months ago

The Operator has been strongly discouraged for over 3 years + stated it would eventually be removed, so IMO its an exceptionally long deprecation window; we just never listed the final removal date.

The reason I want to remove it quickly is its removal will remove tech debt, enabling us to finish our plans to make istioctl and Helm co-exist very well. This, in turn, will allow a migration path from IstioOperator -> Helm directly.

So if we remove it sooner, we actually give a better migration path, since users will be migrating in 1.24 anyways due to impending removal. If we remove in 1.24, they can likely migrate to Helm; if not, they will have to migrate to istioctl, then later to Helm in 1.25 (if they want to)

craigbox commented 2 months ago

OK, I'm sold. Thanks for the explanation.

ramaraochavali commented 2 months ago

if not, they will have to migrate to istioctl, then later to Helm in 1.25 (if they want to)

What are the advantages of helm over istioctl? Any thing significantly stands out or is it a choice based on how they prefer to install?

howardjohn commented 2 months ago

The main benefit is standardization so it integrates with all the other tooling out there like Argo, etc

bleggett commented 2 months ago

The main benefit is standardization so it integrates with all the other tooling out there like Argo, etc

:+1:

The (even more important for Istio IMO) secondary benefits are

bleggett commented 2 months ago

I mean, this sentence:

The istioctl command supports the full IstioOperator API via command-line options for individual settings or for passing a yaml file containing an IstioOperator custom resource (CR).

For use with istioctl, it might be easier if the API was called IstioConfiguration or IstioValues etc.

If we change the name and keep that resource, we might as well change the structure too.

Have we considered hewing (ideally very) closely to regular a Helm Chart.yaml + values.yaml ?

e.g.

IstioManifest.yml ->

apiVersion: istio-manifest-v1
dependencies:
  - name: base
    version: 1.0.0
    repository: "https://istio-release.storage.googleapis.com/charts/base"
  - name: cni
    version: 1.0.0
    repository: "https://istio-release.storage.googleapis.com/charts/istio-cni"
  - name: istiod
    version: 1.0.0
    repository: "https://istio-release.storage.googleapis.com/charts/istio-discovery"
    condition: istiod.enabled
  - name: ztunnel
    version: 1.0.0
    repository: "https://istio-release.storage.googleapis.com/charts/ztunnel"
values:
  <dump of bog-standard values.yaml overrides>

That's unsurprising, and all that we need. The only difference between a helm wrapper chart yaml and what we want IstioNewResource to do is install the named components in a custom order and manage their upgrade/removal in a custom order - but the declarative YAML that lists the components and has value overrides for them might as well be as close to the existing tool as it can be (i.e. as far as I can tell it can be nearly identical).

If we need a custom Istio manifest at all, I don't think we need it to be more complex than the above.