Add optional flag to use v1beta1 CronJob (pre 1.21) #6030
Conversation
Signed-off-by: Michael Nelson <[email protected]>
✅ Deploy Preview for kubeapps-dev canceled.
|
Signed-off-by: Michael Nelson <[email protected]>
|
@absoludity does this also include the other fix #6016? i.e. is its history linear with |
Yep, it's the previous commit shown in the commit history of the branch at: https://github.com/vmware-tanzu/kubeapps/commits/6021-cronjob-api |
OK cheers, sorry I missed that 😓 will test ASAP and report back! |
Cool thanks. Just remember, although the code is all in this one branch, it's two separate images that you'll need, one for the apprepository-controller and the other for the kubeapps-apis service. |
|
@absoludity working great!!
|
|
Great? Not sure if the blank info in the first screenshot or the error in the third are something you expected? |
Ran out of time to do proper thorough testing but initial tests are working as expected. I'll do more in the morning and confirm back here. Thanks again. |
|
@absoludity regarding the secrets-related error above, I am seeing 500s from HAProxy to k8s api for both configmaps and secrets: Still looking into it. |
|
OK. I'm off today but will look deeper tomorrow. I think I only had time to test locally that the repos were being synced, so it could be that I can reproduce the issue if I deploy an app on a 1.20 cluster. |
Everything looking good on my end. Our errors came about from a bug w/ HAProxy similar to haproxy/haproxy#1766. |
| // a v1 batch listener or a v1beta1 batch listener. | ||
| // | ||
| // This allows users on 1.20 to continue to use the latest release. | ||
| func (c *Controller) setBatchLister(useV1Beta1 bool, kubeInformerFactory kubeinformers.SharedInformerFactory) { |
There was a problem hiding this comment.
Kind of makes me wonder if this flag is necessary at all. Could the code just iterate over both v1 and v1beta1 and query the k8s API to see which resource is defined in the cluster, and use that?
There was a problem hiding this comment.
Then you probably wouldn't need as many if checks/logic branches in the rest of the code.
There was a problem hiding this comment.
We could just iterate implicitly but that has drawbacks as well: we'd need to do that iteration at each call-site (where the if logic branches are, which could be extracted, yep), or just iterate once here and keep the conditionals, but more importantly, it'd become an implicit decision: I'm happy for this to be explicit with a branch in each part because we want to remove it again soon. It's not a supported feature (why I left the chart requirement unchanged).
Description of the change
This PR adds an optional boolean flag that for the apprepository controller,
--v1-beta1-cron-jobs, which can be set to ensure that the app repository controller creates and watches the older v1beta1 cronjobs.Note that, although enabling this functionality to help with #6021 , we won't be removing the restriction in the Chart.yaml for > k8s 1.21, so to install the chart on a cluster prior to 1.21 you'll need to manually comment out that line in the Chart.yaml (this just ensures we don't go back and start supporting older k8s versions officially).
To use the flag, you can add the following to your values.yaml:
Benefits
People who need to can choose to install Kubeapps explicitly on 1.20 .
Possible drawbacks
Applicable issues
Additional information
@mecampbellsoup Can you please test the resulting image on your 1.20 dev cluster and works as you expect (I tested locally with a 1.20 kind cluster and see app repos being synced etc.)