don't inject istio sidecar in feed controller job

[rootfs]

Fixes #338

Proposed Changes

• don't inject istio sidecar, this should help those feed sources that need to initialize by accessing external services (e.g. k8s API server and services)

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rootfs
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: inlined

If they are not already assigned, you can assign the PR to them by writing /assign @inlined in a comment when ready.

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[rootfs]

/assign @grantr

[rootfs]

/cc @pmorie

[knative-prow-robot]

@rootfs: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-knative-eventing-unit-tests	465c268	link	/test pull-knative-eventing-unit-tests

Full PR test history. Your PR dashboard.

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.

[rootfs]

Will fix unit tests if this fix is the way to go.

[scothis]

I'm a bit torn on this one...

The pod will only be istio injected if the namespace it is created within has opted-in to injection. I feel like we should respect that preference. On the other hand, needing to create a ServiceEntry to allow outbound communication is annoying, especially since the feedlet is not always Istio injected.

[rootfs]

@scothis to istio or not istio, that's a question.

I believe opt-in is surely required for all watch pods. Otherwise, there is not way for existing event sources, gcp pubsub or k8sevent, work.

On the other hand, we don't want istio to micro manage the feed controller job, the best solution is to disable istio for the job.

@grantr any comments?

[evankanderson]

This feels like another place where launching controllers dynamically removes options from both the contributors and the cluster operators. If we launched Feed controllers as independent Deployments/pods/whatnot, then the cluster operator could inspect the yaml manifest for the controller and update it to work on their cluster as needed. The other option would be to make the Job templates configurable by reading them from a ConfigMap and apply some templating to the ConfigMap values. Personally, I think that's probably a golden path to a hot situation, but it would give us some temporary flexibility to keep using our existing code. @n3wscott is putting together a proposal to stop using Jobs for reconciliation (since it also misses non-create events at the moment).

…

On Thu, Aug 9, 2018 at 1:53 PM Scott Andrews ***@***.***> wrote: I'm a bit torn on this one... The pod will only be istio injected if the namespace it is created within has opted-in to injection. I feel like we should respect that preference. On the other hand, needing to create a ServiceEntry to allow outbound communication is annoying, especially since the feedlet is not always Istio injected. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#339 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlyN90R_3o7CUaIGDe7AFMzEFTkS4c_ks5uPKEvgaJpZM4V2VGJ> .

-- Evan Anderson <argent@google.com>

[n3wscott]

I would like to let this in for now until jobs go away.

/lgtm

Tested locally