teamhephy / controller

Hephy Workflow Controller (API)
https://teamhephy.com
MIT License
14 stars 26 forks source link

chore(chart): update ingress-rule in chart #155

Closed kingdonb closed 2 years ago

kingdonb commented 2 years ago

The chart ingress rule needs to be updated to match the changes to controller, (in the next release we will support networking.k8s.io/v1 Ingress only, and the extensions/v1beta1 Ingress will be removed.)

The expectation is generally that people are upgrading their clusters and the older Ingress and other beta versions will be automatically upgraded, (but they must also be upgraded in the helm charts.)

Note: maybe we should add ingressClassName support before we release this change. It can be merged as-is, however I only stumbled upon this because I was trying to patch ingressClassName into the ingress rule, and I found the helm chart wasn't actually rendering the new version.

I will upgrade to K8s 1.22 again soon so we can flush out any more issues like this, (hopefully prior to making a next release!)

kingdonb commented 2 years ago

Interesting that only the Ingress includes a namespace value in metadata, and none of the other resources do. I wonder why? (I tripped over this while I was trying to patch an "ingressclass" annotation into the extensions/v1beta1 ingress, to see if it might still work. I have upgraded to an ingress-nginx release that is based on the v1 ingress, so it seems that annotation is ignored. The only way for me to reasonably make it work will be to publish a new chart release, with at least this update!)

Then I can always patch ingressClassName into the networking.k8s.io/v1 ingress, but it will not be possible without upgrading the template to the v1 ingress first, since there was no IngressClass resource or ingressClassName field in the Ingress resource from extensions/v1beta1.

kingdonb commented 2 years ago

I added one more commit to create preliminary support for IngressClassName from networking.k8s.io/v1 Ingress.

We can exclude that one if you want since it's not finished, but I tested it with helm template and kubectl apply --dry-run=server -f - so I can see it is at least well-formed syntax, and should be safe to merge.

This change will not be usable without a corresponding change in the teamhephy/workflow chart, although that also might not be strictly true (the user can add the value to their own values, and I think controller will just pick it up...)

Let me know if you want me to rewrite this as two separate PRs, or just go ahead and merge it.

I don't know if you have time to consider making a release right now, I just found this was missing and it should go in before Hephy v2.24, since I think we were intending to drop support for everything in extensions/v1beta1 before that release. 🥇

kingdonb commented 2 years ago

I am testing this from end to end and right now that means publishing a beta chart for controller v2.23.0

That means I need a functioning chartmuseum of my own, which I think I can finally do now that I have a stable (permanent) minio with hosted volume, serving as S3 storage.

At least it's meant to be permanent... I'm not really engineering for "permanent" as it only hosts beta charts for now!

kingdonb commented 2 years ago

It seems to be working, though I am not at all confident with my capability of testing a chart with dependents in Helm Controller and being sure of what chart version has actually been applied. It's not clear if helm dep update is being ran, or if the chart versions used are the expanded charts in charts/workflow/charts/controller (I think it's that one, but...)

Anyway, I can use it like this for now, there is no urgency to do a release just for me, but now that it's testable I think it might be worth a release?

kingdonb commented 2 years ago

The bug I was observing in the latest controller release (where certain combinations of Dockerfiles and Procfiles result in surprises if you have proc types other than web as your default type) is still present, as of the controller I am running in kingdonb/controller:git-7beda111 so I have to do this annoying thing every time I boot my cluster from scratch:

deis ps:scale -a simplest-commitbee cron=0
deis ps:scale -a simplest-commitbee cron=1
deis ps:scale -a ob-mirror cron=0
deis ps:scale -a ob-mirror cron=1

... but other than that, all of my state is restored and I'm able to use Workflow on K8s v1.23.1 without issue, (others will need to wait for this release, or follow my example in these HelmRelease artifacts):

The beta charts are going up at https://charts.hephy.pro as I get around to validating each of them 👍 that repo is to be driven by this one: https://github.com/kingdonb/hephy-workflow-beta

You can see mechanically what it is that I mean by "validating each" sort of right here:

https://github.com/kingdonb/bootstrap-repo/pull/22/files#diff-2a3d2521ae977f4e17a6d4204936f6b4cf21a449130fd75373a4555a5817013a

It's possible I get some druthers this week before my holiday is over and visit each repo and see what it needs for a release, then I'll update this deployment with the rest, and finally we can review it all at once at hephy-workflow-beta where we can implement some new CI around it.

Would be really nice to have a git repo that just hosts the charts for every component, rendered out with the latest version of each one, where we could attach a CI job that just installs and runs the e2e against it. The fabled e2e! Back at last. Maybe I can even use Flux image automation for this 🙌