Fixed names rather than Generate Name

[Harwayne]

Problem
Numerous resources are created with GenerateName. If the listers are stale, this can cause the resources to be created multiple times, as an existing resource is not found. We should use predetermined, fixed names, rather than GenerateName, so that we will get 409, rather than duplication when the lister is stale.

Persona:
Developers.

Exit Criteria
No resource created by an Eventing reconciler uses GenerateName

Non-goal
These are explicit non-goals:

• It is not a goal that names need to be determined before the object is created.
  • For example, a Channel's K8s Service may include the Channel's UID, which will not be known before the Channel is created. It is still deterministic for the given Channel, but not guessable before the Channel is created.
    • A guessable name for Channel's may be a good idea, but is unrelated to this feature.
• It is not a goal that fixed names will stay fixed forever.
  • As long as the reconciler understands the safety of the name change, it is allowed to do so. For example, the K8s Service for a Channel may change to a new fixed name scheme. As all usage of the Channel must be through the addressable contract, this change will not negatively affect other components.

Additional context (optional)
Multiple Subscriptions generated by a single Trigger is especially pernicious and has been observed in practice.

[ian-mi]

I saw this issue with the hello world sample after performing an upgrade to 0.6. Here one can see many redundant subscriptions for the two sample triggers:

$ kubectl get triggers -n kn-eventing-sample
NAME READY REASON BROKER SUBSCRIBER_URI AGE
event-display-dev True default http://event-display-dev.kn-eventing-sample.svc.cluster.local/ 2h
event-display-test True default http://event-display-test.kn-eventing-sample.svc.cluster.local/ 2h

$ kubectl get subscriptions -n kn-eventing-sample
NAME READY REASON AGE
default-event-display-dev-g45m9 True 29m
default-event-display-dev-svw6r True 29m
default-event-display-test-2x5vh True 29m
default-event-display-test-h7vb2 True 29m
default-event-display-test-jjn27 True 29m
default-event-display-test-l57qq True 29m
default-event-display-test-pd9zm True 29m
default-event-display-test-sdw6c True 29m
default-event-display-test-vn4qm True 29m
default-event-display-test-xr4gk True 29m
internal-ingress-default-d7jd5 True 2h

[srvaroa]

@Harwayne I have started with this

[akashrv]

@srvaroa Are you actively working on this issue? If so, could you please assign it to yourself "/assign srvaroa" and also update status?

[Harwayne]

/assign

[Harwayne]

Looking through the code, I see we use GenerateName in the following places. Each has slightly distinct properties, so will be handled differently.

1. Channel Reconcilers
   a. K8s Services - Create the new fixed name K8s Service and use it as the Channel's Address. Leave the old K8s Service in existence, it will be deleted when the Channel is deleted. The old K8s Service is an ExternalName Service, so the only effect it will have is to add an extra, unusable DNS entry.
2. Importer Reconcilers
   a. EventTypes - Create the new fixed name EventTypes and delete the old GenerateName EventTypes. These are intended for human consumption, leaving duplicates around will just confuse people. This will require code cleanup in the subsequent release (no longer deleting old GenerateName EventTypes).
3. Broker Reconciler - We will only be modifying the new, CRD-based Channel path. The old provisioner Channel path will continue to use GenerateName, as it will be removed in the next release anyway.
   a. Channels - The CRD Channel path already uses fixed names, but for the Subscription changes to work well, the name will be changed. The old Channels will not be cleaned up. Rather they will continue to exist until the Broker is deleted. ‡
   a. Subscription - Create a new Subscription on the new Channels. This Subscription occurs after the Channels in the reconcile loop, so a single run through the loop will create the new Subscription on the correct Channels. The old Subscription will continue to exist.
4. Trigger Reconciler
   a. Subscription - Create a new Subscription. Leave the old Subscription unaltered. There is a race condition between the Trigger reconciler and the Broker reconciler. If the Broker reconciler finishes first, then the new Subscription will correctly be on the new Channels. If the Trigger reconciler starts before the Broker reconciler finishes, then the Subscription will initially be on the old Channels. Once the Broker reconciler finishes, then the Trigger reconciler will reconcile again and delete the new Subscription that is on the old Channels and create a new Subscription on the new Channels. While the new Subscription is on the old Channels, there will be duplicate delivery. We could mitigate that by having the Trigger reconciler check the names of the Channels it will Subscribe to, but that seems unnecessary, as subscribers already need to handle duplicate delivery.

‡ - Leaving the old Channels and Subscriptions in place is the best way I have found to ensure events aren't lost. If we immediately deleted them, then anything currently buffered by those Channels and Subscriptions would be lost. There is extra utilization by having the Channels and Subscriptions present, but if no events are flowing through them it should be minuscule. We could add the ability to delete the old versions in the 0.9 release. I propose not doing so unless someone explicitly requests it.

[Harwayne]

An update to #1250 (comment).

1. Channel reconcilers.
   There are three CRD based Channel reconcilers:

• InMemoryChannel
• KafkaChannel
• NatssChannel

InMemoryChannel and KafkaChannel already used a fixed name, fmt.Sprtintf("%s-kn-channel", channel.Name). NatssChannel, as of #1588, now uses a fixed name too. So this work is complete. The older provisioner code still uses List with label selector, but we are not planning to update the provisioner code to use fixed names.

2. Importer reconcilers.
   Both EventTypes and Receive Adapters are GenerateNamed. ApiServerSource uses fixed names, rather than GenerateName #1602 fixes ApiServerSource, we still need to do the same for CronJobSource and ContainerSource.

3. Broker reconciler. Finished via Trigger & Broker - Fixed Names #1563.

4. Trigger reconciler. Finished via Trigger & Broker - Fixed Names #1563.