Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

add tolerations #601

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

add tolerations #601

wants to merge 2 commits into from

Conversation

richardwxn
Copy link
Contributor

To resolve istio/istio#19480

@richardwxn richardwxn requested a review from a team as a code owner December 10, 2019 19:40
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Dec 10, 2019
@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 10, 2019
howardjohn
howardjohn reviewed Dec 10, 2019
View reviewed changes
Copy link
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

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

Why not just use the k8s spec?

Seems like we are missing an {{ end }}?

@richardwxn
Copy link
Contributor Author

@howardjohn yep we should actually use the k8s specs under IstioControlPlane, but there are users complain that we claim to still support helm api at this stage so they tend to believe that overlaying k8s settings under values passthrough part would work as the same as in IstioControlPlane spec.

As an alternative, I have a change locally to improve the validation function, for example it should give a warning and suggests the correct path when user try to set in values pass through

@nick4fake
Copy link

nick4fake commented Dec 12, 2019 •
edited
Loading

@howardjohn

Why not just use the k8s spec?

Because there is no way to specify it through k8s spec. I am using k8s spec as much as possible, but several components are not part of the ControlPlaneSpec

@nick4fake nick4fake mentioned this pull request Dec 12, 2019
Closed
@howardjohn
Copy link
Member

@nick4fake seems like its in the proto: https://github.com/istio/operator/blob/8569a17c89f10190d5e799c3e10b9426c18883b3/pkg/apis/istio/v1alpha2/istiocontrolplane_types.proto#L512 does it not work?

@richardwxn
Copy link
Contributor Author

@howardjohn I think the problem is that it is not defined in the templates of kiali and tracing, unless they do a k8soverlay

@jmazzitelli
Copy link
Member

I think the problem is that it is not defined in the templates of kiali

The Kiali integration will support these toleration settings once these two installer/operator PRs are able to get merged ;-)

@richardwxn
Copy link
Contributor Author

we can unblock it for tracing and kiali toleration first

@richardwxn
Copy link
Contributor Author

PTAL @howardjohn

@istio-testing
Copy link

@richardwxn: PR needs rebase.

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.

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@howardjohn howardjohn howardjohn left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. needs-rebase Indicates a PR needs to be rebased before being merged size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

There is no way to provide "tolerations" to Kiali and tracing components
6 participants
@richardwxn @nick4fake @howardjohn @jmazzitelli @istio-testing @googlebot