Adds ingress2 to perf_script and updates docs/output#3527
Adds ingress2 to perf_script and updates docs/output#3527danehans wants to merge 4 commits intoistio:masterfrom
Conversation
|
Hi @danehans. 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 I understand the commands that are listed here. DetailsInstructions 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. I understand the commands that are listed here. |
|
/cc @ldemailly |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:You can indicate your approval by writing |
| Execute kubectl -n $FORTIO_NAMESPACE expose deployment fortio1 --target-port=8080 --type=LoadBalancer | ||
| Execute kubectl -n $FORTIO_NAMESPACE run fortio2 --image=istio/fortio --port=8080 | ||
| Execute kubectl -n $FORTIO_NAMESPACE expose deployment fortio2 --target-port=8080 | ||
| Execute kubectl -n $FORTIO_NAMESPACE expose deployment fortio2 --target-port=8080 --type=LoadBalancer |
There was a problem hiding this comment.
I'm trying to remember why I didn't put both as load balancer, there was a reason (maybe to compare perf/setup) but I forgot :/
technically for the fortio reference we don't really need direct access to the second service as it can be proxied by the first one
There was a problem hiding this comment.
The gce ingress controller requires a service to be exposed using type LoadBalancer or type NodePort. This is why you could not get the fortio2 ingress to work.
There was a problem hiding this comment.
not sure we need to expose fortio2 through loadbalancer, it's going to consume another external ip
There was a problem hiding this comment.
If we want to create an ingress for fortio2, then we need the fortio2 service to be of type LoadBalancer or type NodePort. This is a requirement of the gce ingress controller. I saw the note # Doesn't work somehow... in the script and came up with this fix.
tools/setup_perf_cluster.sh
Outdated
| # Doesn't work somehow... | ||
| function setup_non_istio_ingress2() { | ||
| cat <<_EOF_ | kubectl apply -n fortio -f - | ||
| function get_fortio2_k8s_ip() { |
There was a problem hiding this comment.
make 1 or 2 a parameter to the function, it's the exact same code
| rules: | ||
| - http: | ||
| paths: | ||
| - path: /fortio1 |
There was a problem hiding this comment.
ah I see what I wanted here ... I was trying to use an (istio like) ingress spec to get the same setup
it'd be nice to make it work rather than just drop it ?
There was a problem hiding this comment.
OK. I was thinking that the ingress spec for fortio1 and fortio2 should be identical. Since the fortio1 spec worked, I used that as the reference.
Can we refactor the ing specs in a future PR?
There was a problem hiding this comment.
this one is supposed to be fortio1 and doesn't work in term of prefix - but yes I guess ingress setup we can change later
There was a problem hiding this comment.
Take a look at the entire file, specifically here. I did some copy/paste between the ingress specs that makes the review changes look weird.
|
@danehans PR needs rebase |
|
Ping ? Is it still needed ? |
|
@costinm that's up to @ldemailly If not, feel free to close the PR. |
|
This pull request has been automatically marked as stale because it has not had activity in the last 2 weeks. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
I changed the diagram since. let’s close this and update as needed |
Previously, the
setup_perf_cluster.shscript deployed a single non-Istio Ingress and tools/README.md contained doc bugs. This PR adds another ingress for thefortio2pod and updates/fixes tools/README.md.