Skip to content

feat: add event source for triggering component version objects#1433

Merged
Skarlso merged 10 commits into
open-component-model:mainfrom
Skarlso:add-event-source-to-resolution-service
Dec 18, 2025
Merged

feat: add event source for triggering component version objects#1433
Skarlso merged 10 commits into
open-component-model:mainfrom
Skarlso:add-event-source-to-resolution-service

Conversation

@Skarlso

@Skarlso Skarlso commented Dec 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Fixes open-component-model/ocm-project#784

Which issue(s) this PR fixes

@github-actions github-actions Bot added kind/feature new feature, enhancement, improvement, extension size/m Medium labels Dec 15, 2025
@Skarlso Skarlso force-pushed the add-event-source-to-resolution-service branch 3 times, most recently from 5508973 to ac69456 Compare December 16, 2025 08:16
@Skarlso

Skarlso commented Dec 16, 2025

Copy link
Copy Markdown
Contributor Author
Screenshot 2025-12-16 at 9 36 41

Looks like multiple versions are also working.

@Skarlso Skarlso force-pushed the add-event-source-to-resolution-service branch from ac69456 to a121e8b Compare December 16, 2025 09:14
@Skarlso

Skarlso commented Dec 16, 2025

Copy link
Copy Markdown
Contributor Author

I suspect the unit test error is because now it was reconciled by another test. I can't reproduce it yet locally.

@Skarlso

Skarlso commented Dec 17, 2025

Copy link
Copy Markdown
Contributor Author

Yes! Managed to reproduce it with -race.

@Skarlso

Skarlso commented Dec 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

ah, the reconciler is just too fast now. :D :D :D I even have that note there :D :D

			// This is a bit awkward because we trust that we will be faster than the reconciler (which is most of the
			// time the case).
			By("checking that the component returns ResolutionInProgress on first reconciliation")

I going to have to be clever if I want to do this.

@Skarlso

Skarlso commented Dec 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Proof that it's being reconciled too fast:

  2025-12-17T09:52:39+01:00	INFO	component version resolution in progress, waiting for event notification	{"controller": "component", "controllerGroup": "delivery.ocm.software", "controllerKind": "Component", "Component": {"name":"test-component","namespace":"returns-a-resolution-in-progress-error-on-the-first-hit"}, "namespace": "returns-a-resolution-in-progress-error-on-the-first-hit", "name": "test-component", "reconcileID": "4df5d228-a39a-4a6d-9730-3e20ec203859", "component": "ocm.software/test-component-returns-a-resolution-in-progress-error-on-the-first-hit", "version": "1.0.0"}
  "level"=0 "msg"="Event received" "event"="Warning ResolutionInProgress component version resolution in progress involvedObject{kind=Component,apiVersion=delivery.ocm.software/v1alpha1}"
  2025-12-17T09:52:39+01:00	DEBUG	worker-pool	processing work item	{"worker": 9, "key": "4ce7d85778497876"}
  2025-12-17T09:52:39+01:00	DEBUG	worker-pool	processed work item	{"worker": 9, "component": "ocm.software/test-component-returns-a-resolution-in-progress-error-on-the-first-hit", "version": "1.0.0", "duration": 0.000983709}
  2025-12-17T09:52:39+01:00	DEBUG	worker-pool	emitted resolution event	{"worker": 9, "component": "ocm.software/test-component-returns-a-resolution-in-progress-error-on-the-first-hit", "version": "1.0.0", "requesterCount": 1, "requesters": [{"NamespacedName":{"Namespace":"returns-a-resolution-in-progress-error-on-the-first-hit","Name":"test-component"}}]}
  2025-12-17T09:52:39+01:00	DEBUG	resolution-event-source	enqueued reconciliation request from resolution event	{"component": "ocm.software/test-component-returns-a-resolution-in-progress-error-on-the-first-hit", "version": "1.0.0", "requester": {"name":"test-component","namespace":"returns-a-resolution-in-progress-error-on-the-first-hit"}, "error": null}

Level:

Level Before Removed:  [{Ready False 1 2025-12-17 10:25:48 +0100 CET ResolutionInProgress component version resolution in progress}]

@github-actions github-actions Bot added the size/l Large label Dec 17, 2025
@Skarlso Skarlso force-pushed the add-event-source-to-resolution-service branch from cc5e183 to fa24296 Compare December 17, 2025 13:59
@Skarlso Skarlso marked this pull request as ready for review December 17, 2025 14:01
@Skarlso Skarlso requested a review from a team as a code owner December 17, 2025 14:01
jakobmoellerdev
jakobmoellerdev reviewed Dec 17, 2025
View reviewed changes

@jakobmoellerdev jakobmoellerdev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was expecting some testing for this channel handling. WDYT?

Comment thread kubernetes/controller/internal/controller/component/component_controller.go Outdated
Comment thread kubernetes/controller/internal/controller/component/component_controller_unit_test.go
Comment thread kubernetes/controller/internal/resolution/cached_repository.go
Comment thread kubernetes/controller/internal/resolution/workerpool/event.go Outdated
Comment thread kubernetes/controller/internal/resolution/workerpool/source.go Outdated
Comment thread kubernetes/controller/internal/resolution/workerpool/source.go Outdated
Comment thread kubernetes/controller/internal/resolution/workerpool/source.go
Comment thread kubernetes/controller/internal/resolution/workerpool/workerpool.go Outdated
Comment thread kubernetes/controller/internal/resolution/workerpool/workerpool.go Outdated
Skarlso
Skarlso commented Dec 18, 2025
View reviewed changes
Comment thread kubernetes/controller/internal/resolution/workerpool/workerpool.go
Skarlso
Skarlso commented Dec 18, 2025
View reviewed changes
Comment thread kubernetes/controller/internal/resolution/workerpool/workerpool.go Outdated
Skarlso
Skarlso commented Dec 18, 2025
View reviewed changes
Comment thread kubernetes/controller/internal/resolution/workerpool/workerpool.go
@Skarlso

Skarlso commented Dec 18, 2025

Copy link
Copy Markdown
Contributor Author

Alright, I addressed a bunch of things, I'm going to now write the tests for the event channel.

@Skarlso
feat: add event source for triggering component version objects
29cee94 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
add a fall-back mechanism for stuck components
904ad40 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
extract the inProgress test into a unit test
1d3abb2 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
non-blocking send and handle key generation failure
c443ba4 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
remove blocking and add context check in the send to prevent panic
7cd6d25 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
pass in the key from the outside
3075bb6 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
remove handler functions and embed and remove requeue
c51c147 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
simplify the event channel and reduce logging and add some more docs
938f516 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
added two tests for the channel
023fae3 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso Skarlso force-pushed the add-event-source-to-resolution-service branch from 591d7a0 to 023fae3 Compare December 18, 2025 11:56
frewilhelm

This comment was marked as resolved.

Sign in to view

@Skarlso Skarlso merged commit a509744 into open-component-model:main Dec 18, 2025
19 checks passed
@Skarlso Skarlso deleted the add-event-source-to-resolution-service branch December 18, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@frewilhelm frewilhelm frewilhelm approved these changes

@jakobmoellerdev jakobmoellerdev jakobmoellerdev approved these changes

Assignees

No one assigned

Labels

kind/feature new feature, enhancement, improvement, extension size/l Large size/m Medium

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement an event source that triggers a reconciliation instead of returning an error

3 participants

@Skarlso @frewilhelm @jakobmoellerdev