Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Events controller #40

Merged
merged 5 commits into from
Apr 3, 2024
Merged

Events controller #40

merged 5 commits into from
Apr 3, 2024

Conversation

joelrebel
Copy link
Member

Events controller abstracts the NATS JS, KV interaction for workqueue specific controllers.
This offloads all the boiler plate for NATS config and events, KV interaction from controllers.

Callers create a new controller instance with the required parameters,

nc := controller.NewNatsController(
	"flasher",
	facilityCode,
	"firmwareInstall",
	natsURL,
	natsCreds,
	"firmwareInstall",
	controller.WithConcurrency(10),
	controller.WithKVReplicas(1),
	controller.WithLogger(flasher.Logger),
	controller.WithConnectionTimeout(connectTimeout),
)

connect,

if err := nc.Connect(ctx); err != nil {
	flasher.Logger.Fatal(err)
}

register an event handler factory method and listen for events,

handlerFactory := func() controller.ConditionHandler {
	return &ConditionTaskHandler{
		store:          repository,
		syncWG:         &sync.WaitGroup{},
		logger:         logger,
		dryrun:         dryrun,
		faultInjection: faultInjection,
		facilityCode:   nc.FacilityCode(),
		controllerID:   nc.ID(),
	}
}

if err := nc.ListenEvents(ctx, handlerFactory); err != nil {
	logger.Fatal(err)
}

a functional implementation is here metal-toolbox/flasher#144

Makefile Show resolved Hide resolved
@joelrebel joelrebel requested a review from DoctorVin March 22, 2024 16:23
Copy link
Contributor

@DoctorVin DoctorVin left a comment

Choose a reason for hiding this comment

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

A few comments. Apologies for the delay in getting this reviewed.

.mockery.yaml Show resolved Hide resolved
events/controller/config.go Show resolved Hide resolved
events/controller/controller.go Show resolved Hide resolved
events/controller/controller.go Show resolved Hide resolved
events/controller/controller.go Show resolved Hide resolved
events/controller/controller.go Outdated Show resolved Hide resolved
events/controller/controller.go Show resolved Hide resolved
events/controller/controller.go Show resolved Hide resolved
go monitor()
defer close(doneCh)

return n.conditionHandlerFactory().Handle(ctx, cond, conditionStatusPublisher)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really a fan of the factory idiom here. n.conditionHandler could be a function pointer (the equivalent of what is allocated from the factory).

events/controller/status_test.go Show resolved Hide resolved
Copy link
Contributor

@DoctorVin DoctorVin left a comment

Choose a reason for hiding this comment

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

There's a need for offline discussion but this is functional now and can be revised later.

@joelrebel joelrebel merged commit 643def4 into main Apr 3, 2024
6 checks passed
@joelrebel joelrebel deleted the controller branch April 3, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants