fixed iris sklearn model example

[karan6190]

Fixed the tunnel issue in iris sklearn model example, as no 80 port running inside the pod. Tunnel should be created on Istio ingress gateway service

This Pull Request is for fixing the example where there is issue in creating tunnel for executing the REST API from local

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[seldondev]

Hi @karan6190. Thanks for your PR.

I'm waiting for a SeldonIO or todo member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions 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 jenkins-x/lighthouse repository.

[karan6190]

/assign @SachinVarghese

[karan6190]

any update ?

[ukclivecox]

Thanks for this. I think this issue is historically because at some point istio ingress changed from pot 80 to 8080. All our other examples port-forward to the istio-ingress pod with port 8080. If we change this to use svc its out of step with the other examples. Maybe we could change to port 8080 which I assume works. A different PR could decide if SVC is better long term?

[RafalSkolasinski]

Would suggest to put recommended command there as

kubectl port-forward  -n istio-system svc/istio-ingressgateway 8003:80

this should work fine (I use it all the time) and seems to be clean and straighfroward.

[karan6190]

@cliveseldon sure that's make sense, I will alter the Pull request to create a tunnel via pod using 8080 port.

[karan6190]

@RafalSkolasinski thanks I will do. So basically Doc say we need to create a tunnel from pod not through svc which do confuse the users.

I will update the Pull request

[RafalSkolasinski]

Well, the whole spirit around it is that model URLs are in format:

http(s)://{ingress}/seldon/{namespace}/{sdep name}/...

and { ingress } can be your public IP (or DNS name) of istio / ambassador ingress. For demos / testing it is just convenient to assume that it is port-forwarded to localhost:8003 or localhost:8004 so then all of the example URLs takes format

http://localhost:8003/seldon/{namespace}/...

[karan6190]

@RafalSkolasinski yeah I understand that, but the users like me who are new to istio, got stuck in creating a tunnel.

[karan6190]

I have updated the pull request, please have a look

[RafalSkolasinski]

Yes, then the command of mine should be straightforward and work without issues, is it not?

I wonder why do you need to add --address 0.0.0.0? If I am not mistaken that will expose that port on your machine beyond your localhost?

[karan6190]

yes, your command will works fine as its doing a port fwd via svc.

--address I have used , in case user is working on Minikube and need to hit the model via local machine in that it will work else the port forwarding will be restricted to 127.0.0.0

[karan6190]

so make it generic should we remove --address field ??

[RafalSkolasinski]

--address I have used , in case user is working on Minikube and need to hit the model via local machine in that it will work else the port forwarding will be restricted to 127.0.0.0

Once the local kubectl is configured against the cluster inside minikube the port-forward command should have the same effect. At least that is what I remember, we haven't use Minikube in a long time as Kindis easier to handle and more lightweight.

Adding --address 0.0.0.0 potentially exposes your services to the outside world when one may not want it.

so make it generic should we remove --address field ??

Yes, please. I think it's better without it :)

[RafalSkolasinski]

LGTM, thanks for the contribution!

[seldondev]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RafalSkolasinski

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [RafalSkolasinski]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[karan6190]

--address I have used , in case user is working on Minikube and need to hit the model via local machine in that it will work else the port forwarding will be restricted to 127.0.0.0

Once the local kubectl is configured against the cluster inside minikube the port-forward command should have the same effect. At least that is what I remember, we haven't use Minikube in a long time as Kindis easier to handle and more lightweight.

Adding --address 0.0.0.0 potentially exposes your services to the outside world when one may not want it.

so make it generic should we remove --address field ??

Yes, please. I think it's better without it :)

Yeah totally make sense. lets keep it unique.

I have removed the --address field

[karan6190]

Hi Team, any idea how these changes can be merged to master ?

[ukclivecox]

@karan6190 Thanks!