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

Update all {F,RH}CoreOS systems that have an aleph version #104

Merged
merged 1 commit into from
Nov 13, 2020

Conversation

cgwalters
Copy link
Member

Came up in review in coreos/fedora-coreos-docs#203
Basically it's confusing to users that they need to understand
the difference between update and adopt-and-update.

From bootupd's perspective these are quite different things;
the first case is a completely known quantity, the second isn't.

However...we can be pretty confident that we can update systems
that have a CoreOS aleph version (i.e. they were generated by coreos-assembler),
since we haven't changed how the bootloader is installed there
really much at all.

This means that for now RHCOS 4.{1,2} bootimages that were
generated via Anaconda will still require adopt-and-update,
but detecting and validating that can come as a second phase.

The high level logic here is that the status gains a new
confident: bool flag (which corresponds right now to
"have CoreOS aleph"). The client side looks at this
and automatically bridges the update CLI to
adopt-and-update.

Closes: #103

Came up in review in coreos/fedora-coreos-docs#203
Basically it's confusing to users that they need to understand
the difference between `update` and `adopt-and-update`.

From bootupd's perspective these are quite different things;
the first case is a completely known quantity, the second isn't.

However...we can be pretty confident that we can update systems
that have a CoreOS aleph version (i.e. they were generated by coreos-assembler),
since we haven't changed how the bootloader is installed there
really much at all.

This means that for now RHCOS 4.{1,2} bootimages that were
generated via Anaconda will still require `adopt-and-update`,
but detecting and validating that can come as a second phase.

The high level logic here is that the status gains a new
`confident: bool` flag (which corresponds right now to
"have CoreOS aleph").  The client side looks at this
and automatically bridges the `update` CLI to
`adopt-and-update`.

Closes: coreos#103
@cgwalters
Copy link
Member Author

OK, this one has green CI.

Copy link
Contributor

@lucab lucab left a comment

Choose a reason for hiding this comment

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

Patch looks good, but I do have a doubt regarding a (possibly unintended?) breaking change.

@@ -106,7 +116,7 @@ pub(crate) struct Status {
/// Maps a component name to status
pub(crate) components: BTreeMap<String, ComponentStatus>,
/// Components that appear to be installed, not via bootupd
pub(crate) adoptable: BTreeMap<String, ContentMetadata>,
pub(crate) adoptable: BTreeMap<String, Adoptable>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this change breaks the "stable format" premise stated in the docstring, but current CI does not catch this because the entry/map in example-status-v0.json fixture is empty.

Is that a problem, or is this breaking change still ok? To my understanding this data-structure is not persisted to disk, and the change only affects client compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is a valid concern and you're right this sneaks by our tests today.

In the end, while we have bootupctl status --json I think the project is still in a "stabilizing" state - there aren't any known consumers of the JSON yet.

Hmm, probably we should add a simple "is up to date" API, since that's all that most consumers would care about.

@lucab
Copy link
Contributor

lucab commented Nov 13, 2020

/hold
/lgtm

Leaving this pre-stamped in case the breaking change was intended and confirmed ok.

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, lucab

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

The pull request process is described here

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

@cgwalters
Copy link
Member Author

Will address that in followup
/hold cancel

@openshift-merge-robot openshift-merge-robot merged commit e00de4a into coreos:master Nov 13, 2020
cgwalters added a commit to cgwalters/bootupd that referenced this pull request Nov 13, 2020
openshift-merge-robot pushed a commit that referenced this pull request Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't require adoption for FCOS
4 participants