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

Run e2e test in parallel #545

Merged
merged 5 commits into from
Aug 15, 2017
Merged

Conversation

chxchx
Copy link
Contributor

@chxchx chxchx commented Aug 8, 2017

This experimental PR aims to improve e2e framework to reduce execution time by running tests on mixer and bookinfo concurrently, and yet console logs are collected atomically without interleaving, preserving the abstraction of serial execution.

Empirical results have shown about 4-5 min reduction (25% improvement) in time elapsed.

Release note:

NONE

@chxchx
Copy link
Contributor Author

chxchx commented Aug 9, 2017

/assign @sebastienvas
/assign @yutongz
PTAL

@chxchx chxchx changed the title Run test in parallel Run e2e test in parallel Aug 9, 2017
Copy link
Contributor

@sebastienvas sebastienvas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL

@@ -106,8 +106,8 @@ func GetIngress(n string) (string, error) {
func GetIngressPod(n string) (string, error) {
retry := Retrier{
BaseDelay: 5 * time.Second,
MaxDelay: 30 * time.Minute,
Retries: 10,
MaxDelay: 20 * time.Minute,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it 5 minutes. We should not have to wait 20 mn for a test, that must be a mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

tests/e2e.sh Outdated
@@ -30,21 +30,46 @@ TESTS_TARGETS=($(bazel query 'tests(//tests/e2e/tests/...)'))
FAILURE_COUNT=0
SUMMARY='Tests Summary'

cd ${ROOT}
declare -A pid2testname
declare -A pid2logfile

for T in ${TESTS_TARGETS[@]}; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this configurable ie run the test sequentially by default and run in parallel when a flag is passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@chxchx
Copy link
Contributor Author

chxchx commented Aug 9, 2017

PTAL

;;
esac
# Filter -p out as it is not defined in the test framework
ARGS+=( ${!i} )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don t you need to have this inside the case ? I am not sure I understand that bash voodoo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is trying to say, append every flag to ARGS except for -p or --parallel. We can no longer use ${@} to have all command line flags to e2e.sh to fall through and apply to the test targets because the new -p flag is meant for the shell script only, and is not even defined in the go programs, so we have to filter it out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I did not see the continue.

@odeke-em
Copy link
Contributor

@chxchx you might need to rebase with master, also could you please squash your commits into one?

@chxchx
Copy link
Contributor Author

chxchx commented Aug 14, 2017

@odeke-em Thanks for the reminder. I would like to keep a complete commit history just so the reviewers do not have to read the entire PR again, but will definitely squash right before merging.

@odeke-em
Copy link
Contributor

Makes sense, thanks for letting me know @chxchx!

@sebastienvas
Copy link
Contributor

@chxchx you don't need to squash your commits. Github will do this for you.

@yutongz
Copy link
Contributor

yutongz commented Aug 15, 2017

@sebastienvas Would you like to approve again in Submit-queue way :)

@yutongz
Copy link
Contributor

yutongz commented Aug 15, 2017

Let's try to get this in first, and I will merge this to my pr and finish that.

@sebastienvas
Copy link
Contributor

/lgtm
/approve no-issue

@istio-merge-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chxchx, sebastienvas
We suggest the following additional approver: geeknoid

Assign the PR to them by writing /assign @geeknoid in a comment when ready.

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@chxchx
Copy link
Contributor Author

chxchx commented Aug 15, 2017

/assign @geeknoid
PTAL

@istio-merge-robot
Copy link

Automatic merge from submit-queue

@istio-merge-robot istio-merge-robot merged commit a6aa17e into istio:master Aug 15, 2017
rshriram pushed a commit that referenced this pull request Oct 30, 2017
Automatic merge from submit-queue

Run e2e test in parallel

This experimental PR aims to improve e2e framework to reduce execution time by running tests on mixer and bookinfo concurrently, and yet console logs are collected atomically without interleaving, preserving the abstraction of serial execution.

Empirical results have shown about 4-5 min reduction (25% improvement) in time elapsed. 

**Release note**:

```release-note
NONE
```

Former-commit-id: a6aa17e
vbatts pushed a commit to vbatts/istio that referenced this pull request Oct 31, 2017
Automatic merge from submit-queue

Run e2e test in parallel

This experimental PR aims to improve e2e framework to reduce execution time by running tests on mixer and bookinfo concurrently, and yet console logs are collected atomically without interleaving, preserving the abstraction of serial execution.

Empirical results have shown about 4-5 min reduction (25% improvement) in time elapsed. 

**Release note**:

```release-note
NONE
```

Former-commit-id: a6aa17e
mandarjog pushed a commit that referenced this pull request Oct 31, 2017
mandarjog pushed a commit that referenced this pull request Nov 2, 2017
Automatic merge from submit-queue

Run e2e test in parallel

This experimental PR aims to improve e2e framework to reduce execution time by running tests on mixer and bookinfo concurrently, and yet console logs are collected atomically without interleaving, preserving the abstraction of serial execution.

Empirical results have shown about 4-5 min reduction (25% improvement) in time elapsed. 

**Release note**:

```release-note
NONE
```

Former-commit-id: a6aa17e
rshriram added a commit to rshriram/istio that referenced this pull request Jul 31, 2018
* fix plurality of EnvoyFilter

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* more conditions
howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020
luksa pushed a commit to luksa/istio that referenced this pull request Sep 15, 2022
…ss (istio#255) (istio#545)

This is a reimplementation of the following for the Maistra 2.1 /
Istio 1.8 rebase:

 - MAISTRA-1723: Allow disabling NodePort gateway support (istio#180)
 - Ensure Pilot can run without privileges for reading nodes (istio#125)

A new istiod flag is added, `disableNodeAccess`, that will disable any
feature relying on access to Node objects.

Co-authored-by: Brad Ison <brad.ison@redhat.com>
luksa pushed a commit to luksa/istio that referenced this pull request Aug 16, 2023
…ss (istio#255) (istio#545)

This is a reimplementation of the following for the Maistra 2.1 /
Istio 1.8 rebase:

 - MAISTRA-1723: Allow disabling NodePort gateway support (istio#180)
 - Ensure Pilot can run without privileges for reading nodes (istio#125)

A new istiod flag is added, `disableNodeAccess`, that will disable any
feature relying on access to Node objects.

Co-authored-by: Brad Ison <brad.ison@redhat.com>
dgn pushed a commit to dgn/istio that referenced this pull request Jun 13, 2024
…ss (istio#255) (istio#545)

This is a reimplementation of the following for the Maistra 2.1 /
Istio 1.8 rebase:

 - MAISTRA-1723: Allow disabling NodePort gateway support (istio#180)
 - Ensure Pilot can run without privileges for reading nodes (istio#125)

A new istiod flag is added, `disableNodeAccess`, that will disable any
feature relying on access to Node objects.

--------

Co-authored-by: Brad Ison <brad.ison@redhat.com>
Co-authored-by: Jacek Ewertowski <jewertow@redhat.com>
Signed-off-by: Yann Liu <yannliu@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants