Make standalone installation kustomizable.#1103
Conversation
|
This change is |
|
Hi @jazzsir. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/ok-to-test |
|
|
||
| If you want to deploy the KFServing controller in `kubeflow` namespace. | ||
|
|
||
| 1. Set the namespace field to `kubeflow` in overlays/env/kustomization.yaml and [ingressGateway](https://github.com/kubeflow/kfserving/blob/master/config/configmap/inferenceservice.yaml#L94) to `kubeflow-gateway.kubeflow` in [config/configmap/inferenceservice.yaml](https://github.com/kubeflow/kfserving/blob/master/config/configmap/inferenceservice.yaml). |
There was a problem hiding this comment.
Can we kustomize the gateway config also?
| @@ -0,0 +1,28 @@ | |||
| apiVersion: kustomize.config.k8s.io/v1beta1 | |||
There was a problem hiding this comment.
Can this file gets merged to config/default/kustomization.yaml? not sure if we want this overlay, we can create an overlay for kubeflow though.
| kubectl apply -f ./install/$TAG/kfserving.yaml --validate=false | ||
| ``` | ||
|
|
||
| If you want to deploy the KFServing controller in `kubeflow` namespace. |
There was a problem hiding this comment.
I'd prefer creating a kubeflow overlay here https://github.com/kubeflow/kfserving/tree/master/config/overlays with the namespace and ingress gateway kustomization and then generate the additional release manifest in here https://github.com/kubeflow/kfserving/blob/master/hack/generate-install.sh
kustomize build config/kubeflow | sed s/:latest/:$TAG/ > $INSTALL_DIR/kubeflow_kfserving.yaml
There was a problem hiding this comment.
I got it, I added kubeflow overlays and updated README.
| - kind: ServiceAccount | ||
| name: default | ||
| namespace: system | ||
| namespace: kfserving-system |
There was a problem hiding this comment.
Here needs to be parameterized ?
There was a problem hiding this comment.
AFAIK, It is not used because HA is disabled, I just updated it for later.
Is there any reason why it shouldn't be changed?
| KFServing can also be installed standalone in `kubeflow` namespace. | ||
| ``` | ||
| TAG=v0.4.0 | ||
| kubectl kustomize ./config/overlays/kubeflow | sed s/:latest/:$TAG/ | kubectl apply -f - |
There was a problem hiding this comment.
Can we generate and materialize the kubeflow kfserving manifest yaml and check into the install directory as we change the kustomize config all the time ?
There was a problem hiding this comment.
Would you be more specific, please? I would like to know what do you want to do.
There was a problem hiding this comment.
@jazzsir we changed kfserving manifests quite a bit after 0.4.0 in master, so it is very likely no longer working with 0.4.0 release if we run the above command and generate from master. So I'd prefer add the above command to the install script and we can use that to generate the kubeflow manifest yaml for KFServing when we release 0.5.0.
INSTALL_DIR=./install/$TAG
INSTALL_KUBEFLOW_PATH=$INSTALL_DIR/kfserving_kubflow.yaml
kustomize build config/overlays/kubeflow | sed s/:latest/:$TAG/ > $INSTALL_KUBEFLOW_PATH
There was a problem hiding this comment.
@yuzisun That's right, so I also applied it to the install script (hack/generate-install.sh)
There was a problem hiding this comment.
I do not see it in generate-install.sh? also here can be removed as it won't work for 0.4.0
There was a problem hiding this comment.
@jazzsir actually this will not work as the v0.4.0 branch does not have the changes you made in master :(
There was a problem hiding this comment.
Let’s remove this instruction, we are releasing 0.5 soon anyway so we can generate the kubeflow manifest at that time. Otherwise this PR lgtm can be merged.
There was a problem hiding this comment.
I didn't come up with that, removed.
1. allow KFServing standalone to install in kubeflow namespace. 2. update README.md
Merge this overlay with config/default
Use kustomize embedded in kubectl to support env
Before deploying standalone installation, checking out TAG is needed to avoid working in master branch.
|
Thanks @jazzsir ! This is really useful! I am excited to see this going out in the upcoming release! /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuzisun The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
What this PR does / why we need it:
To allow KFServing standalone to install in kubeflow namespace, I made standalone installation kustomizable, and updated README.md
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #976
Special notes for your reviewer:
This is my second attempt to merge PR, because of confliction issue at #1019
Release note: