Conversation
chxchx
left a comment
There was a problem hiding this comment.
I guess the e2e smoke has been running all 3 tests for a while instead of just bookinfo and the test infra seems to handle the load well so lets keep it that way.
How does make init differ form make depend? Do you see any changes that need to be made to https://github.com/istio-releases/daily-release/blob/master/prow/daily-release-qualification.sh ?
|
@mattdelco PR needs rebase |
|
It would be possible to still run just bookinfo by having the script use a make target of "e2e_bookinfo" instead of "e2e_all". I removed the "-s bookinfo" mostly because it's currently ignored and was causing problems elsewhere as an unrecognized parameter (this might have been when I first tried to place E2E_ARGS="$E2E_ARGS" as an input parameter to make [apparently make isn't so great about handling parameters with space, even when quoted]). Currently "make depend" is just an alias for "make init", which in turn is mostly an alias for "bin/init.sh". I have a much larger PR where the direct callers of bin/init.sh are complicating things so I'm trying to get rid of the direct callers. I would have just eliminated the call to "bin/init.sh" (like I did elsewhere) but prow/istio-pilot-e2e.sh is calling pilot's Makefile and that file doesn't have all its dependencies set up properly (which my larger PR addresses, in part by deleting pilot's Makefile). |
|
/test istio-presubmit |
|
/lgtm cancel //PR changed after LGTM, removing LGTM. @chxchx @mattdelco |
|
/test istio-pilot-e2e |
|
/lgtm |
|
/assign @sebastienvas |
|
/test istio-pilot-e2e |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chxchx, sebastienvas 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 |
Some parts of the test seem at best sub-optimal. This change does the following: