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 #22378: Generate policies for campaigns before it starts officially, delete them after it stops (1 hour delay each) #4666

Conversation

VinceMacBuche
Copy link
Member

@@ -174,14 +174,15 @@ class MainCampaignService(repo: CampaignEventRepository, campaignRepo: CampaignR
event.copy(state = Skipped(s"Event was cancelled because campaign is archived. ${reasonMessage}"))
)
case Enabled =>
if (event.start.isAfter(now)) {
val effectiveStart = event.start.minusHours(1)
Copy link
Member

Choose a reason for hiding this comment

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

can you make these two time parameter of the MainCampaignService class so that it remains testable / configurable ?

if (event.start.isAfter(now)) {
val effectiveStart = event.start.minusHours(1)
val effectiveEnd = event.end.plusHours(1)
if (effectiveStart.isAfter(now)) {
for {
// Campaign should be planned, not running
_ <-
CampaignLogger.warn(
s"Campaign event ${event.id.value} was considered Running but we are before it start date, setting state to Schedule and wait for event to start, on ${DateFormaterService
Copy link
Member

Choose a reason for hiding this comment

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

s/it start/its start/

for {
// Campaign should be planned, not running
_ <-
CampaignLogger.warn(
s"Campaign event ${event.id.value} was considered Running but we are before it start date, setting state to Schedule and wait for event to start, on ${DateFormaterService
.serialize(event.start)}"
.serialize(effectiveStart)}, one hour before official start date, to ensure policies are correctly dispatched, nothing will be applied on the node"
Copy link
Member

@fanf fanf Feb 16, 2023

Choose a reason for hiding this comment

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

comment will have to be adapted to the param to, something like:

s"Campaign event ${event.id.value} was considered Running but we are before its start date, setting state to
Schedule and wait for event to start, on ${DateFormaterService.serialize(effectiveStart)}, (starting befor official
 start date (${event.start}), to ensure policies are correctly dispatched, nothing will be applied on the node)"

Copy link
Member

@fanf fanf left a comment

Choose a reason for hiding this comment

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

LGTM to me, can be merged once the comment is addressed

@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the bug_22378/generate_policies_for_campaigns_before_it_starts_officially_delete_them_after_it_stops_1_hour_delay_each branch from 02e2945 to fc8fc70 Compare February 16, 2023 16:29
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the bug_22378/generate_policies_for_campaigns_before_it_starts_officially_delete_them_after_it_stops_1_hour_delay_each branch from fc8fc70 to f577b99 Compare February 16, 2023 16:31
…ally, delete them after it stops (1 hour delay each)
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the bug_22378/generate_policies_for_campaigns_before_it_starts_officially_delete_them_after_it_stops_1_hour_delay_each branch from f577b99 to 1e7f91b Compare February 16, 2023 16:36
@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/4666
-- Your faithful QA
Kant merge: "Live your life as though your every act were to become a universal law."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/65072/console)

@fanf
Copy link
Member

fanf commented Feb 16, 2023

OK, merging this PR

@fanf fanf merged commit 93bce59 into Normation:branches/rudder/7.2 Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants