Skip to content

Commit

Permalink
Fix explainer endpoint not working with path based routing (kserve#3257)
Browse files Browse the repository at this point in the history 
* Fix explainer not working with path based routing

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Add test

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Add explainer e2e test for path based routing

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Rebase master

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

---------

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>
  • Loading branch information
@sivanantha321
sivanantha321 authored Sep 19, 2024
1 parent 02293ac commit 536fc9b
Showing 6 changed files with 260 additions and 4 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/e2e-test.yml
View file
Original file line number Diff line number Diff line change
@@ -552,6 +552,12 @@ jobs:
name: ${{ env.TRANSFORMER_ARTIFACT_PREFIX }}-${{ env.IMAGE_TRANSFORMER_IMG }}-${{ github.sha }}
path: ./tmp

- name: Download Art Explainer image
uses: actions/download-artifact@v4
with:
name: ${{ env.EXPLAINER_ARTIFACT_PREFIX }}-${{ env.ART_IMG }}-${{ github.sha }}
path: ./tmp

- name: Load docker images
uses: ./.github/actions/load-docker-images
with:
4 changes: 4 additions & 0 deletions pkg/constants/constants.go
View file
Original file line number Diff line number Diff line change
@@ -560,6 +560,10 @@ func ExplainPrefix() string {
return "^/v1/models/[\\w-]+:explain$"
}

func PathBasedExplainPrefix() string {
return "(/v1/models/[\\w-]+:explain)$"
}

func VirtualServiceHostname(name string, predictorHostName string) string {
index := strings.Index(predictorHostName, ".")
return name + predictorHostName[index:]
35 changes: 35 additions & 0 deletions pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/ingress_reconciler.go
View file
Original file line number Diff line number Diff line change
@@ -486,6 +486,41 @@ func createIngress(isvc *v1beta1.InferenceService, useDefault bool, config *v1be
url.Path = strings.TrimSuffix(path, "/") // remove trailing "/" if present
url.Host = config.IngressDomain
// In this case, we have a path-based URL so we add a path-based rule
if isvc.Spec.Explainer != nil {
httpRoutes = append(httpRoutes, &istiov1beta1.HTTPRoute{
Match: []*istiov1beta1.HTTPMatchRequest{
{
Uri: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: url.Path + constants.PathBasedExplainPrefix(),
},
},
Authority: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.HostRegExp(url.Host),
},
},
Gateways: []string{config.IngressGateway},
},
},
Rewrite: &istiov1beta1.HTTPRewrite{
UriRegexRewrite: &istiov1beta1.RegexRewrite{
Match: url.Path + constants.PathBasedExplainPrefix(),
Rewrite: `\1`,
},
},
Route: []*istiov1beta1.HTTPRouteDestination{
createHTTPRouteDestination(config.LocalGatewayServiceName),
},
Headers: &istiov1beta1.Headers{
Request: &istiov1beta1.Headers_HeaderOperations{
Set: map[string]string{
"Host": network.GetServiceHostname(expBackend, isvc.Namespace),
},
},
},
})
}
httpRoutes = append(httpRoutes, &istiov1beta1.HTTPRoute{
Match: []*istiov1beta1.HTTPMatchRequest{
{
214 changes: 212 additions & 2 deletions pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/ingress_reconciler_test.go
View file
Original file line number Diff line number Diff line change
@@ -22,8 +22,6 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/kserve/kserve/pkg/apis/serving/v1beta1"
"github.com/kserve/kserve/pkg/constants"
"github.com/onsi/gomega"
gomegaTypes "github.com/onsi/gomega/types"
"google.golang.org/protobuf/testing/protocmp"
@@ -34,6 +32,9 @@ import (
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
"knative.dev/pkg/network"

"github.com/kserve/kserve/pkg/apis/serving/v1beta1"
"github.com/kserve/kserve/pkg/constants"
)

func TestCreateVirtualService(t *testing.T) {
@@ -948,6 +949,215 @@ func TestCreateVirtualService(t *testing.T) {
},
},
},
}, {
name: "found predictor and explainer status with path template",
ingressConfig: &v1beta1.IngressConfig{
IngressGateway: constants.KnativeIngressGateway,
IngressServiceName: "someIngressServiceName",
LocalGateway: constants.KnativeLocalGateway,
LocalGatewayServiceName: "knative-local-gateway.istio-system.svc.cluster.local",
UrlScheme: "http",
IngressDomain: "my-domain.com",
PathTemplate: "/serving/{{ .Namespace }}/{{ .Name }}",
DisableIstioVirtualHost: false,
},
useDefault: false,
componentStatus: &v1beta1.InferenceServiceStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{
{
Type: v1beta1.PredictorReady,
Status: corev1.ConditionTrue,
},
{
Type: v1beta1.ExplainerReady,
Status: corev1.ConditionTrue,
},
},
},
Components: map[v1beta1.ComponentType]v1beta1.ComponentStatusSpec{
v1beta1.ExplainerComponent: {
URL: &apis.URL{
Scheme: "http",
Host: explainerHostname,
},
Address: &duckv1.Addressable{
URL: &apis.URL{
Scheme: "http",
Host: network.GetServiceHostname(constants.ExplainerServiceName(serviceName), namespace),
},
},
},
v1beta1.PredictorComponent: {
URL: &apis.URL{
Scheme: "http",
Host: predictorHostname,
},
Address: &duckv1.Addressable{
URL: &apis.URL{
Scheme: "http",
Host: network.GetServiceHostname(constants.PredictorServiceName(serviceName), namespace),
},
},
},
},
},
expectedService: &istioclientv1beta1.VirtualService{
ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace, Annotations: annotations, Labels: labels},
Spec: istiov1beta1.VirtualService{
Hosts: []string{serviceInternalHostName, serviceHostName, "my-domain.com"},
Gateways: []string{constants.KnativeLocalGateway, constants.IstioMeshGateway, constants.KnativeIngressGateway},
Http: []*istiov1beta1.HTTPRoute{
{
Match: []*istiov1beta1.HTTPMatchRequest{
{
Uri: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.ExplainPrefix(),
},
},
Authority: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.HostRegExp(network.GetServiceHostname(serviceName, namespace)),
},
},
Gateways: []string{constants.KnativeLocalGateway, constants.IstioMeshGateway},
},
{
Uri: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.ExplainPrefix(),
},
},
Authority: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.HostRegExp(constants.InferenceServiceHostName(serviceName, namespace, domain)),
},
},
Gateways: []string{constants.KnativeIngressGateway},
},
},
Route: []*istiov1beta1.HTTPRouteDestination{
{
Destination: &istiov1beta1.Destination{Host: constants.LocalGatewayHost, Port: &istiov1beta1.PortSelector{Number: constants.CommonDefaultHttpPort}},
Weight: 100,
},
},
Headers: &istiov1beta1.Headers{
Request: &istiov1beta1.Headers_HeaderOperations{Set: map[string]string{
"Host": network.GetServiceHostname(constants.ExplainerServiceName(serviceName), namespace)},
},
},
},
{
Match: []*istiov1beta1.HTTPMatchRequest{
{
Authority: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.HostRegExp(network.GetServiceHostname(serviceName, namespace)),
},
},
Gateways: []string{constants.KnativeLocalGateway, constants.IstioMeshGateway},
},
{
Authority: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.HostRegExp(constants.InferenceServiceHostName(serviceName, namespace, domain)),
},
},
Gateways: []string{constants.KnativeIngressGateway},
},
},
Route: []*istiov1beta1.HTTPRouteDestination{
{
Destination: &istiov1beta1.Destination{Host: constants.LocalGatewayHost, Port: &istiov1beta1.PortSelector{Number: constants.CommonDefaultHttpPort}},
Weight: 100,
},
},
Headers: &istiov1beta1.Headers{
Request: &istiov1beta1.Headers_HeaderOperations{Set: map[string]string{
"Host": network.GetServiceHostname(constants.PredictorServiceName(serviceName), namespace)}},
},
},
{
Match: []*istiov1beta1.HTTPMatchRequest{
{
Uri: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: fmt.Sprintf("/serving/%s/%s%s", namespace, serviceName, constants.PathBasedExplainPrefix()),
},
},
Authority: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.HostRegExp("my-domain.com"),
},
},
Gateways: []string{constants.KnativeIngressGateway},
},
},
Rewrite: &istiov1beta1.HTTPRewrite{
UriRegexRewrite: &istiov1beta1.RegexRewrite{
Match: fmt.Sprintf("/serving/%s/%s%s", namespace, serviceName, constants.PathBasedExplainPrefix()),
Rewrite: `\1`,
},
},
Route: []*istiov1beta1.HTTPRouteDestination{
createHTTPRouteDestination("knative-local-gateway.istio-system.svc.cluster.local"),
},
Headers: &istiov1beta1.Headers{
Request: &istiov1beta1.Headers_HeaderOperations{
Set: map[string]string{
"Host": network.GetServiceHostname(constants.ExplainerServiceName(serviceName), namespace),
},
},
},
},
{
Match: []*istiov1beta1.HTTPMatchRequest{
{
Uri: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Prefix{
Prefix: fmt.Sprintf("/serving/%s/%s/", namespace, serviceName),
},
},
Authority: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.HostRegExp("my-domain.com"),
},
},
Gateways: []string{constants.KnativeIngressGateway},
},
{
Uri: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Exact{
Exact: fmt.Sprintf("/serving/%s/%s", namespace, serviceName),
},
},
Authority: &istiov1beta1.StringMatch{
MatchType: &istiov1beta1.StringMatch_Regex{
Regex: constants.HostRegExp("my-domain.com"),
},
},
Gateways: []string{constants.KnativeIngressGateway},
},
},
Rewrite: &istiov1beta1.HTTPRewrite{
Uri: "/",
},
Route: []*istiov1beta1.HTTPRouteDestination{
createHTTPRouteDestination("knative-local-gateway.istio-system.svc.cluster.local"),
},
Headers: &istiov1beta1.Headers{
Request: &istiov1beta1.Headers_HeaderOperations{
Set: map[string]string{
"Host": network.GetServiceHostname(constants.PredictorServiceName(serviceName), namespace),
},
},
},
},
},
},
},
}, {
name: "found predictor status with default suffix",
ingressConfig: &v1beta1.IngressConfig{
4 changes: 2 additions & 2 deletions test/e2e/common/utils.py
View file
Original file line number Diff line number Diff line change
@@ -191,8 +191,8 @@ async def explain_response(client, service_name, input_path) -> Dict:
namespace=KSERVE_TEST_NAMESPACE,
version=constants.KSERVE_V1BETA1_VERSION,
)
cluster_ip, host, _ = get_isvc_endpoint(isvc)
url = f"http://{cluster_ip}"
cluster_ip, host, path = get_isvc_endpoint(isvc)
url = f"http://{cluster_ip}{path}"
headers = {"Host": host}
with open(input_path) as json_file:
data = json.load(json_file)
1 change: 1 addition & 0 deletions test/e2e/explainer/test_art_explainer.py
View file
Original file line number Diff line number Diff line change
@@ -35,6 +35,7 @@
kserve_client = KServeClient(config_file=os.environ.get("KUBECONFIG", "~/.kube/config"))


@pytest.mark.path_based_routing
@pytest.mark.explainer
@pytest.mark.asyncio(scope="session")
async def test_tabular_explainer(rest_v1_client):

0 comments on commit 536fc9b

Please sign in to comment.