istio icon indicating copy to clipboard operation
istio copied to clipboard

`istioctl` does not follow Helm install or uninstall order for k8s resources

Open bleggett opened this issue 1 year ago • 4 comments

Is this the right place to submit this?

  • [X] This is not a security vulnerability or a crashing bug
  • [X] This is not a question about how to use Istio

Bug Description

Helm installs K8S resources in a specific, hardcoded order:

https://github.com/helm/helm/blob/main/pkg/releaseutil/kind_sorter.go#L31

And it uninstalls those resources in reverse order.

istioctl does not follow those ordering semantics, and probably should.

(in fact I don't think istioctl actually follows any specific ordering semantics for removal but I might be missing something).


Why does this matter?

  • We eventually want istioctl to be a light helm wrapper for install/uninstall.
  • Helm's ordering is actually pretty rational and safe in all cases, e.g - you must install ConfigMaps before Pods that reference them, and it makes sense for consistency that uninstall order is a mirror of the install order - occasionally you can hit weird issues caused by not having a consistent install/remove order for K8S, and Helm has already hit virtually all of those issues, which is why they have the ordering semantics they do.

We will probably get this "for free" if/when we replace istioctl's install/remove with a light Helm wrapper, but raising the issue here as a reminder that it will be a behavior change for istioctl (I think the differences are subtle enough to where it won't be a problem)

Version

NA

Additional Information

No response

bleggett avatar Jul 10 '24 18:07 bleggett

We have an order defined,so it's just the ordering is t ideal (?)

howardjohn avatar Jul 11 '24 12:07 howardjohn

We have an order defined,so it's just the ordering is t ideal (?)

I see what appears to be an order for install: https://github.com/istio/istio/blob/d86983c25c52002ca3db9ff8e444340dcd7956cf/operator/pkg/object/objects.go#L421

...it's not the same as Helm tho (it's a few specifics + "everything else"). This is probably fine, but will be different when we wrap Helm.

Prune/remove seem to be entirely unordered, rather than a fixed order, reversed from install: https://github.com/istio/istio/blob/master/operator/pkg/helmreconciler/prune.go#L208, unless I am missing something.

So also not the same as Helm - which is also probably fine, but will be different.

It isn't worth fixing until wrapping Helm fixes it for us, but I noticed it and wanted to make sure I didn't forget about it/make sure it's not a surprise to anyone.

bleggett avatar Jul 11 '24 16:07 bleggett

https://github.com/istio/istio/pull/52541#discussion_r1714470816 is A Reason (specific to test code) why following the Helm order doesn't work for us specifically - leader election lock timeouts.

bleggett avatar Aug 13 '24 01:08 bleggett

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2024-08-13. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

istio-policy-bot avatar Nov 26 '24 05:11 istio-policy-bot