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

Fixes #20462: Delay policy generation until rudder app is fully boot #4066

Conversation

fanf
Copy link
Member

@fanf fanf commented Dec 21, 2021

https://issues.rudder.io/issues/20462

That PR add a promise (ie something that can be done exactly once, and that other things can wait for it to be done) that is checked for completion before actually starting policy generation.

The promise is completed at end of Boot.boot method, ie once lift knows rudder is started.

To be able to use the promise.await in policy generation agent, I needed to transform the code into a ZIO effect.

Also remove a warning for missing pattern match which would lead to a runtime exception, likely introduced in pr #4065

Copy link
Member

@ncharles ncharles left a comment

Choose a reason for hiding this comment

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

You need to rebase the test, and correct a comment
The rest is 💯

@@ -554,6 +554,10 @@ class Boot extends Loggable {
ApplicationLogger.error(s"Error when trying to save the EventLog for application start: ${err.fullMsg}")
case Right(_) => ApplicationLogger.info("Application Rudder started")
}

// release guard for promise generation awaiting end of boot
RudderConfig.policyGenerationBootGuard.succeed(()).runNow
Copy link
Member

Choose a reason for hiding this comment

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

I would never have thought to put it there, i was more thinking to put it at the end of bootcheck. It is without doubt better the way you did

@fanf
Copy link
Member Author

fanf commented Dec 21, 2021

PR rebased

@fanf fanf force-pushed the ust_20462/delay_policy_generation_until_rudder_app_is_fully_boot branch from 5d631d3 to a339863 Compare December 21, 2021 20:07
@fanf fanf requested a review from ncharles December 21, 2021 20:09
@@ -190,6 +190,7 @@ final case class FullNodeGroupCategory(
case FullOtherTarget(t) => t match {
case AllTarget => true
case AllTargetExceptPolicyServers => !node.isPolicyServer
case AllPolicyServers => node.isPolicyServer
Copy link
Member

Choose a reason for hiding this comment

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

this file should not change

Copy link
Member Author

Choose a reason for hiding this comment

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

it removes a warning missing pattern match (and the corresponding runtime exception) for a previous pr regarding migration.
I'm adding a comment in head.

Copy link
Member

Choose a reason for hiding this comment

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

ok

@Normation-Quality-Assistant
Copy link
Contributor

OK, merging this PR

@Normation-Quality-Assistant Normation-Quality-Assistant merged commit a339863 into Normation:branches/rudder/7.0 Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants