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

Add istio addon for minikube #6154

Merged
merged 1 commit into from
Dec 27, 2019
Merged

Add istio addon for minikube #6154

merged 1 commit into from
Dec 27, 2019

Conversation

fenglixa
Copy link
Contributor

@fenglixa fenglixa commented Dec 23, 2019

fixes #3597

Usage and UT as below:
image

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 23, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fenglixa
To complete the pull request process, please assign medyagh
You can assign the PR to them by writing /assign @medyagh 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 files:

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

@fenglixa
Copy link
Contributor Author

/cc @priyawadhwa

@codecov-io
Copy link

codecov-io commented Dec 23, 2019

Codecov Report

Merging #6154 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6154      +/-   ##
==========================================
- Coverage   38.38%   38.35%   -0.04%     
==========================================
  Files         120      120              
  Lines        8096     8103       +7     
==========================================
  Hits         3108     3108              
- Misses       4585     4591       +6     
- Partials      403      404       +1
Impacted Files Coverage Δ
pkg/minikube/assets/addons.go 36.66% <ø> (ø) ⬆️
cmd/minikube/cmd/config/config.go 10.41% <ø> (ø) ⬆️
cmd/minikube/cmd/config/util.go 32.63% <0%> (-1.67%) ⬇️

@medyagh
Copy link
Member

medyagh commented Dec 23, 2019

Does the istio addon works with default 2000mb memory?

@medyagh
Copy link
Member

medyagh commented Dec 23, 2019

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Dec 23, 2019
@minikube-bot
Copy link
Collaborator

Error: running mkcmp: exit status 1

@fenglixa
Copy link
Contributor Author

Does the istio addon works with default 2000mb memory?

Thanks @medyagh
Yes, it works under my testing for default minikube assigned mem/cpus, the istio profile I used is default, which is not fully istio components installed (e.g grafana is not installed with default profile)
for detail istio profile defination, could be refer to istio docs here:
https://istio.io/docs/setup/additional-setup/config-profiles/

@medyagh
Copy link
Member

medyagh commented Dec 23, 2019

@fenglixa it would be usefull to warn the user that this addon needs more memory !
( in the screenshot I see a few of the pods are erroring, ... how did u test functionality of it )

@fenglixa
Copy link
Contributor Author

@medyagh Thanks for your review, as your suggestion, I added mem&cpus detect and warning message in the code, the error is because my home network issue. : ) After switch the network, it's OK now, as below UT picture with warn message together. Any other comments?

image

@medyagh
Copy link
Member

medyagh commented Dec 24, 2019

@fenglixa that is really good ! I am okay with merging this. would you like to add a page in the website about using this addon ? maybe with a very simple example ? would you like to do that in this PR or in the follow up PR ?

@fenglixa
Copy link
Contributor Author

@fenglixa that is really good ! I am okay with merging this. would you like to add a page in the website about using this addon ? maybe with a very simple example ? would you like to do that in this PR or in the follow up PR ?

@medyagh , I added the document page for istio.

@medyagh
Copy link
Member

medyagh commented Dec 26, 2019

@fenglixa thanks for adding docs. Maybe also include a link to istios main documentation.

And also a small comment I added

@medyagh
Copy link
Member

medyagh commented Dec 26, 2019

@fenglixa
Copy link
Contributor Author

fenglixa commented Dec 26, 2019

@medyagh
Copy link
Member

medyagh commented Dec 27, 2019

@fenglixa thank you for this contribution.

@medyagh medyagh merged commit df9cac8 into kubernetes:master Dec 27, 2019
@fenglixa fenglixa deleted the add-istio branch December 30, 2019 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Istio addon
5 participants