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

Provide a mechanism to allow updates in certain time window #241

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

gthao313
Copy link
Member

@gthao313 gthao313 commented Sep 12, 2022

users could have workload that cannot be interrupted during some periods of the day, so we provide a mechanism to prevent Bottlerocket nodes update on user customized time window.

Issue number:

Closes: #67

Description of changes:

Author: Tianhao Geng <tianhg@amazon.com>

Provide a mechanism to allow updates in certain update time window

Users could have workload that cannot be interrupted during some periods
of the day, so we provide a mechanism to allow Bottlerocket nodes updates
on user customized update time window. Beyond update time window,
bottlerocker update operator will prevent any updates.

Testing done:
Set up update time window and then confirm if brupop stops updating nodes except in-process node.

UPDATE_WINDOW_START=23:55:0
UPDATE_WINDOW_STOP=0:10:0
UPDATE_WINDOW_START=0:15:0
UPDATE_WINDOW_STOP=0:24:0

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@gthao313
Copy link
Member Author

This PR has conflicts with #238. I'll solve the conflicts when #238 is merged.

@gthao313 gthao313 force-pushed the time-window branch 2 times, most recently from 4afb619 to 06d484e Compare September 20, 2022 00:19
@gthao313 gthao313 requested a review from jpmcb September 21, 2022 17:13
controller/src/controller.rs Outdated Show resolved Hide resolved
controller/src/controller.rs Outdated Show resolved Hide resolved
.env Show resolved Hide resolved
.env Outdated Show resolved Hide resolved
@gthao313 gthao313 force-pushed the time-window branch 3 times, most recently from 8e9c0b8 to e602f1f Compare September 28, 2022 17:18
@gthao313
Copy link
Member Author

Push above:

  1. Add a buffer 6 mins before time window coming - this can make sure that brupop completes in-process updates and doesn't have any actions when time window starts.
    2.Add a strategy to deal with if window start time later than end time - will recognize it as cross day period. like 11pm to 2am.
  2. rename time window related env variable.
  3. rebase

Copy link
Contributor

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

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

This looks good to me! Just a few doc nits


Testing:

Build the image from this pull request and deployed to a cluster with v1.9.1 nodes. I set the following parameters for the time window:

            - name: PREVENT_UPDATE_SINCE
              value: "19:0:0"
            - name: PREVENT_UPDATE_UNTIL
              value: "20:0:0"

Where the current utc time was 19:45 - I saw that the bottlerocketshadows were idel until just after 20:00 where the update occurred


On the discussion if pushing a currently updating BR node to completion if the time window is reached is a good idea or not, I don't have enough context to have a strong opinion. But, I think this behavior is fine for now and if we get user feedback that it's not what they want or expected, we can re-visit it 👍🏼

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
controller/src/controller.rs Show resolved Hide resolved
.env Outdated
Comment on lines 32 to 33
PREVENT_UPDATE_SINCE=0:0:0
PREVENT_UPDATE_UNTIL=0:0:0
Copy link
Contributor

Choose a reason for hiding this comment

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

Flipping this around might be easier to understand:

UPDATE_WINDOW_BEGIN
UPDATE_WINDOW_END

Or

UPDATE_WINDOW_START
UPDATE_WINDOW_STOP

Copy link
Member Author

Choose a reason for hiding this comment

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

@webern you think it's better to rename or you think use update window logic instead of prevent window logic?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems more cognitively intuitive to me since I am familiar with the idea of a "maintenance window", but not a "no maintenance window".

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, that's a good point!
I'd like to keep "no maintenance window" because actually this feature was asked by customers when we was implementing brupop 0.1.0. Customer wanted us to provide a feature which can prevent update on current time. Therefore, I implemented on this way.

I like maintenance window this idea, if you think it worths to do in this way, I can update it! thanks : )

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm Matt brings up a good point: I'd be surprised if update windows were extremely wide leading to the need for "preventative, no maintenance" windows. My assumption is that most people running kubernetes clusters will have small windows of time when updates should be allowed.

So, that's a +1 from me for UPDATE_WINDOW_START and UPDATE_WINDOW_END

@gthao313
Copy link
Member Author

Push above changes logic from prevent update time window to update time window - "maintenance window".

Copy link
Contributor

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

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

Deployed a nodegroup with bottlerocket version 1.9.0 and applied the update operator yamls with

            - name: UPDATE_WINDOW_START
              value: "17:0:0"
            - name: UPDATE_WINDOW_STOP
              value: "18:0:0"

where the current UTC was ~16:45. I watched the nodes in the background and saw them begin to update at 17:00. Great work!!!!! 👏🏼

controller/src/controller.rs Outdated Show resolved Hide resolved
@gthao313 gthao313 changed the title Provide a mechanism to prevent updates in certain time window Provide a mechanism to allow updates in certain time window Oct 11, 2022
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Users could have workload that cannot be interrupted during some periods
of the day, so we provide a mechanism to allow Bottlerocket nodes updates
on user customized update time window. Beyond update time window,
bottlerocker update operator will prevent any updates.
@gthao313
Copy link
Member Author

Push above Rebase and fix README according to Matt's comments.

@gthao313 gthao313 merged commit 2a11832 into bottlerocket-os:develop Oct 19, 2022
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.

Provide a mechanism to prevent updates in certain time window
5 participants