Skip to content
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

Deprecate In-Cluster IstioOperator #166

Open
3 tasks
Tracked by #156
whitneygriffith opened this issue Nov 13, 2023 · 21 comments
Open
3 tasks
Tracked by #156

Deprecate In-Cluster IstioOperator #166

whitneygriffith opened this issue Nov 13, 2023 · 21 comments

Comments

@whitneygriffith
Copy link
Contributor

whitneygriffith commented Nov 13, 2023

Part of #156

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

  • Remove documentation about "Istio Operator" entirely. Hide commands in istioctl. Pretend it does not exist for all intents and purposes, other than for docs. We can move docs into operator/ or some wiki or point to archives. Add warnings to all istioctl commands using it.
  • Provide an alternative solution to In-Cluster IstioOperator
  • Announce deprecation
@whitneygriffith whitneygriffith converted this from a draft issue Nov 13, 2023
@whitneygriffith
Copy link
Contributor Author

#44814

@whitneygriffith
Copy link
Contributor Author

#47838

@ericvn
Copy link

ericvn commented Feb 27, 2024

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.

@whitneygriffith
Copy link
Contributor Author

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
Copy link
Member

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.

@whitneygriffith
Copy link
Contributor Author

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
Copy link
Member

I suppose just docs + test

@zirain
Copy link
Member

zirain commented Jun 28, 2024

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
Copy link
Contributor

craigbox commented Jul 1, 2024

@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
Copy link
Contributor

craigbox commented Jul 1, 2024

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
Copy link
Member

@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
Copy link
Contributor

craigbox commented Jul 1, 2024

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
Copy link
Member

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
Copy link
Member

linsun commented Jul 8, 2024

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

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

@ramaraochavali
Copy link

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
Copy link
Member

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

@bleggett
Copy link

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

👍

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

  • We don't need to reinvent what Helm does well-enough in istioctl (less code to test and maintain)
  • We can combine + reduce some of our install mechanisms (fewer bugs, less docs, less confusion)
  • Most of our other CNCF analogs have Helm charts, and people expect us to have them as well, which feeds back into the above points around less code, less docs, fewer pathways.

@bleggett
Copy link

bleggett commented Jul 22, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready
Development

No branches or pull requests

8 participants