alauda / kubeflow-chart

Kubeflow helm chart
Apache License 2.0
133 stars 24 forks source link

Knative serving version hpa to autoscaling/v2 as autoscaling/v2beta2 was deprecated in k8s 1.23 #41

Open DreamingRaven opened 1 year ago

DreamingRaven commented 1 year ago

Hey I installed this chart but I found on my kubernetes 1.27.1 cluster that:

knative

I checked what versions were installed in my cluster, and found that autoscaling/v2beta2 is not present.

# kubectl api-versions
...
autoscaling/v1
autoscaling/v2
...

I think the easiest fix is to change the chart version of knative as newer versions fix this since it comes from knative. Also allowing for knative to be disabled to use one that is already in the cluster like the other resources would be excellent as its the only one that doesn't:

https://github.com/alauda/kubeflow-chart/blob/7c7b6e9ee44b8cd258b58bac2d9cca8cd28c75e7/charts/kubeflow/Chart.yaml#L42-L43

I am going to fix these in my fork but I thought I would open an issue to let you know! Otherwise tho I hope you have a lovely weekend, and thanks for the helm chart, this is sorely needed for kubeflow!

typhoonzero commented 1 year ago

For a quick fix, you can install knative-serving from knative official repo to fit the Kubernetes version > 1.25, then turn off auto installing knative-serving in values.yaml before installing this chart.

https://github.com/alauda/kubeflow-chart/blob/84ed10058e085b18eb8dfa96e640184a388ca690/charts/kubeflow/values.yaml#L81-L82

DreamingRaven commented 1 year ago

Hey thanks for the response, the only issue is that flag doesn't actually have any affect! It will always try to install the knative-serving chart as there is no condition for it like the others in the chart dependencies: https://github.com/alauda/kubeflow-chart/blob/84ed10058e085b18eb8dfa96e640184a388ca690/charts/kubeflow/Chart.yaml#L31-L46

We would probably need to add that in something like:

    dependencies:
    - name: cert-manager
      version: "1.5.0"
      alias: cert-manager
      condition: cert-manager.enabled
    - name: dex
      version: "2.31.2"
      condition: dex.enabled
    - name: istio
      version: "1.14.1"
      condition: istio.enabled
    - name: knative-serving
      version: "1.2.5"
      condition: knative-serving.enabled # <----
    - name: minio
      version: "1.0"
      condition: minio.enabled

Also as another note, I use rook-ceph in my cluster to provision object storage. I note everything is build with minio as the object storage provider, is there a way to override this?