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

HelmRelease with non-existing service account gets stuck being deleted #554

Closed
gfichtenholt opened this issue Oct 28, 2022 · 2 comments · Fixed by #738
Closed

HelmRelease with non-existing service account gets stuck being deleted #554

gfichtenholt opened this issue Oct 28, 2022 · 2 comments · Fixed by #738

Comments

@gfichtenholt
Copy link

gfichtenholt commented Oct 28, 2022

$ flux version
flux: v0.35.0
helm-controller: v0.26.0
image-automation-controller: v0.26.1
image-reflector-controller: v0.22.1
kustomize-controller: v0.30.0
notification-controller: v0.28.0
source-controller: v0.31.0

$ kubectl create ns test-123
namespace/test-123 created

$ flux create source helm podinfo \
>       --url https://stefanprodan.github.io/podinfo \
>       --namespace test-123
✚ generating HelmRepository source
► applying HelmRepository source
✔ source created
◎ waiting for HelmRepository source reconciliation
✔ HelmRepository source reconciliation completed
✔ fetched revision: d48c458634fa13868f45eb656c808ed2a61ec3a96cef394d90f98e69b2d3cd63

$ kubectl create -f - <<EOF
> apiVersion: helm.toolkit.fluxcd.io/v2beta1
> kind: HelmRelease
> metadata:
>   name: podinfo
>   namespace: test-123
> spec:
>   chart:
>     spec:
>       chart: podinfo
>       reconcileStrategy: ChartVersion
>       sourceRef:
>         kind: HelmRepository
>         name: podinfo
>         namespace: test-123
>       version: 6.2.2
>   interval: 1m0s
>   serviceAccountName: non-existing-name
> EOF
helmrelease.helm.toolkit.fluxcd.io/podinfo created

$ kubectl get helmrelease/podinfo -n test-123 -o yaml
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  creationTimestamp: "2022-10-28T03:24:43Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 1
  name: podinfo
  namespace: test-123
  resourceVersion: "153074"
  uid: 5de641d9-343f-4cd2-a6c7-bb7c519f1cb8
spec:
  chart:
    spec:
      chart: podinfo
      reconcileStrategy: ChartVersion
      sourceRef:
        kind: HelmRepository
        name: podinfo
        namespace: test-123
      version: 6.2.2
  interval: 1m0s
  serviceAccountName: non-existing-name
status:
  conditions:
  - lastTransitionTime: "2022-10-28T03:24:43Z"
    message: failed to get last release revision
    reason: GetLastReleaseFailed
    status: "False"
    type: Ready
  failures: 7
  helmChart: test-123/test-123-podinfo
  observedGeneration: 1

So far so good. This is all as expected
Now the command
$ kubectl delete hr/podinfo -n test-123 will output
helmrelease.helm.toolkit.fluxcd.io "podinfo" deleted
and hang forever. Meanwhile in another terminal you can do

$ kubectl get helmrelease/podinfo -n test-123 -o yaml
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  creationTimestamp: "2022-10-28T03:24:43Z"
  deletionGracePeriodSeconds: 0
  deletionTimestamp: "2022-10-28T03:26:45Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 2
  name: podinfo
  namespace: test-123
  resourceVersion: "153328"
  uid: 5de641d9-343f-4cd2-a6c7-bb7c519f1cb8
spec:
  chart:
    spec:
      chart: podinfo
      reconcileStrategy: ChartVersion
      sourceRef:
        kind: HelmRepository
        name: podinfo
        namespace: test-123
      version: 6.2.2
  interval: 1m0s
  serviceAccountName: non-existing-name
status:
  conditions:
  - lastTransitionTime: "2022-10-28T03:24:43Z"
    message: failed to get last release revision
    reason: GetLastReleaseFailed
    status: "False"
    type: Ready
  failures: 9
  helmChart: test-123/test-123-podinfo
  observedGeneration: 1


i.e. the object still exists (with observedGeneration:1 and metadata.generation:2 ?)
and will result in errors if another object with the same name were to be created

I found one way to workaround is ctrl+C and then
kubectl edit hr/podinfo -n test-123 to remove

 finalizers:
- finalizers.fluxcd.io

then the object is really purged. The fact that the finalizer stops the object from being deleted is an issue that should be addressed, in my opinion.
Thank you

gfichtenholt added a commit to vmware-tanzu/kubeapps that referenced this issue Nov 5, 2022
…X, stays around in k8s flux HelmRelease CR #5577  (#5584)

fixed an inconsistency between GetInstalledPackageSummaries() and
GetInstalledPackageDetail() in one corner case.
Main fix is dependent on flux
fluxcd/helm-controller#554

There is only one small change to production code. The rest is
test-related code. Also,

+ added a few integration tests.
+ bump flux version in tests
+ fix for available package handling with flux in multi-tenant mode
#5541
@kingdonb
Copy link
Member

Looks like this is related to: fluxcd/flux2#997

The missing service account means that HelmRelease can't actually do anything, because its permissions in RBAC only come from the service account. So the clean-up tasks that are normally associated with deleting a HelmRelease can't be performed, or checked for performance.

Deleting a HelmRelease normally would perform helm uninstall (unless it has been suspended before deleting.)

There is an example command here that shows how to wield kubectl patch in order to remove a finalizer, without invoking kubectl edit or necessarily requiring that you drop into an editor or imperative workflow.

https://kubernetes.io/blog/2021/05/14/using-finalizers-to-control-deletion/#understanding-finalizers

kubectl patch hr <myHelmRelease> -p '{"metadata":{"finalizers":null}}'

@stefanprodan
Copy link
Member

I think helm-controller should behave like kustomize-controller. To avoid deadocks of a tenant's namespace, if the tenant service account is not found, kustomize-controller logs the error, then it removes the Kustomization object leaving the reconciled resources in place. cc @hiddeco

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

Successfully merging a pull request may close this issue.

3 participants