Skip to content

Commit

Permalink
* OpenShift patches to Kustomize manifests
Browse files Browse the repository at this point in the history
* Remove manager's rbac-proxy and add ODH requried network policies
* Workaround for Kustomize bug about creationTimestamp
  See: kubernetes-sigs/kustomize#5031

  Without this workaround, some Kustomize versions are generating
  `creationTimestamp: "null"` (null as a string).

Signed-off-by: Edgar Hernández <23639005+israel-hdez@users.noreply.github.com>
  • Loading branch information
israel-hdez committed Jul 12, 2023
1 parent 4e7f0df commit 9d58b32
Show file tree
Hide file tree
Showing 14 changed files with 125 additions and 73 deletions.
2 changes: 1 addition & 1 deletion config/crd/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ kind: Kustomization
resources:
- serving.kserve.io_inferenceservices.yaml
- serving.kserve.io_trainedmodels.yaml
- serving.kserve.io_clusterservingruntimes.yaml
# - serving.kserve.io_clusterservingruntimes.yaml # Not supported in ODH
- serving.kserve.io_servingruntimes.yaml
- serving.kserve.io_inferencegraphs.yaml

Expand Down
2 changes: 1 addition & 1 deletion config/default/cainjection_conversion_webhook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
cert-manager.io/inject-ca-from: $(kserveNamespace)/serving-cert
service.beta.openshift.io/inject-cabundle: "true"
name: inferenceservices.serving.kserve.io
1 change: 1 addition & 0 deletions config/default/inferenceservice_conversion_webhook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: inferenceservices.serving.kserve.io
creationTimestamp: null
spec:
preserveUnknownFields: false
conversion:
Expand Down
2 changes: 1 addition & 1 deletion config/default/isvc_mutatingwebhook_cainjection_patch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ kind: MutatingWebhookConfiguration
metadata:
name: inferenceservice.serving.kserve.io
annotations:
cert-manager.io/inject-ca-from: $(kserveNamespace)/serving-cert
service.beta.openshift.io/inject-cabundle: "true"
webhooks:
- name: inferenceservice.kserve-webhook-server.defaulter
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ kind: ValidatingWebhookConfiguration
metadata:
name: inferenceservice.serving.kserve.io
annotations:
cert-manager.io/inject-ca-from: $(kserveNamespace)/serving-cert
service.beta.openshift.io/inject-cabundle: "true"
webhooks:
- name: inferenceservice.kserve-webhook-server.validator
33 changes: 31 additions & 2 deletions config/default/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ kind: Kustomization
# Adds namespace to all resources.
namespace: kserve

# Add recommended Kubernetes labels
commonLabels:
app.kubernetes.io/part-of: kserve

# Labels to add to all resources and selectors.
#commonLabels:
# app.kubernetes.io/name: kserve
Expand All @@ -13,7 +17,8 @@ resources:
- ../rbac
- ../manager
- ../webhook
- ../certmanager
# - ../certmanager # not needed, because ODH is using OpenShift's serving certificates for WebHooks
- network-policies.yaml # ODH specific

generatorOptions:
disableNameSuffixHash: true
Expand Down Expand Up @@ -156,11 +161,35 @@ replacements:
#- manager_prometheus_metrics_patch.yaml
patches:
- path: manager_image_patch.yaml
- path: manager_auth_proxy_patch.yaml
#- path: manager_auth_proxy_patch.yaml
- path: isvc_mutatingwebhook_cainjection_patch.yaml
- path: isvc_validatingwebhook_cainjection_patch.yaml
- path: trainedmodel_mutatingwebhook_cainjection_patch.yaml
- path: trainedmodel_validatingwebhook_cainjection_patch.yaml
- path: svc_webhook_cainjection_patch.yaml
- path: manager_resources_patch.yaml
- path: inferenceservice_conversion_webhook.yaml
- path: cainjection_conversion_webhook.yaml

patches:
# Since OpenShift serving-certificates are being used,
# remove CA bundle placeholders
- patch: |-
- op: remove
path: "/spec/conversion/webhook/clientConfig/caBundle"
target:
kind: CustomResourceDefinition
name: inferenceservices.serving.kserve.io
- patch: |-
- op: remove
path: "/webhooks/0/clientConfig/caBundle"
- op: remove
path: "/webhooks/1/clientConfig/caBundle"
target:
kind: MutatingWebhookConfiguration
- patch: |-
- op: remove
path: "/webhooks/0/clientConfig/caBundle"
target:
kind: ValidatingWebhookConfiguration
11 changes: 11 additions & 0 deletions config/default/network-policies.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: kserve-controller-manager
spec:
podSelector:
matchLabels:
control-plane: kserve-controller-manager
ingress:
- {}
7 changes: 7 additions & 0 deletions config/default/svc_webhook_cainjection_patch.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: v1
kind: Service
metadata:
name: kserve-webhook-server-service
namespace: kserve
annotations:
service.beta.openshift.io/serving-cert-secret-name: kserve-webhook-server-cert
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ kind: ValidatingWebhookConfiguration
metadata:
name: inferencegraph.serving.kserve.io
annotations:
cert-manager.io/inject-ca-from: $(kserveNamespace)/serving-cert
service.beta.openshift.io/inject-cabundle: "true"
webhooks:
- name: inferencegraph.kserve-webhook-server.validator
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ kind: ValidatingWebhookConfiguration
metadata:
name: trainedmodel.serving.kserve.io
annotations:
cert-manager.io/inject-ca-from: $(kserveNamespace)/serving-cert
service.beta.openshift.io/inject-cabundle: "true"
webhooks:
- name: trainedmodel.kserve-webhook-server.validator
29 changes: 15 additions & 14 deletions pkg/apis/serving/v1beta1/predictor_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,14 @@ func (m *ModelSpec) GetSupportingRuntimes(cl client.Client, namespace string, is
// Sort namespace-scoped runtimes by created timestamp desc and name asc.
sortServingRuntimeList(runtimes)

// List all cluster-scoped runtimes.
clusterRuntimes := &v1alpha1.ClusterServingRuntimeList{}
if err := cl.List(context.TODO(), clusterRuntimes); err != nil {
return nil, err
}
// Sort cluster-scoped runtimes by created timestamp desc and name asc.
sortClusterServingRuntimeList(clusterRuntimes)
// ODH does not support ClusterServingRuntimes
//// List all cluster-scoped runtimes.
//clusterRuntimes := &v1alpha1.ClusterServingRuntimeList{}
//if err := cl.List(context.TODO(), clusterRuntimes); err != nil {
// return nil, err
//}
//// Sort cluster-scoped runtimes by created timestamp desc and name asc.
//sortClusterServingRuntimeList(clusterRuntimes)

srSpecs := []v1alpha1.SupportedRuntime{}
for i := range runtimes.Items {
Expand All @@ -116,13 +117,13 @@ func (m *ModelSpec) GetSupportingRuntimes(cl client.Client, namespace string, is
}
}

for i := range clusterRuntimes.Items {
crt := &clusterRuntimes.Items[i]
if !crt.Spec.IsDisabled() && crt.Spec.IsMultiModelRuntime() == isMMS &&
m.RuntimeSupportsModel(&crt.Spec) && crt.Spec.IsProtocolVersionSupported(modelProtcolVersion) {
srSpecs = append(srSpecs, v1alpha1.SupportedRuntime{Name: crt.GetName(), Spec: crt.Spec})
}
}
//for i := range clusterRuntimes.Items {
// crt := &clusterRuntimes.Items[i]
// if !crt.Spec.IsDisabled() && crt.Spec.IsMultiModelRuntime() == isMMS &&
// m.RuntimeSupportsModel(&crt.Spec) && crt.Spec.IsProtocolVersionSupported(modelProtcolVersion) {
// srSpecs = append(srSpecs, v1alpha1.SupportedRuntime{Name: crt.GetName(), Spec: crt.Spec})
// }
//}
return srSpecs, nil
}

Expand Down
55 changes: 28 additions & 27 deletions pkg/apis/serving/v1beta1/predictor_model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestGetSupportingRuntimes(t *testing.T) {
pmmlRuntime := "pmml-runtime"
mlserverRuntime := "mlserver-runtime"
xgboostRuntime := "xgboost-runtime"
clusterServingRuntimePrefix := "cluster-"
//clusterServingRuntimePrefix := "cluster-"

protocolV2 := constants.ProtocolV2
protocolV1 := constants.ProtocolV1
Expand Down Expand Up @@ -179,28 +179,29 @@ func TestGetSupportingRuntimes(t *testing.T) {
},
}

clusterRuntimes := &v1alpha1.ClusterServingRuntimeList{
Items: []v1alpha1.ClusterServingRuntime{
{
ObjectMeta: metav1.ObjectMeta{
Name: clusterServingRuntimePrefix + mlserverRuntime,
},
Spec: servingRuntimeSpecs[mlserverRuntime],
},
{
ObjectMeta: metav1.ObjectMeta{
Name: clusterServingRuntimePrefix + tfRuntime,
},
Spec: servingRuntimeSpecs[tfRuntime],
},
{
ObjectMeta: metav1.ObjectMeta{
Name: clusterServingRuntimePrefix + xgboostRuntime,
},
Spec: servingRuntimeSpecs[xgboostRuntime],
},
},
}
// ODH does not support ClusterServingRuntimeList
//clusterRuntimes := &v1alpha1.ClusterServingRuntimeList{
// Items: []v1alpha1.ClusterServingRuntime{
// {
// ObjectMeta: metav1.ObjectMeta{
// Name: clusterServingRuntimePrefix + mlserverRuntime,
// },
// Spec: servingRuntimeSpecs[mlserverRuntime],
// },
// {
// ObjectMeta: metav1.ObjectMeta{
// Name: clusterServingRuntimePrefix + tfRuntime,
// },
// Spec: servingRuntimeSpecs[tfRuntime],
// },
// {
// ObjectMeta: metav1.ObjectMeta{
// Name: clusterServingRuntimePrefix + xgboostRuntime,
// },
// Spec: servingRuntimeSpecs[xgboostRuntime],
// },
// },
//}

var storageUri = "s3://test/model"
scenarios := map[string]struct {
Expand All @@ -218,7 +219,7 @@ func TestGetSupportingRuntimes(t *testing.T) {
},
},
isMMS: false,
expected: []v1alpha1.SupportedRuntime{{Name: tfRuntime, Spec: servingRuntimeSpecs[tfRuntime]}, {Name: clusterServingRuntimePrefix + tfRuntime, Spec: servingRuntimeSpecs[tfRuntime]}},
expected: []v1alpha1.SupportedRuntime{{Name: tfRuntime, Spec: servingRuntimeSpecs[tfRuntime]} /*, {Name: clusterServingRuntimePrefix + tfRuntime, Spec: servingRuntimeSpecs[tfRuntime]}*/},
},
"RuntimeNotFound": {
spec: &ModelSpec{
Expand Down Expand Up @@ -255,7 +256,7 @@ func TestGetSupportingRuntimes(t *testing.T) {
},
},
isMMS: true,
expected: []v1alpha1.SupportedRuntime{{Name: clusterServingRuntimePrefix + mlserverRuntime, Spec: servingRuntimeSpecs[mlserverRuntime]}},
expected: []v1alpha1.SupportedRuntime{ /*{Name: clusterServingRuntimePrefix + mlserverRuntime, Spec: servingRuntimeSpecs[mlserverRuntime]}*/ },
},
"SMSRuntimeModelFormatSpecified": {
spec: &ModelSpec{
Expand All @@ -280,7 +281,7 @@ func TestGetSupportingRuntimes(t *testing.T) {
},
},
isMMS: false,
expected: []v1alpha1.SupportedRuntime{{Name: clusterServingRuntimePrefix + xgboostRuntime, Spec: servingRuntimeSpecs[xgboostRuntime]}},
expected: []v1alpha1.SupportedRuntime{ /*{Name: clusterServingRuntimePrefix + xgboostRuntime, Spec: servingRuntimeSpecs[xgboostRuntime]}*/ },
},
"RuntimeV1ProtocolNotFound": {
spec: &ModelSpec{
Expand All @@ -303,7 +304,7 @@ func TestGetSupportingRuntimes(t *testing.T) {
t.Errorf("unable to add scheme : %v", err)
}

mockClient := fake.NewClientBuilder().WithLists(runtimes, clusterRuntimes).WithScheme(s).Build()
mockClient := fake.NewClientBuilder().WithLists(runtimes /*, clusterRuntimes*/).WithScheme(s).Build()
for name, scenario := range scenarios {
t.Run(name, func(t *testing.T) {
res, _ := scenario.spec.GetSupportingRuntimes(mockClient, namespace, scenario.isMMS)
Expand Down
18 changes: 10 additions & 8 deletions pkg/controller/v1beta1/inferenceservice/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,14 +246,16 @@ func GetServingRuntime(cl client.Client, name string, namespace string) (*v1alph
return nil, err
}

clusterRuntime := &v1alpha1.ClusterServingRuntime{}
err = cl.Get(context.TODO(), client.ObjectKey{Name: name}, clusterRuntime)
if err == nil {
return &clusterRuntime.Spec, nil
} else if !errors.IsNotFound(err) {
return nil, err
}
return nil, goerrors.New("No ServingRuntimes or ClusterServingRuntimes with the name: " + name)
// ODH does not support ClusterServingRuntimes
//clusterRuntime := &v1alpha1.ClusterServingRuntime{}
//err = cl.Get(context.TODO(), client.ObjectKey{Name: name}, clusterRuntime)
//if err == nil {
// return &clusterRuntime.Spec, nil
//} else if !errors.IsNotFound(err) {
// return nil, err
//}

return nil, goerrors.New("No ServingRuntimes with the name: " + name)
}

// ReplacePlaceholders Replace placeholders in runtime container by values from inferenceservice metadata
Expand Down
32 changes: 16 additions & 16 deletions pkg/controller/v1beta1/inferenceservice/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -941,16 +941,16 @@ func TestGetServingRuntime(t *testing.T) {
},
}

clusterRuntimes := &v1alpha1.ClusterServingRuntimeList{
Items: []v1alpha1.ClusterServingRuntime{
{
ObjectMeta: metav1.ObjectMeta{
Name: sklearnRuntime,
},
Spec: servingRuntimeSpecs[sklearnRuntime],
},
},
}
//clusterRuntimes := &v1alpha1.ClusterServingRuntimeList{
// Items: []v1alpha1.ClusterServingRuntime{
// {
// ObjectMeta: metav1.ObjectMeta{
// Name: sklearnRuntime,
// },
// Spec: servingRuntimeSpecs[sklearnRuntime],
// },
// },
//}

scenarios := map[string]struct {
runtimeName string
Expand All @@ -960,16 +960,16 @@ func TestGetServingRuntime(t *testing.T) {
runtimeName: tfRuntime,
expected: servingRuntimeSpecs[tfRuntime],
},
"ClusterServingRuntime": {
runtimeName: sklearnRuntime,
expected: servingRuntimeSpecs[sklearnRuntime],
},
//"ClusterServingRuntime": {
// runtimeName: sklearnRuntime,
// expected: servingRuntimeSpecs[sklearnRuntime],
//},
}

s := runtime.NewScheme()
v1alpha1.AddToScheme(s)

mockClient := fake.NewClientBuilder().WithLists(runtimes, clusterRuntimes).WithScheme(s).Build()
mockClient := fake.NewClientBuilder().WithLists(runtimes /*, clusterRuntimes*/).WithScheme(s).Build()
for name, scenario := range scenarios {
t.Run(name, func(t *testing.T) {
res, _ := GetServingRuntime(mockClient, scenario.runtimeName, namespace)
Expand All @@ -985,7 +985,7 @@ func TestGetServingRuntime(t *testing.T) {
if !g.Expect(res).To(gomega.BeNil()) {
t.Errorf("got %v, want %v", res, nil)
}
g.Expect(err.Error()).To(gomega.ContainSubstring("No ServingRuntimes or ClusterServingRuntimes with the name"))
g.Expect(err.Error()).To(gomega.ContainSubstring("No ServingRuntimes with the name"))
})

}
Expand Down

0 comments on commit 9d58b32

Please sign in to comment.