Eventing container names are not unique

[aliok]

Describe the bug
In eventing and serving operators, we have a feature that we can override the image for a container using a structure like this:

apiVersion: operator.knative.dev/v1alpha1
kind: KnativeEventing
metadata:
...
spec:
  registry:
    override:
      eventing-controller: docker.io/my-eventing-controller

However, the container names are not unique.

Deployments and their container names are listed below:

Deployment : Container
broker-controller : eventing-controller
eventing-controller: eventing-controller
eventing-webhook: eventing-webhook
imc-controller: controller
imc-dispatcher: dispatcher

Expected behavior
Clear, consistent and unique container names.

To Reproduce
Try overriding the image as explained above. You can't do it properly.

Knative release version
Eventing version: 0.13.0
Eventing operator version: 0.13.0

Additional context
None

[siddharth952]

I would like to work on this.

[aliok]

@siddharth952 sorry I didn't see your comment :( I already created a PR for this: #2810

We need this change at Red Hat side, so I just worked on it as soon as I can.

[n3wscott]

All the controller container names should be called controller.

[lionelvillard]

@n3wscott why?

[n3wscott]

because that would be more consistent and easier to debug these things. Needing to know the unique container name for each snowflake deployment I find very annoying.

[matzew]

not sure why the more explicit name is an issue on the debugging side of the house?

[n3wscott]

if the name is always controller, I can use scripts to inspect the deployments, but if they are all unique names, I have to write something very custom to each deployment.

[lionelvillard]

or all have a common suffix, eg (-?)controller.

[vaikas]

Are multiple containers being wrapped into a single deployment? In which case you'd need unique controller names.

[aliok]

Let's ping @k4leung4 and @houshengbo on the operators side of things. We need to improve the way of overriding images.

[aliok]

Are multiple containers being wrapped into a single deployment? In which case you'd need unique controller names.

Not yet. But with non-unique container names, we cannot override the container images using the operator.

I created a ticket to improve this mechanism on the operator: knative/eventing-operator#148

[houshengbo]

@n3wscott What about we use the SAME deployment name for the container within it? (When you know the deployment name, then you know the container name). Deployment is unique, so container name will be unique as well.

[aliok]

/close

We now do image overriding with deploymentName+container name in the operator: knative/eventing-operator#159

So, this issue doesn't block the image override in eventing operator.
However, because of varying container names and container name patterns it is a bit confusing.
Created #3048

[knative-prow-robot]

@aliok: Closing this issue.

In response to this:

/close

We now do image overriding with deploymentName+container name in the operator: knative/eventing-operator#159

So, this issue doesn't block the image override in eventing operator.
However, because of varying container names and container name patterns it is a bit confusing.
Created #3048

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.