Skip to content
This repository was archived by the owner on Jun 19, 2022. It is now read-only.

Implementation of Targets that knows it's backed by a configmap #830

Closed
grantr opened this issue Apr 14, 2020 · 2 comments
Closed

Implementation of Targets that knows it's backed by a configmap #830

grantr opened this issue Apr 14, 2020 · 2 comments
Assignees
Labels
area/broker kind/feature-request New feature or request priority/2 Nice to have feature but doesn't block current release defined by release/* release/1
Milestone

Comments

@grantr
Copy link
Contributor

grantr commented Apr 14, 2020

Problem
Currently the Broker reconciler uses the in-memory variant of config.Targets to aggregate broker and trigger routing and handles persisting that to a configmap. This adds a lot of complexity to the reconciler: since reconciler threads can process multiple brokers at once, it has to run a separate goroutine to update the configmap safely. It would be nicer if there were a variant of Targets that wrapped up all the configmap load/store logic in the same interface.

It's also challenging to test this logic with normal reconciler table tests, because the configmap update happens outside the Reconcile method.

Persona:
Contributor

Exit Criteria
The Broker reconciler can delegate load/store to a separate library.

Additional context (optional)
This should probably wait until #815 is resolved.

@grantr grantr added kind/feature-request New feature or request area/broker labels Apr 14, 2020
@yolocs
Copy link
Member

yolocs commented Apr 15, 2020

/assign

@grantr grantr added priority/2 Nice to have feature but doesn't block current release defined by release/* release/1 labels Apr 21, 2020
@grantr grantr added this to the Backlog milestone Apr 21, 2020
@grantr
Copy link
Contributor Author

grantr commented Jun 15, 2020

This is no longer necessary with #852 and #863. Can reopen if we decide it's still useful.

@grantr grantr closed this as completed Jun 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/broker kind/feature-request New feature or request priority/2 Nice to have feature but doesn't block current release defined by release/* release/1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants