search

parodos-dev / orchestrator-helm-chart

Helm chart to deploy the Orchestrator solution suite.
https://parodos.dev/orchestrator-helm-chart/
Apache License 2.0
2 stars 22 forks source link

Update README.md for cleanup #183

Closed gabriel-farache closed 5 months ago

gabriel-farache commented 5 months ago

Closes https://issues.redhat.com/browse/FLPATH-1320

If M2K workflow is installed oc get crd -o name | grep -e sonataflow -e rhdh -e knative | xargs oc delete will delete services.serving.knative.dev and ingresses.networking.internal.knative.dev. But then m2k-save-transformation-func ingresses.networking.internal.knative.dev is still there and blocks the deletion of the CRD:

$ oc get ingresses.networking.internal.knative.dev -A
NAMESPACE          NAME                           READY   REASON
sonataflow-infra   m2k-save-transformation-func   True

If a workflow is installing Knative resources, when cleanuping the orchestrator helm chart, those Knative resource will likely generate an issue, like for M2K

This PR explicitly state to remove all additional resources before deleting the CRD and the namespace.

rgolangh commented 5 months ago

It is probably best to first documentation the limitation we have and say the chart for the workflows must be removed before the orchestrator.

Also, I think the cleanup is too aggresive, and will remove resources which are not our own I'm referring to the expression here which will remove every knative crd https://github.com/parodos-dev/orchestrator-helm-chart/blob/b43895758b74b377f5c89123f617deaeae3c65d4/README.md?plain=1#L256

It is the rare case to require from users to delete all the deps (like knative and so on ) . What if the system depends heavily on knative now, we shouldn't just remove it.

gabriel-farache commented 5 months ago

@rgolangh Agreed, I made the changes you suggested by removing the deletion of the knative CRD but still providing the command if the user wants to remove them nonetheless

And I added a warning stating to remove the installed workflows first

rgolangh commented 5 months ago

/lgtm @batzionb @y-first any additions?

batzionb commented 5 months ago

why do we need to delete the CRDs? Maybe deleting all of the instances of SonataFlow is a cleaner approach?

gabriel-farache commented 5 months ago

why do we need to delete the CRDs?

@batzionb to completely remove the resources the orchestrator installed. If the knative operator is not installed, the orchestrator will install it

Maybe deleting all of the instances of SonataFlow is a cleaner approach?

Still, the CRD installed by the orchestrator would still be there even if there is nothing anymore that will use them

y-first commented 5 months ago

lgtm

rgolangh commented 5 months ago

I'm merging this. We definitely do need to refine the cleanup process more.