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

fix: Replace migrate hooks with longer timeouts and retries #136

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tamalsaha
Copy link
Contributor

@tamalsaha tamalsaha commented Jun 4, 2024

This makes it possible to use this chart with fluxcd

Description

Helm charts and Jobs don't work well together. This is evident from many prs trying to address some aspects of this issue. I try to document the issues as best I can think of here: https://x.com/tsaha/status/1805382111844778275

This pr tries to address this issue as best as possible I think.

If the hooks are used, we can't use fluxcd to install this chart with --wait flag. See here: fluxcd/flux2#1085 (comment)

What seems to work well in our experience is just run the migrate job at the same time as the potential db sub-chart, increase the timeout of the migrate job and increase number of retries. #130 pr ensures that at least one successful run of the migrate job will be considered successful. So, extending the timeout and increasing retries works well even if the db subchart takes unusally long time to be ready.

References

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@tamalsaha tamalsaha requested review from a team as code owners June 4, 2024 17:48
@tamalsaha tamalsaha changed the title Replace migrate hooks with longer timeouts and retries feat: Replace migrate hooks with longer timeouts and retries Jun 4, 2024
@tamalsaha tamalsaha changed the title feat: Replace migrate hooks with longer timeouts and retries fxi: Replace migrate hooks with longer timeouts and retries Jun 4, 2024
@tamalsaha tamalsaha changed the title fxi: Replace migrate hooks with longer timeouts and retries fix: Use pre hooks with longer timeouts and retries and option to auto cleanup Jun 25, 2024
@tamalsaha tamalsaha force-pushed the nohook branch 4 times, most recently from 5408b1c to 9e7b50d Compare June 25, 2024 13:50
@tamalsaha tamalsaha changed the title fix: Use pre hooks with longer timeouts and retries and option to auto cleanup fix: Replace migrate hooks with longer timeouts and retries Jun 25, 2024
@tamalsaha tamalsaha force-pushed the nohook branch 2 times, most recently from 95d47a1 to a570549 Compare June 25, 2024 13:53
@tamalsaha
Copy link
Contributor Author

Tested in ArgoCD using

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: openfga
  namespace: argocd
spec:
  project: default
  source:
    chart: openfga
    repoURL: ghcr.io/appscode-charts
    targetRevision: 0.2.13
    helm:
      releaseName: openfga
      valuesObject:
        datastore:
          engine: postgres
          uri: "postgres://postgres:[email protected]:5432/postgres?sslmode=disable"
        postgresql:
          enabled: true
          auth:
            postgresPassword: password
            database: postgres
  destination:
    server: "https://kubernetes.default.svc"
    namespace: default

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 this pull request may close these issues.

1 participant