Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change bookinfo gateway Port to 80 from 8080 #50693

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shweshi
Copy link

@shweshi shweshi commented Apr 26, 2024

Please provide a description of this PR:
The sample bookinfo example doesn't work with the istio-ingressgateway and bookinfo gateway because of port mismatch.

istio-ingressgateway.yaml

  - name: http2
    nodePort: 31156
    port: 80
    protocol: TCP
    targetPort: 80

bookinfo.gateway

    port:
      name: http
      number: 8080
      protocol: HTTP

This will cause error when accessing locally. (mac+minikube)

curl http://localhost:80/productpage
curl: (56) Recv failure: Connection reset by peer

Changing to port 80 works as expected.
bookinfo.gateway

    port:
      name: http
      number: 80
      protocol: HTTP
curl -s "http://localhost:80/productpage" | grep -o "<title>.*</title>"
<title>Simple Bookstore App</title>

This is causing issues while testing out the sample bookinfo and the documentation doesn't provide anything to fix the issue.

The sample bookinfo example doesn't work with the istio-ingressgateway and bookinfo gateway because of port mismatch.

istio-ingressgateway.yaml
```
  - name: http2
    nodePort: 31156
    port: 80
    protocol: TCP
    targetPort: 80
```

bookinfo.gateway
```
    port:
      name: http
      number: 8080
      protocol: HTTP
```

This will cause error when accessing locally. (mac+minikube)
curl http://localhost:80/productpage
curl: (56) Recv failure: Connection reset by peer

Changing to port 80 works as expected.
@istio-policy-bot
Copy link

😊 Welcome @shweshi! This is either your first contribution to the Istio istio repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

Copy link

linux-foundation-easycla bot commented Apr 26, 2024

CLA Signed


The committers listed above are authorized under a signed CLA.

@istio-testing istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test labels Apr 26, 2024
@istio-testing
Copy link
Collaborator

Hi @shweshi. Thanks for your PR.

I'm waiting for a istio 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 kubernetes/test-infra repository.

@shweshi
Copy link
Author

shweshi commented Apr 26, 2024

If this PR doens't get merge or meanwhile for people facing this issue:

  • Edit the bookinfo-gateway.yaml
kubectl edit gateway bookinfo-gateway -n bookinfo
  • Change the port from 8080 to 80 and save.

The changes will be applied, try the curl again and it should work.

curl -s "http://localhost:80/productpage" | grep -o "<title>.*</title>"
<title>Simple Bookstore App</title>

@wulianglongrd
Copy link
Member

#45943 cc @hanxiaop

@zirain
Copy link
Member

zirain commented Apr 26, 2024

I recall the targetPort of ingressgateway should be 8080, ingressgateway listen at 8080, and this example work as expected.

@shweshi
Copy link
Author

shweshi commented Apr 26, 2024

I recall the targetPort of ingressgateway should be 8080, ingressgateway listen at 8080, and this example work as expected.

@zirain targetPort is 80 when we install ingress-gateway from helm chart.

Screenshot 2024-04-26 165857

So either we need to make change in bookinfo gateway or in istio ingress gateway to have target port as 8080 to make the bookinfo apps works.

@dhawton
Copy link
Member

dhawton commented Apr 27, 2024

Changing this would reintroduce #45726 (comment). There is a mismatch in the configuration of the gateway depending on which chart is used (gateway vs gateways):

- port: 80
targetPort: 8080
name: http2
protocol: TCP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants