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.

Sign up for GitHub

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

Jump to bottom

Enable MCP by default in helm values and e2e tests #10222

Merged
merged 5 commits into from
Dec 3, 2018
Merged

Enable MCP by default in helm values and e2e tests #10222

merged 5 commits into from
Dec 3, 2018

Conversation

ayj
Copy link
Contributor

@ayj ayj commented Nov 30, 2018

This PR inverts the default helm options so that MCP is enabled by default. For the time being users can still disable MCP to compare performance and functional behavior with MCP.

The MCP and non-MCP e2e tests have also been inverted so that the full test suite now runs with MCP enabled. A subset of e2e tests are run w/o MCP as a reference for flakiness.

@ayj ayj requested review from geeknoid and hklai November 30, 2018 23:23
@ayj
Copy link
Contributor Author

ayj commented Nov 30, 2018

We'll also need the following changes:

  1. update test/infra to account for the revised e2e test names
  2. updates k8s testgrid to account for the revised e2e test names
  3. update circleci so that the galley and non-mcp tests are mandatory.

ayj added a commit to ayj/test-infra that referenced this pull request Dec 1, 2018
@ayj
update prow to use the new non-mcp test names
ce1080f 
See istio/istio#10222 for the corresponding
istio/istio change.
@ayj ayj mentioned this pull request Dec 1, 2018
Merged
@ayj
Copy link
Contributor Author

ayj commented Dec 1, 2018

cc @hklai, all tests are green now. Once this is merged, we can merge istio/test-infra#1077 and then create a follow-up PR in istio/istio to remove the *-mcp.sh variants.

@rshriram
Copy link
Member

rshriram commented Dec 3, 2018 •
edited
Loading

update circleci so that the galley and non-mcp tests are mandatory.

why do we need to run 2x in circleci? can't we simply test only with mcp?

@hklai
Copy link
Contributor

hklai commented Dec 3, 2018

why do we need to run 2x in circleci? can't we simply test only with mcp?

How about we run non mcp in circle periodic only?

@ayj
Copy link
Contributor Author

ayj commented Dec 3, 2018

How about we run non mcp in circle periodic only?

Done - circleci non-MCP tests are only run periodically. The prow tests will be updated in a follow-up test-infra PR

hklai
hklai approved these changes Dec 3, 2018
View reviewed changes
Copy link
Contributor

@hklai hklai left a comment

Choose a reason for hiding this comment

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

/lgtm

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ayj, hklai

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:

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

@istio-testing istio-testing merged commit 970e079 into istio:release-1.1 Dec 3, 2018
@istio-testing
Copy link
Collaborator

@ayj: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow//e2e-simpleTests-minProfile.sh 2a1f7ce link /test e2e-simpleTestsMinProfile
prow/istio-pilot-multicluster-e2e.sh 2a1f7ce link /test istio-pilot-multicluster-e2e

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. I understand the commands that are listed here.

ayj added a commit to ayj/istio that referenced this pull request Dec 3, 2018
@ayj
Enable MCP by default in helm values and e2e tests (istio#10222)
c56cd02 
* enable mcp by default in test framework and top-level help charts

* invert mcp and non-mcp circleci tests

* invert prow mcp and non-mcp tests

* temporarily add mcp specific tests back while we transition prow to use the non-mcp variants

* remove non-mcp circleci tests from presubmit

(cherry picked from commit 970e079)
@ayj ayj deleted the enable-mcp-by-default branch December 3, 2018 23:05
istio-testing pushed a commit to istio/test-infra that referenced this pull request Dec 4, 2018
@ayj @istio-testing
Remove mcp specific test (#1077)
c1cfb6d 
* update prow to use the new non-mcp test names

See istio/istio#10222 for the corresponding
istio/istio change.

* update mandatory circleci tests

* remove non-mcp prow tests from presubmit

* remove non-mcp circle tests from mandatory status checks
hklai added a commit that referenced this pull request Dec 4, 2018
@hklai
Merge pull request #10256 from ayj/cherry-pick-10222
33f661e 
Enable MCP by default in helm values and e2e tests (#10222)
@ayj ayj mentioned this pull request Feb 19, 2019
Closed
@hzxuzhonghu hzxuzhonghu mentioned this pull request Feb 20, 2019
Closed
ChristinaLyu0710-zz pushed a commit to ChristinaLyu0710-zz/istio-flakey-test that referenced this pull request Jun 11, 2019
@ayj @clyu0710
Remove mcp specific test (#1077)
1084e45 
* update prow to use the new non-mcp test names

See istio/istio#10222 for the corresponding
istio/istio change.

* update mandatory circleci tests

* remove non-mcp prow tests from presubmit

* remove non-mcp circle tests from mandatory status checks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rshriram rshriram rshriram approved these changes

@hklai hklai hklai approved these changes

@geeknoid geeknoid Awaiting requested review from geeknoid

@costinm costinm Awaiting requested review from costinm

@morvencao morvencao Awaiting requested review from morvencao

Assignees

@hklai hklai

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ayj @rshriram @hklai @istio-testing @googlebot