Implement ex livefs --replace #1028

cgwalters · 2017-10-02T14:00:31Z

Lots of tradeoffs in this. See the comments in the code for
more info. WIP for now.

dustymabe · 2017-10-02T15:46:11Z

what does livefs --replace give us? wholesale replace of /usr/ ?

cgwalters · 2017-10-02T15:47:35Z

what does livefs --replace give us? wholesale replace of /usr/ ?

Yep.

cgwalters · 2017-10-02T15:53:37Z

Related: GNOME/libglnx#89

dustymabe · 2017-10-02T16:05:17Z

Yep.

just wondering.. what problem does that solve? just wondering why users might be interested in such a feature

cgwalters · 2017-10-02T16:13:09Z

Some of the use cases here go into what's currently covered by ostree admin unlock. The problem with admin unlock is rpm-ostree has no idea what's going on anymore (and the changes don't persist).
Let's say I want to test an ex override replace docker live; going down this path leaves /usr still appearing to be read-only, rpm-ostree is still in full control, and it is persistent.

Another case is doing quick testing of e.g. a FAH release update live. Yes, you don't get the new kernel, but OTOH you can do some things quickly. If things go wrong, you have the rollback.

In general, supporting this goes much farther down the path of having an option to do live updates where possible.

jlebon · 2017-10-02T16:46:04Z

Let's say I want to test an ex override replace docker live; going down this path leaves /usr still appearing to be read-only, rpm-ostree is still in full control, and it is persistent.

Hmm, TBH I'm still leaning towards either (1) not including the livefs deployment in the boot entries and/or (2) make the livefs deployment as "ephemeral" as possible. (Those were discussed in #639). Why do we want the changes to be persistent? The usefulness of livefs is that you essentially delay a reboot, but if you're rebooting anyway, we want to encourage users to go to either the pending deployment or the rollback, right? Esp. that we have override replace now, you should be able to move forward while avoiding a buggy package if needed.

Anyway, the above isn't directly related to this PR, which looks sane at a quick glance (though maybe we should rename the switch to --hold-my-beer-and-replace-usr).

cgwalters · 2017-10-02T19:07:09Z

I completely agree that it's broken to boot into something that says it was live-modified. The whole livefs thing is clearly going to go through a lot of changes before we lift the ex flag. It's not clear to me that it even makes sense as a separate command so much as a "mode".

Anyways back to this PR; if I'm doing override replace, I think in a lot of cases I want that to be persistent (assuming it works!) I may also want it to be live to test it (which becomes a bit circular ∞, yes).

Lots of tradeoffs in this. See the comments in the code for more info. WIP for now.

cgwalters · 2017-10-03T21:56:29Z

OK, I added a basic sanity test. Lifting WIP, though this could still use more tests.

cgwalters · 2017-10-04T13:26:16Z

Rawr.

vm_cmd test -f /usr/share/licenses/ostree/COPYING

C7 doesn't have the license bits standardized. Will hack around this.

cgwalters · 2017-10-04T14:06:17Z

Fixed up.

jlebon · 2017-10-04T19:11:44Z

src/daemon/rpmostreed-transaction-livefs.c

+        break;
+      const char *name = dent->d_name;
+      /* Keep track of what entries are in the new /usr */
+      g_hash_table_add (seen_new_children, g_strdup (name));


We're leaking this, right? Hash table should probably be g_hash_table_new_full (..., ..., g_free, NULL)

jlebon · 2017-10-04T19:14:16Z

src/daemon/rpmostreed-transaction-livefs.c

+ * all of it. We could do a diff, but doing that precisely and quickly depends
+ * on https://github.com/ostreedev/ostree/issues/1224
+ *
+ * In this approach, we iterate over and exchange subdirectories. Some issues


If we're not recursing, let's clarify that it's only first-level subdirectories of /usr.

jlebon · 2017-10-04T19:18:48Z

src/daemon/rpmostreed-transaction-livefs.c

@@ -522,6 +522,106 @@ replace_subpath (OstreeRepo *repo,
  return TRUE;
 }

+/* The sledgehammer 🔨 approach. Because /usr is a mount point, we can't replace
+ * all of it. We could do a diff, but doing that precisely and quickly depends
+ * on https://github.com/ostreedev/ostree/issues/1224


Hmm, it's true, we are talking about a lot of fdopendir/readdir/stat calls, though I do still wonder how bad it would be.

Actually, that reminds me of cgwalters#2, in which I measured adding a stat penalty on a real compose to be around ~60 ms. Of course, it'd be much larger, but even so, even if it's say, 1 second, it seems like an OK tradeoff considering the added safety? E.g. I would be very hesitant to ever use this feature on my Atomic Workstation; feels a bit too violent. ⚡️ 🔥 💻 😱

jlebon · 2017-10-04T19:24:27Z

tests/vmcheck/test-livefs.sh

+assert_file_has_content livefs-analysis.txt 'livefs OK (dry run)'
+vm_assert_status_jq '.deployments|length == 2' '.deployments[0]["live-replaced"]|not' \
+                    '.deployments[1]["live-replaced"]|not'
+vm_rpmostree ex livefs --replace


Let's add another vm_assert_status_jq here to confirm it's what we expect?

jlebon · 2017-10-04T19:25:52Z

tests/vmcheck/test-livefs.sh

-(cat vmcheck/usr/bin/ls; echo more stuff) > vmcheck/usr/bin/ls.new
-chmod a+x vmcheck/usr/bin/ls.new
-mv vmcheck/usr/bin/ls{.new,}
+(date; echo "JUST KIDDING DO WHATEVER") >vmcheck/${dummy_file_to_modify}.new && mv vmcheck/${dummy_file_to_modify}{.new,}


As a corner-case, maybe also mkdir vmcheck/usr/newdir and check that it was created later on down there?

jlebon · 2017-10-05T13:19:18Z

Hmm, weird. Not sure what's going on with the CentOS one. Let's give it a try.
@rh-atomic-bot r+ 8e941dd

rh-atomic-bot · 2017-10-05T13:19:22Z

⌛ Testing commit 8e941dd with merge 95227f0...

rh-atomic-bot · 2017-10-05T13:41:20Z

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing 95227f0 to master...

cgwalters added the WIP label Oct 2, 2017

cgwalters added 2 commits October 3, 2017 14:37

WIP: Implement ex livefs --replace

5f9c03b

Lots of tradeoffs in this. See the comments in the code for more info. WIP for now.

fixup! WIP: Implement ex livefs --replace

9859f8d

cgwalters force-pushed the livefs-all branch from 1166c28 to 9859f8d Compare October 3, 2017 21:55

cgwalters changed the title ~~WIP: Implement ex livefs --replace~~ Implement ex livefs --replace Oct 3, 2017

cgwalters removed the WIP label Oct 3, 2017

fixup! WIP: Implement ex livefs --replace

a335f4f

jlebon reviewed Oct 4, 2017

View reviewed changes

cgwalters added 2 commits October 4, 2017 17:04

fixup! WIP: Implement ex livefs --replace

f0450cf

fixup! WIP: Implement ex livefs --replace

8e941dd

jlebon added the reviewed/approved label Oct 5, 2017

rh-atomic-bot closed this in 95227f0 Oct 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement ex livefs --replace #1028

Implement ex livefs --replace #1028

cgwalters commented Oct 2, 2017

dustymabe commented Oct 2, 2017

cgwalters commented Oct 2, 2017

cgwalters commented Oct 2, 2017

dustymabe commented Oct 2, 2017

cgwalters commented Oct 2, 2017

jlebon commented Oct 2, 2017

cgwalters commented Oct 2, 2017

cgwalters commented Oct 3, 2017

cgwalters commented Oct 4, 2017

cgwalters commented Oct 4, 2017

jlebon Oct 4, 2017

jlebon Oct 4, 2017

jlebon Oct 4, 2017

jlebon Oct 4, 2017

jlebon Oct 4, 2017

jlebon commented Oct 5, 2017

rh-atomic-bot commented Oct 5, 2017

rh-atomic-bot commented Oct 5, 2017

Implement ex livefs --replace #1028

Implement ex livefs --replace #1028

Conversation

cgwalters commented Oct 2, 2017

dustymabe commented Oct 2, 2017

cgwalters commented Oct 2, 2017

cgwalters commented Oct 2, 2017

dustymabe commented Oct 2, 2017

cgwalters commented Oct 2, 2017

jlebon commented Oct 2, 2017

cgwalters commented Oct 2, 2017

cgwalters commented Oct 3, 2017

cgwalters commented Oct 4, 2017

cgwalters commented Oct 4, 2017

jlebon Oct 4, 2017

Choose a reason for hiding this comment

jlebon Oct 4, 2017

Choose a reason for hiding this comment

jlebon Oct 4, 2017

Choose a reason for hiding this comment

jlebon Oct 4, 2017

Choose a reason for hiding this comment

jlebon Oct 4, 2017

Choose a reason for hiding this comment

jlebon commented Oct 5, 2017

rh-atomic-bot commented Oct 5, 2017

rh-atomic-bot commented Oct 5, 2017