config/v1/types_cluster_version: Add ClusterVersionSpec.AutomaticUpdates #326

wking · 2019-05-29T19:48:05Z

Give cluster admins a knob for turning these on and off.

This is part of addressing openshift/cluster-version-operator#99.

openshift-ci-robot · 2019-05-29T19:48:27Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wking
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: eparis

If they are not already assigned, you can assign the PR to them by writing /assign @eparis in a comment when ready.

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

OWNERS

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

config/v1/types_cluster_version.go

smarterclayton

Needs a design

Give cluster admins a knob for turning these on and off. You could conceivably build a bunch of logic in here, like an update-schedule schema configuring allowed upgrade times [1]. But it's going to be hard to get a spec schema that supports all the upgrade-gating logic that users will need. Instead, we have a simple boolean switch here and very basic logic in the cluster-version operator. Users who want more control can run their own Cincinnati proxy/mirrors with Policy Engines [2] that add their desired behavior (e.g. removing all edges during times when they don't want clusters auto-upgrading). [1]: openshift#326 (comment) [2]: https://github.com/openshift/cincinnati/blob/1777fc45cefaab3c8c0e870ef1dfc29564d2d37b/docs/design/cincinnati.md#policy-engine

With: $ make generate

abhinavdahiya · 2019-05-30T16:35:00Z

config/v1/types_cluster_version.go


+	// automaticUpdates enables automatic updates.
+	// +optional
+	AutomaticUpdates bool `json:"automaticUpdates,omitempty"`


We want people to opt-out, this API is opt-in.

We want people to opt-out, this API is opt-in.

Existing users will need to opt in. How do we do that with an opt-out API? Can we have an opt-in API here and have an opt-out API in the install-config, so new installers will generate ClusterVersion with this set true by default?

The zero value of a bool is false so "auto-updates=false" means "no auto updates by default", thus it's opt in.

It's opt in here, and I think we want that for compat with existing clusters (which are all manual-update). If/when we add an installer knob that feeds this property, that installer knob should be opt out. Are we all on the same page, or am I missing something?

wking · 2019-05-30T16:37:15Z

I meant it to be a boolean. Schedule structure sounds good; I'll reroll tonight.

I've pushed d3cfead -> 0b9690b to go with a boolean after all. Talking this over with @abhinavdahiya and @crawford, I think getting a spec structure that makes most users happy is going to be hard, and it diverges from the "simple CVO, complex logic in Cincinnati" approach we've followed so far. With a boolean "should I auto-upgrade?" switch for the CVO, users that need more complicated logic can implement that by pointing clusters at their own Cincinnati proxy that slurps the graph from our upstream Cincinnati (or wherever) and applies the logic they want (e.g. removing all upgrade edges during times when they don't want clusters auto-upgrading). If we find there's a lot of demand for specific logic (like your cron schedule schema), we can ship images for Policy Engine Cincinnati proxies like that to make it easy for users to set them up locally. But we don't need to bake that logic into the cluster itself. Thoughts?

smarterclayton · 2019-05-31T08:05:57Z

I disagree. I think Cincinnati should be responsible for a single job - telling a cluster which versions it can update to. Given the disagreement, this needs to go to a design doc where we can describe the use cases we're trying to solve. For example, we should never allow a compromise of Cincinnati to force updates to a known bad version, or to allow an attacker to take advantage of a temporary mistake.

…

On Thu, May 30, 2019 at 12:37 PM W. Trevor King ***@***.***> wrote: I meant it to be a boolean. Schedule structure sounds good; I'll reroll tonight. I've pushed d3cfead <d3cfead> -> 0b9690b <0b9690b> to go with a boolean after all. Talking this over with @abhinavdahiya <https://github.com/abhinavdahiya> and @crawford <https://github.com/crawford>, I think getting a spec structure that makes most users happy is going to be hard, and it diverges from the "simple CVO, complex logic in Cincinnati" approach we've followed so far. With a boolean "should I auto-upgrade?" switch for the CVO, users that need more complicated logic can implement that by pointing clusters at their own Cincinnati proxy that slurps the graph from our upstream Cincinnati (or wherever) and applies the logic they want (e.g. removing all upgrade edges during times when they don't want clusters auto-upgrading). If we find there's a lot of demand for specific logic (like your cron schedule schema), we can ship images for Policy Engine Cincinnati proxies like that to make it easy for users to set them up locally. But we don't need to bake that logic into the cluster itself. Thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#326?email_source=notifications&email_token=AAI37JYTTYY4YG7CZXIBFALPX77D3A5CNFSM4HQTF7L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWS2PRI#issuecomment-497395653>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI37J6CS55RFJECPBZWD3DPX77D3ANCNFSM4HQTF7LQ> .

smarterclayton · 2019-05-31T08:54:36Z

/hold

…

On Thu, May 30, 2019 at 3:22 PM Clayton Coleman ***@***.***> wrote: I disagree. I think Cincinnati should be responsible for a single job - telling a cluster which versions it can update to. Given the disagreement, this needs to go to a design doc where we can describe the use cases we're trying to solve. For example, we should never allow a compromise of Cincinnati to force updates to a known bad version, or to allow an attacker to take advantage of a temporary mistake. On Thu, May 30, 2019 at 12:37 PM W. Trevor King ***@***.***> wrote: > I meant it to be a boolean. Schedule structure sounds good; I'll reroll > tonight. > > I've pushed d3cfead > <d3cfead> > -> 0b9690b > <0b9690b> > to go with a boolean after all. Talking this over with @abhinavdahiya > <https://github.com/abhinavdahiya> and @crawford > <https://github.com/crawford>, I think getting a spec structure that > makes most users happy is going to be hard, and it diverges from the > "simple CVO, complex logic in Cincinnati" approach we've followed so far. > With a boolean "should I auto-upgrade?" switch for the CVO, users that need > more complicated logic can implement that by pointing clusters at their own > Cincinnati proxy that slurps the graph from our upstream Cincinnati (or > wherever) and applies the logic they want (e.g. removing all upgrade edges > during times when they don't want clusters auto-upgrading). If we find > there's a lot of demand for specific logic (like your cron schedule > schema), we can ship images for Policy Engine Cincinnati proxies like that > to make it easy for users to set them up locally. But we don't need to bake > that logic into the cluster itself. Thoughts? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#326?email_source=notifications&email_token=AAI37JYTTYY4YG7CZXIBFALPX77D3A5CNFSM4HQTF7L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWS2PRI#issuecomment-497395653>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAI37J6CS55RFJECPBZWD3DPX77D3ANCNFSM4HQTF7LQ> > . >

crawford · 2019-06-01T15:31:53Z

I'll try to get my thoughts into a doc early next week.

crawford · 2019-06-08T01:17:29Z

I've got my proposed big picture down here: https://docs.google.com/document/d/1i4srAn3exEHWBjQmVE0Vz4V5gB8n7eOJi4MUm1rDhFo. ^{(sorry OKD. We'll be right back)}

I think we should adopt something similar to this:

apiVersion: config.openshift.io/v1
kind: ClusterVersion
spec:
  channel: stable-4.1
  clusterID: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
  upstream: https://api.openshift.com/api/upgrades_info/v1/graph
  automaticUpdates:
    schedule:
      - Mon..Fri *-2..10-* 10:00,14:00
      - Tues..Thurs *-11..1-* 11:00

schedule is a set of times that the CVO should check for updates (I don't have much preference for the actual syntax). An empty schedule implies that automatic updates should be disabled. We can leave everything more complex than that to a policy engine.

deads2k · 2019-09-25T18:03:19Z

/close

closing without prejudice due to age. Please open an enhancement in openshift/enhancements

openshift-ci-robot · 2019-09-25T18:03:21Z

@deads2k: Closed this PR.

Details

In response to this:

/close

closing without prejudice due to age. Please open an enhancement in openshift/enhancements

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.

cgwalters · 2019-10-10T01:02:10Z

Is it written down somewhere an example "best practice" for basic enablement of automatic updates today? Just something like a cron job pod that runs oc adm upgrade using the cli container? I think we should have this in the docs.

wking · 2019-11-19T21:57:53Z

Please open an enhancement in openshift/enhancements

Opened: openshift/enhancements#124

Just something like a cron job pod that runs oc adm upgrade using the cli container?

Probably oc adm upgrade --to-latest, but yeah, should work. I don't see a need to doc a stopgap though, we should just land the feature.

openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 29, 2019

openshift-ci-robot requested review from abhinavdahiya and adambkaplan May 29, 2019 19:48

abhinavdahiya reviewed May 29, 2019

View reviewed changes

config/v1/types_cluster_version.go Outdated Show resolved Hide resolved

smarterclayton suggested changes May 29, 2019

View reviewed changes

wking added 2 commits May 30, 2019 09:23

generated

0b9690b

With: $ make generate

wking force-pushed the auto-update-spec branch from d3cfead to 0b9690b Compare May 30, 2019 16:27

abhinavdahiya reviewed May 30, 2019

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2019

openshift-ci-robot closed this Sep 25, 2019

wking mentioned this pull request Nov 19, 2019

enhancements/update/automatic-updates: Propose a new enhancement openshift/enhancements#124

Closed

config/v1/types_cluster_version: Add ClusterVersionSpec.AutomaticUpdates #326

config/v1/types_cluster_version: Add ClusterVersionSpec.AutomaticUpdates #326

Uh oh!

Conversation

wking commented May 29, 2019

Uh oh!

openshift-ci-robot commented May 29, 2019

Uh oh!

Uh oh!

smarterclayton left a comment

Choose a reason for hiding this comment

Uh oh!

abhinavdahiya May 30, 2019

Choose a reason for hiding this comment

Uh oh!

wking May 30, 2019

Choose a reason for hiding this comment

Uh oh!

chancez Aug 31, 2019

Choose a reason for hiding this comment

Uh oh!

wking Aug 31, 2019

Choose a reason for hiding this comment

Uh oh!

wking commented May 30, 2019

Uh oh!

smarterclayton commented May 31, 2019 via email

Uh oh!

smarterclayton commented May 31, 2019 via email

Uh oh!

crawford commented Jun 1, 2019

Uh oh!

crawford commented Jun 8, 2019

Uh oh!

deads2k commented Sep 25, 2019

Uh oh!

openshift-ci-robot commented Sep 25, 2019

Uh oh!

cgwalters commented Oct 10, 2019

Uh oh!

wking commented Nov 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants