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

Use cli-utils kstatus library in wait logic #8661

Open
seaneagan opened this issue Aug 26, 2020 · 8 comments
Open

Use cli-utils kstatus library in wait logic #8661

seaneagan opened this issue Aug 26, 2020 · 8 comments
Labels
keep open proposal v4.x Issues deferred for v4 discussion

Comments

@seaneagan
Copy link

The cli-utils kstatus library has extensive logic for statusing and polling of resources. The existing helm wait logic is fairly limited (for example see #8660).

Would there be interest in at some point integrating kstatus into the wait logic?

Even if that is not something that can happen in the near term, would there be interest in taking learnings from the kstatus library's implementation and applying those to the helm wait logic in the short term?

@thomastaylor312
Copy link
Contributor

I am all for code reuse. The --wait logic has always been one of the more brittle parts of Helm, so if we can re-use a standard set of tools, I think that is the best solution. This would probably be a good candidate for the new HIP proposal system and I personally think it would be accepted. Is that something you'd be willing to write up and champion?

@seaneagan
Copy link
Author

I'm guessing we would want to wait for a 1.0 release of cli-utils / kstatus first. I'll try to check with the cli-utils team to see if they have any plans around that. If we could get that in place, I'd be happy to take a swing at writing a HIP.

@technosophos
Copy link
Member

Keep in mind that we must follow the SemVer restrictions for Helm. For that reason, this could be deferred until Helm 4 unless we are sure we are getting the same results that users already expect.

@seaneagan
Copy link
Author

If it is deemed a backward incompatible change, one way to still make it available in Helm 3 could be to require setting an environment variable e.g. HELM_EXPERIMENTAL_WAIT similar to HELM_EXPERIMENTAL_OCI.

@seaneagan
Copy link
Author

@mortent I think this would align well with one of the goals of kstatus, to encourage adoption of the recommended set of status conditions for kubernetes resources. Tagging this as "experimental" in Helm 3 could also allow Helm to track the latest releases of kstatus, while it works towards a 1.0 release. Do you foresee any concerns with this long term as a use case for kstatus?

@github-actions
Copy link

github-actions bot commented Dec 1, 2020

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@raffis
Copy link

raffis commented Nov 9, 2023

Whats the status of this? Would you accept a pr behind a feature flag for this?

What people can do now (just for the record) is utilizing a hook like the following.
However this only forks for charts being owned and obviously it's not a solution for 3rd party charts.

{{- if and .Values.waitFor.enabled }}
apiVersion: v1
kind: ServiceAccount
metadata:
  name: {{ template "service.fullname" . }}-wait-for-crd
  annotations:
    "helm.sh/hook": post-install, post-upgrade
    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded  
---
kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  name: {{ template "service.fullname" . }}-wait-for-crd
  annotations:
    "helm.sh/hook": post-install, post-upgrade
    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded  
rules:
  - apiGroups:
      - myapigroup
    resources:
      - resource
    verbs:
      - get
      - list
      - watch
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
  name: {{ template "service.fullname" . }}-wait-for-crd
  annotations:
    "helm.sh/hook": post-install, post-upgrade
    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded  
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: Role
  name: {{ template "service.fullname" . }}-wait-for-crd
subjects:
- kind: ServiceAccount
  name: {{ template "service.fullname" . }}-wait-for-crd
  namespace: {{ .Release.Namespace }}
---
apiVersion: v1
kind: Pod
metadata:
  name: {{ template "service.fullname" . }}-wait-for-crd
  annotations:
    "helm.sh/hook": post-install, post-upgrade
    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded  
spec:
  restartPolicy: OnFailure
  serviceAccountName: {{ template "service.fullname" . }}-wait-for-crd
  containers:
    - args:
      - --namespace={{ .Release.Namespace }}
      - wait
      - --for=condition=Ready=True
      - mycrdresource/{{ include "service.fullname" . }}
      - --timeout={{ .Values.waitFor.timeout }}
      image: bitnami/kubectl:1.28.3
      imagePullPolicy: IfNotPresent
      name: wait-for
{{- end }}

@gjenkins8 gjenkins8 added the v4.x Issues deferred for v4 discussion label Nov 18, 2023
@gjenkins8
Copy link
Contributor

kubectl wait's implementation might be worth considering too: https://github.com/kubernetes/kubectl/blob/bd48fd4c165349ab529fcaf27df872705ec1d105/pkg/cmd/wait/wait.go#L442-L521 (also curious as to why kubectl wait didn't use kstatus)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep open proposal v4.x Issues deferred for v4 discussion
Projects
None yet
Development

No branches or pull requests

5 participants