feat: added hook to job to delete/recreate during upgrade/rollback

[jliedy]

Added annotations to the job in order for the migration job to be deleted after a successful run.

Description

In reference to Issue #69 I've added hook annotations to the migration job in order for the job to be deleted and then recreated during a helm upgrade/rollback.

References

For views on how to add hooks to jobs, one can reference the Helm documentation at: https://helm.sh/docs/topics/charts_hooks/

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

[jliedy]

I've noticed something awkward when deleting a job before the deployment can init is causing the k8s-wait-for initcontainer to not start if you're using v1.7 or later. Gonna see what I can figure out.

[jliedy]

So with groundnuty/k8s-wait-for v1.6, it didn't cause things to get jammed up with starting the deployment if the job completed and got deleted. v1.7 and later, it DID cause the deployment to fail to init due to the job not being found. Thus, deleting the job at the end of an upgrade/rollback could cause issues with starting the deployment.

Setting the job to be deleted at the beginning of the install/upgrade/rollback process will allow the job to linger as a way to ensure that the deployment initContainer will be able to successfully validate job completion.

groundnuty/k8s-wait-for can remain at v1.6 for flexibility (if the job doesn't exist, the deployment can still init), but using v1.7 and later can help ensure that the new OpenFGA release doesn't actually start until after the db migration job has completed.

Also, groundnuty/k8s-wait-for v1.7 and later uses get and/or list when used in this fashion.

[jliedy]

Found an issue that by doing this, the job doesn't use the latest image, but the previously deployed one.

[jliedy]

This solution still isn't my favorite. If you deploy with helm using the --wait flag, it will cause the deploy process to fail as the deployment won't start until the migration job runs, and the migration job won't get applied until the deployment succeeds.

[jliedy]

I've gotten this style to work fairly consistently with a separate helm chart that I manage. Able to consistently deploy and update without issue.

[jon-whit]

@jliedy sorry this one has fallen through the cracks. We've been heads down working on some big features in the core OpenFGA server. Would you mind rebasing on main? We can take a final look afterwards. Thanks!

🙇

[jliedy]

I've had my head down in some stuff myself, but we've been running in openshift what I've suggested with the change here. I'll try to get back to this this week with an update.

[jon-whit]

@jliedy just following up? Any updates/progress since our last chat?

[jliedy]

Sorry, no, I stink. Had some personal things that needed to be taken care of.

Lemme go ahead and get cracking on this now.

[jliedy]

This wasn't supposed to be closed.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jliedy / name: Jonathan Liedy (244a04e, 9d78537, fbc8c4e, 3e1c0e7, 2f7e879, 9bd0b13, f97dbdf, 0ef93e7, 58cda47)

[linux-foundation-easycla]

• ❌ - login: @jliedy / name: Jonathan Liedy . The commit (244a04e, 9d78537, fbc8c4e, 3e1c0e7, 2f7e879, 9bd0b13) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[rhamzeh]

Hi @jliedy - thanks!

I converted it to a draft PR. Once you believe it's ready for review, feel free to mark it as such

[jliedy]

I think it's ready to be reviewed. Let me know if you'd prefer me to add the job migration annotations for the helm hooks to the values.yaml file instead of the job.yaml file.

[rhamzeh]

Thanks! Will refer to the team for review

[jon-whit]

I think it's ready to be reviewed. Let me know if you'd prefer me to add the job migration annotations for the helm hooks to the values.yaml file instead of the job.yaml file.

@jliedy everything looks good, but I think this would be a good idea as it is more extensible. If you are using the chart and you wish not to have these hooks, then that should be overridable with the values. Mind making that change real quick and then we can merge.

[jliedy]

Yeah, I could make an argument for it either way so I figured I'd put the option out there.

Done.