Skip to content

Commit

Permalink
added a flag to make istio optional (kubeflow#2380)
Browse files Browse the repository at this point in the history 
Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>

Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>
  • Loading branch information
@Suresh-Nakkeran
Suresh-Nakkeran authored Sep 9, 2022
1 parent cede4f4 commit 48f45eb
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 45 deletions.
18 changes: 12 additions & 6 deletions cmd/manager/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,23 @@ func main() {
log.Error(err, "unable to get deploy config.")
os.Exit(1)
}
ingressConfig, err := v1beta1.NewIngressConfig(client)
if err != nil {
log.Error(err, "unable to get ingress config.")
os.Exit(1)
}
if deployConfig.DefaultDeploymentMode == string(constants.Serverless) {
log.Info("Setting up Knative scheme")
if err := knservingv1.AddToScheme(mgr.GetScheme()); err != nil {
log.Error(err, "unable to add Knative APIs to scheme")
os.Exit(1)
}

log.Info("Setting up Istio schemes")
if err := v1alpha3.AddToScheme(mgr.GetScheme()); err != nil {
log.Error(err, "unable to add Istio v1alpha3 APIs to scheme")
os.Exit(1)
if ingressConfig.DisableIstioVirtualHost == false {
log.Info("Setting up Istio schemes")
if err := v1alpha3.AddToScheme(mgr.GetScheme()); err != nil {
log.Error(err, "unable to add Istio v1alpha3 APIs to scheme")
os.Exit(1)
}
}
}

Expand All @@ -152,7 +158,7 @@ func main() {
Scheme: mgr.GetScheme(),
Recorder: eventBroadcaster.NewRecorder(
mgr.GetScheme(), v1.EventSource{Component: "v1beta1Controllers"}),
}).SetupWithManager(mgr, deployConfig); err != nil {
}).SetupWithManager(mgr, deployConfig, ingressConfig.DisableIstioVirtualHost); err != nil {
setupLog.Error(err, "unable to create controller", "v1beta1Controller", "InferenceService")
os.Exit(1)
}
Expand Down
3 changes: 2 additions & 1 deletion config/configmap/inferenceservice.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ data:
"ingressDomain" : "example.com",
"ingressClassName" : "istio",
"domainTemplate": "{{ .Name }}-{{ .Namespace }}.{{ .IngressDomain }}",
"urlScheme": "http"
"urlScheme": "http",
"disableIstioVirtualHost": false
}
logger: |-
{
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/serving/v1beta1/configmap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ type IngressConfig struct {
IngressClassName *string `json:"ingressClassName,omitempty"`
DomainTemplate string `json:"domainTemplate,omitempty"`
UrlScheme string `json:"urlScheme,omitempty"`
DisableIstioVirtualHost bool `json:"disableIstioVirtualHost,omitempty"`
}

// +kubebuilder:object:generate=false
Expand Down
12 changes: 9 additions & 3 deletions pkg/controller/v1beta1/inferenceservice/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func (r *InferenceServiceReconciler) Reconcile(ctx context.Context, req ctrl.Req
} else {
reconciler := ingress.NewIngressReconciler(r.Client, r.Scheme, ingressConfig)
r.Log.Info("Reconciling ingress for inference service", "isvc", isvc.Name)
if err := reconciler.Reconcile(isvc); err != nil {
if err := reconciler.Reconcile(isvc, ingressConfig.DisableIstioVirtualHost); err != nil {
return reconcile.Result{}, errors.Wrapf(err, "fails to reconcile ingress")
}
}
Expand Down Expand Up @@ -274,19 +274,25 @@ func inferenceServiceStatusEqual(s1, s2 v1beta1api.InferenceServiceStatus, deplo
return equality.Semantic.DeepEqual(s1, s2)
}

func (r *InferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager, deployConfig *v1beta1api.DeployConfig) error {
func (r *InferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager, deployConfig *v1beta1api.DeployConfig, disableIstioVirtualHost bool) error {
if deployConfig.DefaultDeploymentMode == string(constants.RawDeployment) {
return ctrl.NewControllerManagedBy(mgr).
For(&v1beta1api.InferenceService{}).
Owns(&appsv1.Deployment{}).
Complete(r)
} else {
} else if disableIstioVirtualHost == false {
return ctrl.NewControllerManagedBy(mgr).
For(&v1beta1api.InferenceService{}).
Owns(&knservingv1.Service{}).
Owns(&v1alpha3.VirtualService{}).
Owns(&appsv1.Deployment{}).
Complete(r)
} else {
return ctrl.NewControllerManagedBy(mgr).
For(&v1beta1api.InferenceService{}).
Owns(&knservingv1.Service{}).
Owns(&appsv1.Deployment{}).
Complete(r)
}

}
Expand Down
109 changes: 109 additions & 0 deletions pkg/controller/v1beta1/inferenceservice/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1653,4 +1653,113 @@ var _ = Describe("v1beta1 inference service controller", func() {
})
})

Context("When creating inference service with predictor and without top level istio virtual service", func() {
It("Should have knative service created", func() {
By("By creating a new InferenceService")
// Create configmap
copiedConfigs := make(map[string]string)
for key, value := range configs {
if key == "ingress" {
copiedConfigs[key] = `{
"disableIstioVirtualHost": true,
"ingressGateway": "knative-serving/knative-ingress-gateway",
"ingressService": "test-destination",
"localGateway": "knative-serving/knative-local-gateway",
"localGatewayService": "knative-local-gateway.istio-system.svc.cluster.local"
}`
} else {
copiedConfigs[key] = value
}
}
var configMap = &v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: constants.InferenceServiceConfigMapName,
Namespace: constants.KServeNamespace,
},
Data: copiedConfigs,
}
Expect(k8sClient.Create(context.TODO(), configMap)).NotTo(HaveOccurred())
defer k8sClient.Delete(context.TODO(), configMap)
serviceName := "foo-disable-istio"
var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: serviceName, Namespace: "default"}}
var serviceKey = expectedRequest.NamespacedName
var storageUri = "s3://test/mnist/export"
ctx := context.Background()
isvc := &v1beta1.InferenceService{
ObjectMeta: metav1.ObjectMeta{
Name: serviceKey.Name,
Namespace: serviceKey.Namespace,
},
Spec: v1beta1.InferenceServiceSpec{
Predictor: v1beta1.PredictorSpec{
ComponentExtensionSpec: v1beta1.ComponentExtensionSpec{
MinReplicas: v1beta1.GetIntReference(1),
MaxReplicas: 3,
},
Tensorflow: &v1beta1.TFServingSpec{
PredictorExtensionSpec: v1beta1.PredictorExtensionSpec{
StorageURI: &storageUri,
RuntimeVersion: proto.String("1.14.0"),
Container: v1.Container{
Name: "kfs",
Resources: defaultResource,
},
},
},
},
},
}
Expect(k8sClient.Create(ctx, isvc)).Should(Succeed())
defer k8sClient.Delete(ctx, isvc)
inferenceService := &v1beta1.InferenceService{}

Eventually(func() bool {
err := k8sClient.Get(ctx, serviceKey, inferenceService)
if err != nil {
return false
}
return true
}, timeout, interval).Should(BeTrue())

actualService := &knservingv1.Service{}
predictorServiceKey := types.NamespacedName{Name: constants.DefaultPredictorServiceName(serviceKey.Name),
Namespace: serviceKey.Namespace}
Eventually(func() error {
return k8sClient.Get(context.TODO(), predictorServiceKey, actualService)
}, timeout).Should(Succeed())
predictorUrl, _ := apis.ParseURL("http://" + constants.InferenceServiceHostName(constants.DefaultPredictorServiceName(serviceKey.Name), serviceKey.Namespace, domain))
// update predictor
updatedService := actualService.DeepCopy()
updatedService.Status.LatestCreatedRevisionName = "revision-v1"
updatedService.Status.LatestReadyRevisionName = "revision-v1"
updatedService.Status.URL = predictorUrl
updatedService.Status.Conditions = duckv1.Conditions{
{
Type: knservingv1.ServiceConditionReady,
Status: "True",
},
}
Expect(k8sClient.Status().Update(context.TODO(), updatedService)).NotTo(gomega.HaveOccurred())
// get inference service
time.Sleep(10 * time.Second)
actualIsvc := &v1beta1.InferenceService{}
Eventually(func() bool {
err := k8sClient.Get(ctx, expectedRequest.NamespacedName, actualIsvc)
if err != nil {
return false
}
return true
}, timeout, interval).Should(BeTrue())

Expect(actualIsvc.Status.URL).To(gomega.Equal(&apis.URL{
Scheme: "http",
Host: constants.InferenceServiceHostName(constants.DefaultPredictorServiceName(serviceKey.Name), serviceKey.Namespace, domain),
}))
Expect(actualIsvc.Status.Address.URL).To(gomega.Equal(&apis.URL{
Scheme: "http",
Host: network.GetServiceHostname(fmt.Sprintf("%s-%s-default", serviceKey.Name, string(constants.Predictor)), serviceKey.Namespace),
Path: constants.PredictPath(serviceKey.Name, constants.ProtocolV1),
}))
})
})
})
90 changes: 56 additions & 34 deletions pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/ingress_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func getServiceHost(isvc *v1beta1.InferenceService) string {
}
}

func getServiceUrl(isvc *v1beta1.InferenceService, urlScheme string) string {
func getServiceUrl(isvc *v1beta1.InferenceService, urlScheme string, disableIstioVirtualHost bool) string {
if isvc.Status.Components == nil {
return ""
}
Expand All @@ -102,7 +102,11 @@ func getServiceUrl(isvc *v1beta1.InferenceService, urlScheme string) string {
} else {
url := transformerStatus.URL
url.Scheme = urlScheme
return strings.Replace(url.String(), fmt.Sprintf("-%s-default", string(constants.Transformer)), "", 1)
if disableIstioVirtualHost == false {
return strings.Replace(url.String(), fmt.Sprintf("-%s-default", string(constants.Transformer)), "", 1)
} else {
return url.String()
}
}
}

Expand All @@ -113,7 +117,11 @@ func getServiceUrl(isvc *v1beta1.InferenceService, urlScheme string) string {
} else {
url := predictorStatus.URL
url.Scheme = urlScheme
return strings.Replace(url.String(), fmt.Sprintf("-%s-default", string(constants.Predictor)), "", 1)
if disableIstioVirtualHost == false {
return strings.Replace(url.String(), fmt.Sprintf("-%s-default", string(constants.Predictor)), "", 1)
} else {
return url.String()
}
}
}

Expand Down Expand Up @@ -323,46 +331,49 @@ func createIngress(isvc *v1beta1.InferenceService, config *v1beta1.IngressConfig
return desiredIngress
}

func (ir *IngressReconciler) Reconcile(isvc *v1beta1.InferenceService) error {
func (ir *IngressReconciler) Reconcile(isvc *v1beta1.InferenceService, disableIstioVirtualHost bool) error {
serviceHost := getServiceHost(isvc)
serviceUrl := getServiceUrl(isvc, ir.ingressConfig.UrlScheme)
serviceUrl := getServiceUrl(isvc, ir.ingressConfig.UrlScheme, disableIstioVirtualHost)
if serviceHost == "" || serviceUrl == "" {
return nil
}
//Create ingress
desiredIngress := createIngress(isvc, ir.ingressConfig)
if desiredIngress == nil {
return nil
}
// When Istio virtual host is disabled, we return the underlying component url.
// When Istio virtual host is enabled. we return the url using inference service virtual host name and redirect to the corresponding transformer, predictor or explainer url.
if disableIstioVirtualHost == false {
desiredIngress := createIngress(isvc, ir.ingressConfig)
if desiredIngress == nil {
return nil
}

//Create external service which points to local gateway
if err := ir.reconcileExternalService(isvc, ir.ingressConfig); err != nil {
return errors.Wrapf(err, "fails to reconcile external name service")
}
//Create external service which points to local gateway
if err := ir.reconcileExternalService(isvc, ir.ingressConfig); err != nil {
return errors.Wrapf(err, "fails to reconcile external name service")
}

if err := controllerutil.SetControllerReference(isvc, desiredIngress, ir.scheme); err != nil {
return errors.Wrapf(err, "fails to set owner reference for ingress")
}
if err := controllerutil.SetControllerReference(isvc, desiredIngress, ir.scheme); err != nil {
return errors.Wrapf(err, "fails to set owner reference for ingress")
}

existing := &v1alpha3.VirtualService{}
err := ir.client.Get(context.TODO(), types.NamespacedName{Name: desiredIngress.Name, Namespace: desiredIngress.Namespace}, existing)
if err != nil {
if apierr.IsNotFound(err) {
log.Info("Creating Ingress for isvc", "namespace", desiredIngress.Namespace, "name", desiredIngress.Name)
err = ir.client.Create(context.TODO(), desiredIngress)
existing := &v1alpha3.VirtualService{}
err := ir.client.Get(context.TODO(), types.NamespacedName{Name: desiredIngress.Name, Namespace: desiredIngress.Namespace}, existing)
if err != nil {
if apierr.IsNotFound(err) {
log.Info("Creating Ingress for isvc", "namespace", desiredIngress.Namespace, "name", desiredIngress.Name)
err = ir.client.Create(context.TODO(), desiredIngress)
}
} else {
if !routeSemanticEquals(desiredIngress, existing) {
existing.Spec = desiredIngress.Spec
existing.Annotations = desiredIngress.Annotations
existing.Labels = desiredIngress.Labels
log.Info("Update Ingress for isvc", "namespace", desiredIngress.Namespace, "name", desiredIngress.Name)
err = ir.client.Update(context.TODO(), existing)
}
}
} else {
if !routeSemanticEquals(desiredIngress, existing) {
existing.Spec = desiredIngress.Spec
existing.Annotations = desiredIngress.Annotations
existing.Labels = desiredIngress.Labels
log.Info("Update Ingress for isvc", "namespace", desiredIngress.Namespace, "name", desiredIngress.Name)
err = ir.client.Update(context.TODO(), existing)
if err != nil {
return errors.Wrapf(err, "fails to create or update ingress")
}
}
if err != nil {
return errors.Wrapf(err, "fails to create or update ingress")
}

if url, err := apis.ParseURL(serviceUrl); err == nil {
isvc.Status.URL = url
Expand All @@ -379,9 +390,10 @@ func (ir *IngressReconciler) Reconcile(isvc *v1beta1.InferenceService) error {
path = constants.PredictPath(isvc.Name, constants.ProtocolV2)
}
}
hostPrefix := getHostPrefix(isvc, disableIstioVirtualHost)
isvc.Status.Address = &duckv1.Addressable{
URL: &apis.URL{
Host: network.GetServiceHostname(isvc.Name, isvc.Namespace),
Host: network.GetServiceHostname(hostPrefix, isvc.Namespace),
Scheme: "http",
Path: path,
},
Expand All @@ -401,3 +413,13 @@ func routeSemanticEquals(desired, existing *v1alpha3.VirtualService) bool {
equality.Semantic.DeepEqual(desired.ObjectMeta.Labels, existing.ObjectMeta.Labels) &&
equality.Semantic.DeepEqual(desired.ObjectMeta.Annotations, existing.ObjectMeta.Annotations)
}

func getHostPrefix(isvc *v1beta1.InferenceService, disableIstioVirtualHost bool) string {
if disableIstioVirtualHost == true {
if isvc.Spec.Transformer != nil {
return constants.DefaultTransformerServiceName(isvc.Name)
}
return constants.DefaultPredictorServiceName(isvc.Name)
}
return isvc.Name
}
2 changes: 1 addition & 1 deletion pkg/controller/v1beta1/inferenceservice/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ var _ = BeforeSuite(func() {
Scheme: k8sClient.Scheme(),
Log: ctrl.Log.WithName("V1beta1InferenceServiceController"),
Recorder: k8sManager.GetEventRecorderFor("V1beta1InferenceServiceController"),
}).SetupWithManager(k8sManager, deployConfig)
}).SetupWithManager(k8sManager, deployConfig, false)
Expect(err).ToNot(HaveOccurred())

go func() {
Expand Down

0 comments on commit 48f45eb

Please sign in to comment.