Skip to content

Bring the install in line with current pilot and istioctl. #749

Merged
douglas-reid merged 12 commits intomasterfrom
costin-install
Sep 13, 2017
Merged

Bring the install in line with current pilot and istioctl. #749
douglas-reid merged 12 commits intomasterfrom
costin-install

Conversation

@costinm
Copy link
Copy Markdown
Contributor

@costinm costinm commented Sep 12, 2017

This is needed if the new istioctl is used.

Unfortunately the change is not backward or forward compatible - IMHO the right
solution is to fix istioctl and config loading or revert the change - but this PR will get
the istio/istio work again with current istioctl.

NONE

kyessenov
kyessenov requested changes Sep 12, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@kyessenov kyessenov left a comment

Choose a reason for hiding this comment

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

Run updateVersion. Why is this out of sync with installation files?

@costinm
Copy link
Copy Markdown
Contributor Author

costinm commented Sep 12, 2017

/retest

@ldemailly
Copy link
Copy Markdown
Member

ldemailly commented Sep 12, 2017

verified c09e19c7205fa36b7e8e9d9966cff08b84fe8f2f is recent pilot
/lgtm
but I think the routing rules are broken so it won't pass/land with that pilot/mixer combo

@costinm
Copy link
Copy Markdown
Contributor Author

costinm commented Sep 12, 2017

How do you suggest we proceed ? Force it ? Wait until we fix the rules ?

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Sep 12, 2017

Deflake the test. Route rules seem to work locally, so it's either a flake or something is in the cluster.

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Sep 12, 2017

No forcing please.

@ldemailly ldemailly mentioned this pull request Sep 12, 2017
Closed
@sebastienvas sebastienvas mentioned this pull request Sep 12, 2017
Closed
@sebastienvas sebastienvas mentioned this pull request Sep 12, 2017
Closed
@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Sep 12, 2017

/lgtm cancel //PR changed after LGTM, removing LGTM. @costinm @ldemailly

@costinm
Copy link
Copy Markdown
Contributor Author

costinm commented Sep 12, 2017

/retest

@costinm
Copy link
Copy Markdown
Contributor Author

costinm commented Sep 13, 2017

/test

@rshriram
Copy link
Copy Markdown
Member

rshriram commented Sep 13, 2017

Please wait. I am almost done with my PR. This one has no docs, no comments on the variable values..

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Sep 13, 2017

@rshriram this is just to bring sanity. improvements to defaults come later.

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Sep 13, 2017

Passed locally at SHA fe18026

@andraxylia
Copy link
Copy Markdown
Contributor

andraxylia commented Sep 13, 2017

Updated the branch to pick up RBAC fixes.

@douglas-reid
Copy link
Copy Markdown
Contributor

douglas-reid commented Sep 13, 2017

/lgtm

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Sep 13, 2017

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: douglas-reid, ldemailly

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

Details Needs approval from an approver in each of these OWNERS Files:
  • OWNERS [douglas-reid,ldemailly]

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

@douglas-reid
Copy link
Copy Markdown
Contributor

douglas-reid commented Sep 13, 2017 •
edited
Loading

this passes on my local cluster with rbac (commit: fe18026):

$ tests/e2e.sh --rbac_path=install/kubernetes/istio-rbac-beta.yaml --auth_enable
...
****************************************************
Tests Summary
PASSED: //tests/e2e/tests/mixer:go_default_test
PASSED: //tests/e2e/tests/bookinfo:go_default_test

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Sep 13, 2017

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

Test name Commit Details Rerun command
prow/new-e2e-rbac_no_auth.sh 030c91f link /test new-e2e-rbac_no_auth
prow/e2e-suite-rbac-auth.sh 030c91f link /test e2e-suite-rbac-auth
prow/e2e-suite-rbac-no_auth.sh 030c91f link /test e2e-suite-rbac-no_auth
Details

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.

@douglas-reid douglas-reid merged commit c4852e7 into master Sep 13, 2017
@ldemailly
Copy link
Copy Markdown
Member

ldemailly commented Sep 13, 2017

we merged this despite rbac errors because it's better anyway and the rbac issue is solved or tracked in ... ?

@andraxylia
Copy link
Copy Markdown
Contributor

andraxylia commented Sep 13, 2017

#769 should fix all rbac errors.

@costinm costinm deleted the costin-install branch September 14, 2017 20:42
mandarjog pushed a commit to mandarjog/istio that referenced this pull request Oct 30, 2017
@sebastienvas @douglas-reid
Publish grafana images as part of testing (istio#749)
0bdbd8f 
Former-commit-id: 01212aca9b9a23ed85f23548dc2992268ddec9e9
rshriram pushed a commit that referenced this pull request Oct 30, 2017
@costinm @douglas-reid
Bring the install in line with current pilot and istioctl. (#749)
df77eef 
* Fix pilot config to match the current format

* Rollback change to updateVersion

* Update version

* Update istioctl to latest version (from PR753)

* Update pilot as well to latest version (from PR753)

* More recent pilot, maybe it works this time

* rename to destination

* update yamls


Former-commit-id: c4852e7
mandarjog pushed a commit that referenced this pull request Oct 31, 2017
@sebastienvas @douglas-reid
Publish grafana images as part of testing (#749)
aebcb81 
Former-commit-id: b5aea97c6396ad34d092c53262ba3b5ef898469e
vbatts pushed a commit to vbatts/istio that referenced this pull request Oct 31, 2017
@costinm @douglas-reid
Bring the install in line with current pilot and istioctl. (istio#749)
3adf7a9 
* Fix pilot config to match the current format

* Rollback change to updateVersion

* Update version

* Update istioctl to latest version (from PR753)

* Update pilot as well to latest version (from PR753)

* More recent pilot, maybe it works this time

* rename to destination

* update yamls


Former-commit-id: c4852e7
mandarjog pushed a commit that referenced this pull request Oct 31, 2017
@kyessenov @rshriram
Update to Envoy with zipkin (#749)
c28339f
mandarjog pushed a commit that referenced this pull request Nov 2, 2017
@costinm @douglas-reid
Bring the install in line with current pilot and istioctl. (#749)
a25f530 
* Fix pilot config to match the current format

* Rollback change to updateVersion

* Update version

* Update istioctl to latest version (from PR753)

* Update pilot as well to latest version (from PR753)

* More recent pilot, maybe it works this time

* rename to destination

* update yamls


Former-commit-id: c4852e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kyessenov kyessenov kyessenov approved these changes

@rshriram rshriram Awaiting requested review from rshriram

@andraxylia andraxylia Awaiting requested review from andraxylia

Assignees

@ldemailly ldemailly

@douglas-reid douglas-reid

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@costinm @ldemailly @kyessenov @istio-merge-robot @rshriram @andraxylia @douglas-reid @istio-testing @googlebot