upgrade: Add support for --pull-only and --deploy-only #642

cgwalters · 2017-01-05T16:34:35Z

This makes it easier to script downloading updates in the background,
and only do deployments just before rebooting.

Partially addresses #640

cgwalters · 2017-01-05T16:35:00Z

I didn't test this locally yet, and would definitely need test suite additions.

cgwalters · 2017-01-05T16:36:30Z

One major piece this is also missing is that --deploy --reboot will always do a deployment and reboot, even if there are no updates.

paulvt · 2017-01-05T16:51:43Z

I'd say that --reboot conflicts with --pull-only (why would you do that?) and also with --deploy-only.
I see --reboot as a mechanism for rebooting conditionallly when something was pulled and then deployed. (Same as I wrongly considered ostree admin upgrade to pull and deploy if and only if something was pulled.)

cgwalters · 2017-01-05T17:03:48Z

Okay, took a crack at fixing the gap by adding a "synthetic" pull flag that's used with --deploy-only. This gets us the proper changed handling, so ostree admin upgrade --deploy-only will say "No changes" if there are no pending updates.

(I still didn't test this much sorry, typing some code here while waiting for some tests to run)

This makes it easier to script downloading updates in the background, and only do deployments just before rebooting. Partially addresses ostreedev#640

cgwalters · 2017-02-13T21:04:46Z

Rebased 🏄, now with docs 📚 and tests ✅.

cgwalters · 2017-02-13T21:05:00Z

Removing WIP.

jlebon · 2017-02-24T22:15:26Z

src/libostree/ostree-sysroot-upgrader.h

@@ -85,7 +85,8 @@ gboolean ostree_sysroot_upgrader_check_timestamps (OstreeRepo     *repo,

 typedef enum {
  OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_NONE = 0,
-  OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_ALLOW_OLDER = (1 << 0)
+  OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_ALLOW_OLDER = (1 << 0),
+  OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_SYNTHETIC = (1 << 1) /* Don't actually do a pull, just check timestamps/changed */


Do we need this flag though? Seems like it would be cleaner to just seed new_revision with the refspec on upgrader init (similar to what we do in rpm-ostree) and then just not call ostree_sysroot_upgrader_pull() at all, no?

The equivalent of RPMOSTREE_TRANSACTION_DEPLOY_FLAG_NO_PULL_BASE? So...yeah, I guess we could do that, but I like keeping the timestamp check in there, and I'd argue we should probably do the same in rpm-ostree.

What if e.g. someone did a raw ostree pull --commit-metadata-only or something, which won't do timestamp checks and hence could be more easily MITM'd - then later an admin does ostree admin upgrade --deploy-only, we'd be skipping the timestamp check.

jlebon · 2017-02-24T22:18:15Z

src/ostree/ot-admin-builtin-upgrade.c

+   * compatiblity, we only do cleanup if a user specifies the new --pull-only
+   * option.  Otherwise, we would break the case of trying to deploy a commit
+   * that isn't directly referenced.
+   */


I'm still not sure I understand this. Is there any risk to leaving the intermediate data until the cleanup that happens after deployment? Isn't this making us lose potential static delta jumps between consecutive --pull-only operations?

If a pull is successful, it will set the ref. Let's call this the "new pulled base", as distinct from creating a new deployment. ostree_sysroot_cleanup won't reset the ref.

Hence we should (though this isn't explicitly tested right now) keep deltas, since we still have the ref. Now, that's all the argument for why this won't break anything.

Why I wanted to do this is - think about a scenario where due to a flaky internet connection we do a pull, it gets interrupted, do another pull, it gets interrupted, repeat. If this happens a lot, we will endlessly accumulate partial new commits.

With this change, we'll trigger the one-day-timeout on staged commit data. Now...perhaps we should really do the cleanup after an unsuccessful pull, to ensure that we will always reuse downloaded-but-not-consumed deltas? Will look at that.

Moved some of that to #713

jlebon · 2017-02-27T20:57:21Z

Looks good!
@rh-atomic-bot r+ 7e9cd67

rh-atomic-bot · 2017-02-27T20:57:26Z

⌛ Testing commit 7e9cd67 with merge 36b28cb...

rh-atomic-bot · 2017-02-27T21:12:05Z

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing 36b28cb to master...

@cgwalters

As @cgwalters mentioned in coreos#1035, the new `--cache-only` implemented only the rpmmd half of the story. Here we complete that story by also ensuring that when in cache-only mode, we don't download new ostree data nor new packages. We try to complete the requested operation with what we have. To do this, we add support for the same `SYNTHETIC` pull that was added in ostree[1] so that we don't actually pull, but still perform timestamp checking. On the pkgcache side, we disable all remote repos and instead insert all our cached RPMs into the `DnfSack`. Care is taken to still perform SHA256 verification for local pkg installs/replacements. [1] ostreedev/ostree#642

As Colin mentioned in coreos#1035, the new `--cache-only` implemented only the rpmmd half of the story. Here we complete that story by also ensuring that when in cache-only mode, we don't download new ostree data nor new packages. We try to complete the requested operation with what we have. To do this, we add support for the same `SYNTHETIC` pull that was added in ostree[1] so that we don't actually pull, but still perform timestamp checking. On the pkgcache side, we disable all remote repos and instead insert all our cached RPMs into the `DnfSack`. Care is taken to still perform SHA256 verification for local pkg installs/replacements. [1] ostreedev/ostree#642

As Colin mentioned in coreos#1035, the new `--cache-only` implemented only the rpmmd half of the story. Here we complete that story by also ensuring that when in cache-only mode, we don't download new ostree data nor new packages. We try to complete the requested operation with what we have. To do this, we add support for the same `SYNTHETIC` pull that was added in ostree[1] so that we don't actually pull, but still perform timestamp checking. On the pkgcache side, we disable all remote repos and instead insert all our cached RPMs into the `DnfSack`. Care is taken to still perform SHA256 verification for local pkg installs/replacements. [1] ostreedev/ostree#642 Closes: coreos#687

As Colin mentioned in #1035, the new `--cache-only` implemented only the rpmmd half of the story. Here we complete that story by also ensuring that when in cache-only mode, we don't download new ostree data nor new packages. We try to complete the requested operation with what we have. To do this, we add support for the same `SYNTHETIC` pull that was added in ostree[1] so that we don't actually pull, but still perform timestamp checking. On the pkgcache side, we disable all remote repos and instead insert all our cached RPMs into the `DnfSack`. Care is taken to still perform SHA256 verification for local pkg installs/replacements. [1] ostreedev/ostree#642 Closes: #687 Closes: #1049 Approved by: cgwalters

cgwalters mentioned this pull request Jan 5, 2017

question: ostree admin upgrade does it twice #640

Closed

cgwalters force-pushed the upgrade-split-pull-deploy branch from 47cfd4a to 22810f4 Compare January 5, 2017 17:01

cgwalters added the WIP label Jan 18, 2017

cgwalters mentioned this pull request Feb 4, 2017

Add pending-base-commit to status coreos/rpm-ostree#595

Closed

upgrade: Add support for --pull-only and --deploy-only

47f08ff

This makes it easier to script downloading updates in the background, and only do deployments just before rebooting. Partially addresses ostreedev#640

cgwalters force-pushed the upgrade-split-pull-deploy branch from 00aa835 to 47f08ff Compare February 13, 2017 21:04

cgwalters removed the WIP label Feb 13, 2017

jlebon reviewed Feb 24, 2017

View reviewed changes

cgwalters mentioned this pull request Feb 27, 2017

More intelligent pruning of tmp/ #713

Open

fixup! upgrade: Add support for --pull-only and --deploy-only

7e9cd67

rh-atomic-bot closed this in 36b28cb Feb 27, 2017

cgwalters mentioned this pull request Mar 29, 2017

rpm-ostree pull 🖧 coreos/rpm-ostree#713

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrade: Add support for --pull-only and --deploy-only #642

upgrade: Add support for --pull-only and --deploy-only #642

cgwalters commented Jan 5, 2017

cgwalters commented Jan 5, 2017

cgwalters commented Jan 5, 2017 •

edited

Loading

paulvt commented Jan 5, 2017

cgwalters commented Jan 5, 2017 •

edited

Loading

cgwalters commented Feb 13, 2017

cgwalters commented Feb 13, 2017

jlebon Feb 24, 2017

cgwalters Feb 27, 2017

jlebon Feb 24, 2017

cgwalters Feb 27, 2017

cgwalters Feb 27, 2017

jlebon commented Feb 27, 2017

rh-atomic-bot commented Feb 27, 2017

rh-atomic-bot commented Feb 27, 2017

upgrade: Add support for --pull-only and --deploy-only #642

upgrade: Add support for --pull-only and --deploy-only #642

Conversation

cgwalters commented Jan 5, 2017

cgwalters commented Jan 5, 2017

cgwalters commented Jan 5, 2017 • edited Loading

paulvt commented Jan 5, 2017

cgwalters commented Jan 5, 2017 • edited Loading

cgwalters commented Feb 13, 2017

cgwalters commented Feb 13, 2017

jlebon Feb 24, 2017

Choose a reason for hiding this comment

cgwalters Feb 27, 2017

Choose a reason for hiding this comment

jlebon Feb 24, 2017

Choose a reason for hiding this comment

cgwalters Feb 27, 2017

Choose a reason for hiding this comment

cgwalters Feb 27, 2017

Choose a reason for hiding this comment

jlebon commented Feb 27, 2017

rh-atomic-bot commented Feb 27, 2017

rh-atomic-bot commented Feb 27, 2017

cgwalters commented Jan 5, 2017 •

edited

Loading

cgwalters commented Jan 5, 2017 •

edited

Loading