Skip to content

enhancements/update/phased-rollouts: Propose a new enhancement - #427

Closed
wking wants to merge 1 commit into
openshift:masterfrom
wking:phased-update-rollouts
Closed

enhancements/update/phased-rollouts: Propose a new enhancement#427
wking wants to merge 1 commit into
openshift:masterfrom
wking:phased-update-rollouts

Conversation

@wking

@wking wking commented Aug 7, 2020

Copy link
Copy Markdown
Member

Trying to build consensus around possible graph-data schemas for phased update rollouts.

Comment thread enhancements/update/phased-rollouts.md
Comment thread enhancements/update/phased-rollouts.md Outdated

## Summary

This enhancement request proposes phased update rollouts, allowing release administrators to declare a time window over which update recommendations are phased into per-cluster requets for stable channel [Cincinnati graphs][cincinnati].

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.

s/requets/requests/

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.

Also, I do not think we should link it to Cincinnati graphs here. The phased rollout is for the stable channel and the implementation details are not very important.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

s/requets/requests/

Fixed with 9b1cb55 -> eab2408.

Also, I do not think we should link it to Cincinnati graphs here.

You need to understand the nodes/edges/channels view of update recommendations to understand this proposal. I think the link to the Cincinnati in OpenShift design doc gives that background in a reasonably high-level way, and folks who feel comfortable moving on to the body of the proposal without reading that design doc are free to not click through ;).


1. Reduced likelihood of a thundering herd of updating clusters swamping canonical image registries.
We currently lack built-in support for [automatic updates][automatic-updates], but some users have built analogous functionality as a higher-level driver for ClusterVersion.
Phased rollouts make it less likely to have many clusters simultaneously initiate a given update by removing the synchronized update recommendation.

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.

In long term we will see many clusters simultaneously update even with phased rollout(when our customer base grows). However the difference is , with phased rollout majority of the overall cluster fleet can not update at the same time.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm arguing that this is part of spreading the load on canonical image registries here, not trying to argue that there will be no partial overlaps between multiple clusters. But is there particular wording you think I should add or adjust to make that more clear?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Phased rollouts reduces the number of clusters able to simultaneously initiate a given update by removing the synchronized update recommendation."

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 am ok with current wording. Also fine with Jack's suggestion.

Comment thread enhancements/update/phased-rollouts.md
@wking
wking force-pushed the phased-update-rollouts branch from 9b1cb55 to 17d9655 Compare September 10, 2020 03:09
@openshift-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wking
To complete the pull request process, please assign sttts
You can assign the PR to them by writing /assign @sttts in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wking
wking force-pushed the phased-update-rollouts branch 2 times, most recently from eab2408 to 765620f Compare September 10, 2020 04:16
@wking

wking commented Sep 10, 2020

Copy link
Copy Markdown
Member Author

I've added a new section walking through feedback latency based on this discussion with eab2408 -> 765620f.

Comment thread enhancements/update/phased-rollouts.md Outdated
Comment thread enhancements/update/phased-rollouts.md Outdated
Comment thread enhancements/update/phased-rollouts.md Outdated
Comment thread enhancements/update/phased-rollouts.md Outdated
### Non-Goals

* Deciding how to sort clusters within a given rollout window.
For an initial implementaiton, the "phased" rollout can be showing _all_ stable clusters the edges in question at the end of the configured windows, since that's effectively what we're doing today.

@jottofar jottofar Sep 22, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't the schema def be defining stable clusters even if only to support _all_?

Unless I'm misunderstanding where the subset of clusters (or all) get specified. In which case it would be helpful to speak to that as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Did this reword address your concerns here? The idea is that, for the initial policy-engine implementation, clients requesting stable channels would never see stable edges before the end of the window, and would always see stable edges after the end of the window, regardless of the cluster ID associated with the request.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understood the semantics of all. What I didn't fully understand, but do now after you explained it in our meeting, was how a given cluster would get the indication to upgrade. And my comment above was that I thought a brief explanation of that mechanism -the policy engine, etc. - would be helpful in understanding the details of the schema changes and the alternatives.

@wking
wking force-pushed the phased-update-rollouts branch 3 times, most recently from 30163ca to c0b1f6a Compare September 22, 2020 18:48
Trying to build consensus around possible graph-data schemas for
phased update rollouts.
@wking
wking force-pushed the phased-update-rollouts branch from c0b1f6a to a12905a Compare September 22, 2020 18:51

As another alternative, we could set start times slightly in the past, while keeping the fast promotion pinned to the configured start time.
That would minimize the divergence between the errata going public and the appearance of update recommendations in their fast-channel clusters.
But it would still introduce the same Update Service pod consistency issues.

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.

Also if we define the start time in the past that would create some inconsistency around the time window we provide for phased rollout.


In the rare cases when the cluster-version operator receives inconsistent graph responses as a graph-data change propagates, the impact should be small.
Clusters with [automatic updates][automatic-updates] enabled should initiate an update before the next upstream poll.
Clusters without automatic updates may have [the `UpdateAvailable` alert][cluster-version-operator-update-available] fire, resolve, and fire again.

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.

+1 to promote the edge as soon as the PR in Cincinnati graph data lands. IMO the impact would be very small to even notice.

## Motivation

We [currently][channel-semantics] has `fast-4.y` and `stable-4.y` channels for each 4.y series.
That allows users who want to hear about releases and related update edges immediately after we declare them supported to do so via `fast-4.y`, while users who want to wait and see if we still recommend the edge after its seen more volume can do so via `stable-4.y`, as discussed [here][fast-vs-stable].

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.

That allows users who want to hear about releases and related update edges immediately after we declare them supported to do so via fast-4.y , +1


We [currently][channel-semantics] has `fast-4.y` and `stable-4.y` channels for each 4.y series.
That allows users who want to hear about releases and related update edges immediately after we declare them supported to do so via `fast-4.y`, while users who want to wait and see if we still recommend the edge after its seen more volume can do so via `stable-4.y`, as discussed [here][fast-vs-stable].
However, clusters default to `stable-4.y` and users need to explicitly configure `fast-4.y` to populate that higher-risk cluster pool.

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.

We should just mention that clusters use stable-4.y by default with standard installation. I do not think we need to mention users need to explicitly configure fast-4.y to populate that higher-risk cluster pool. Will help improving the readability of this PR


1. Reduced likelihood of a thundering herd of updating clusters swamping canonical image registries.
We currently lack built-in support for [automatic updates][automatic-updates], but some users have built analogous functionality as a higher-level driver for ClusterVersion.
Phased rollouts make it less likely to have many clusters simultaneously initiate a given update by removing the synchronized update recommendation.

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 am ok with current wording. Also fine with Jack's suggestion.


Currently fast and stable promotion are two separate, manual steps (e.g. [here][4.5.2-fast] and [here][4.5.2-stable]).
With this enhancement, that will become a single supported-release promotion.
The release _nodes_ will appear in both the fast and stable channels immediately at the start of the configured time window, to avoid `VersionNotFound` errors for users in `stable-4.y` running the release (e.g. because they installed it immediately after it was announced as GA, avoiding contention like [this][4.4.3-version-not-found]).

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.

Fast channel should see the nodes immediately after we merge the PR as you have mentioned beginning of the PR.

Currently fast and stable promotion are two separate, manual steps (e.g. [here][4.5.2-fast] and [here][4.5.2-stable]).
With this enhancement, that will become a single supported-release promotion.
The release _nodes_ will appear in both the fast and stable channels immediately at the start of the configured time window, to avoid `VersionNotFound` errors for users in `stable-4.y` running the release (e.g. because they installed it immediately after it was announced as GA, avoiding contention like [this][4.4.3-version-not-found]).
Update-recommendation edges associated with nodes in `fast-4.y` will also appear immediately at the start of the configured time window.

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.

-1 on this as we should not introduce cosmetic delay for fast channel promotion.

@openshift-bot

Copy link
Copy Markdown

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 22, 2020
@sdodson

sdodson commented Dec 25, 2020

Copy link
Copy Markdown
Member

/lifecycle frozen

@openshift-ci-robot openshift-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 25, 2020
@spadgett
spadgett removed their request for review January 8, 2021 20:41
@dhellmann

Copy link
Copy Markdown
Contributor

This proposal is over a year old. As part of recent efforts to clean up old pull requests, I am removing the life-cycle/frozen label to allow it to age out and be closed. If the proposal is still active, please restore the label.

/remove-lifecycle frozen

@openshift-ci openshift-ci Bot removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Sep 21, 2021
@openshift-bot

Copy link
Copy Markdown

Inactive enhancement proposals go stale after 28d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Mark the proposal as fresh by commenting /remove-lifecycle stale.
Stale proposals rot after an additional 7d of inactivity and eventually close.
Exclude this proposal from closing by commenting /lifecycle frozen.

If this proposal is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 19, 2021
@openshift-bot

Copy link
Copy Markdown

Stale enhancement proposals rot after 7d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Mark the proposal as fresh by commenting /remove-lifecycle rotten.
Rotten proposals close after an additional 7d of inactivity.
Exclude this proposal from closing by commenting /lifecycle frozen.

If this proposal is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci Bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 26, 2021
@openshift-bot

Copy link
Copy Markdown

Rotten enhancement proposals close after 7d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Reopen the proposal by commenting /reopen.
Mark the proposal as fresh by commenting /remove-lifecycle rotten.
Exclude this proposal from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci Bot closed this Nov 2, 2021
@openshift-ci

openshift-ci Bot commented Nov 2, 2021

Copy link
Copy Markdown
Contributor

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten enhancement proposals close after 7d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Reopen the proposal by commenting /reopen.
Mark the proposal as fresh by commenting /remove-lifecycle rotten.
Exclude this proposal from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants