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.

Sign up for GitHub

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

Jump to bottom

Turn staged deployments on by default #1430

Closed
wants to merge 4 commits into from
Closed

Turn staged deployments on by default #1430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
414da27
tests: Remove dead `assert_status_jq`
cgwalters Aug 3, 2018
9094557
tests/vmcheck: Display human-readable status on jq failure
cgwalters Aug 3, 2018
a0de9ca
livefs: Require deployment staging
cgwalters Jul 13, 2018
aa26b20
Turn staged deployments on by default
cgwalters Jun 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ LIBS="$save_LIBS"
# Remember to update AM_CPPFLAGS in Makefile.am when bumping GIO req.
PKG_CHECK_MODULES(PKGDEP_GIO_UNIX, [gio-unix-2.0])
PKG_CHECK_MODULES(PKGDEP_RPMOSTREE, [gio-unix-2.0 >= 2.50.0 json-glib-1.0
ostree-1 >= 2018.2
ostree-1 >= 2018.7
libsystemd
polkit-gobject-1
rpm librepo libsolv
Expand Down Expand Up @@ -186,6 +186,14 @@ AS_IF([test x$enable_compose_tooling = xyes], [
])
if test x$enable_compose_tooling != xno; then RPM_OSTREE_FEATURES="$RPM_OSTREE_FEATURES compose"; fi

AC_ARG_ENABLE(staged,
AS_HELP_STRING([--disable-staged],
[Disable staged deployments by default]),,
[enable_staged=yes])
AS_IF([test x$enable_staged = xyes], [
AC_DEFINE(BUILDOPT_STAGE_DEPLOYMENTS, 1, [Define if we are staging deployments])
])

AC_ARG_ENABLE(rust,
AS_HELP_STRING([--enable-rust],
[Compile Rust features (e.g. compose tree --yaml)]),,
Expand Down
14 changes: 12 additions & 2 deletions src/daemon/rpmostreed-daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,8 +422,18 @@ rpmostreed_daemon_reload_config (RpmostreedDaemon *self,
return FALSE;
}

self->ex_stage_deployments = g_key_file_get_boolean (config, EXPERIMENTAL_CONFIG_GROUP,
"StageDeployments", NULL);
/* The default used to be `false`, so check for the key being present */
if (g_key_file_has_key (config, EXPERIMENTAL_CONFIG_GROUP, "StageDeployments", NULL))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just one release with that, and then we drop support for the key entirely? Or were you thinking of keeping around support for StageDeployments=false? I'm leaning more towards just not supporting turning it off.

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, agree.

Copy link
Member Author

Choose a reason for hiding this comment

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

In this PR there's a build-time option; @dustymabe was arguing that this should go into rawhide first or something. I dunno if that's really worth it. We have FAHC for automated testing before release.

Copy link
Member

Choose a reason for hiding this comment

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

yeah preferably it would go to rawhide/f29 first.. especially since we have silverblue, IoT, and FAH to consider.

Copy link
Member

Choose a reason for hiding this comment

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

No strong opinions either way. Seems relatively low risk at this point, but I'm fine with delaying f28 for one more release or so. (Maybe at the same time we drop support for the StageDeployments option completely),

self->ex_stage_deployments = g_key_file_get_boolean (config, EXPERIMENTAL_CONFIG_GROUP,
"StageDeployments", NULL);
else
{
#ifdef BUILDOPT_STAGE_DEPLOYMENTS
self->ex_stage_deployments = TRUE;
#else
self->ex_stage_deployments = FALSE;
#endif
}

/* don't update changed for this; it's contained to RpmostreedDaemon so no other objects
* need to be reloaded if it changes */
Expand Down
6 changes: 2 additions & 4 deletions src/daemon/rpmostreed-transaction-livefs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,8 @@ livefs_transaction_execute_inner (LiveFsTransaction *self,
*/
if (origin_merge_deployment == booted_deployment)
return glnx_throw (error, "No pending deployment");
if (!ostree_deployment_is_staged (origin_merge_deployment))
return glnx_throw (error, "livefs requires staged deployments");

/* Open a fd for the booted deployment */
g_autofree char *deployment_path = ostree_sysroot_get_deployment_dirpath (sysroot, booted_deployment);
Expand Down Expand Up @@ -877,10 +879,6 @@ livefs_transaction_execute_inner (LiveFsTransaction *self,
return FALSE;
}

/* XXX: right now we don't have a good solution for this:
* https://github.com/projectatomic/rpm-ostree/issues/40 */
rpmostree_output_message ("WARNING: any changes to /etc will be lost on next reboot");

/* Write out the origin as having completed this */
if (!write_livefs_state (sysroot, booted_deployment, NULL, target_csum, error))
return FALSE;
Expand Down
16 changes: 0 additions & 16 deletions tests/common/libtest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,22 +359,6 @@ skip_one_with_asan () {
fi
}

assert_status_file_jq() {
status_file=$1; shift
for expression in "$@"; do
if ! jq -e "${expression}" >/dev/null < $status_file; then
jq . < $status_file | sed -e 's/^/# /' >&2
echo 1>&2 "${expression} failed to match in $status_file"
exit 1
fi
done
}

assert_status_jq() {
rpm-ostree status --json > status.json
assert_status_file_jq status.json "$@"
}

get_obj_path() {
repo=$1; shift
csum=$1; shift
Expand Down
12 changes: 11 additions & 1 deletion tests/common/libvm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,19 @@ vm_assert_layered_pkg() {
fi
}

# Takes a list of `jq` expressions, each of which should evaluate to a boolean,
# and asserts that they are true.
vm_assert_status_jq() {
vm_rpmostree status --json > status.json
assert_status_file_jq status.json "$@"
vm_rpmostree status > status.txt
for expression in "$@"; do
if ! jq -e "${expression}" >/dev/null < status.json; then
jq . < status.json | sed -e 's/^/# /' >&2
echo 1>&2 "${expression} failed to match status.json"
cat status.txt
exit 1
fi
done
}

vm_pending_is_staged() {
Expand Down
5 changes: 5 additions & 0 deletions tests/vmcheck/test-layering-scripts.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ set -euo pipefail

set -x

# Uses livefs
vm_cmd 'echo "[Experimental]" >> /etc/rpm-ostreed.conf'
vm_cmd 'echo StageDeployments=true >> /etc/rpm-ostreed.conf'
vm_rpmostree reload

# SUMMARY: check that RPM scripts are properly handled during package layering

# do a bunch of tests together so that we only have to reboot once
Expand Down
13 changes: 11 additions & 2 deletions tests/vmcheck/test-livefs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ set -euo pipefail

set -x

# We do various assertions on deployment length, need a reliably
# clean slate.
vm_rpmostree cleanup -pr

vm_cmd 'echo "[Experimental]" >> /etc/rpm-ostreed.conf'
vm_cmd 'echo StageDeployments=true >> /etc/rpm-ostreed.conf'
vm_rpmostree reload

vm_assert_layered_pkg foo absent

vm_build_rpm foo
Expand All @@ -40,7 +48,8 @@ assert_livefs_ok() {
}
assert_livefs_ok

vm_assert_status_jq '.deployments|length == 2' '.deployments[0]["live-replaced"]|not' \
vm_assert_status_jq '.deployments|length == 2' \
'.deployments[0]["live-replaced"]|not' \
'.deployments[1]["live-replaced"]|not'
vm_rpmostree ex livefs
vm_cmd rpm -q foo > rpmq.txt
Expand Down Expand Up @@ -110,7 +119,7 @@ echo "ok livefs preserved rollback"

# Reset to rollback, undeploy pending
reset() {
vm_rpmostree rollback
vm_rpmostree ex reset
vm_reboot
vm_rpmostree cleanup -r
vm_assert_status_jq '.deployments|length == 1' '.deployments[0]["live-replaced"]|not'
Expand Down
4 changes: 2 additions & 2 deletions tests/vmcheck/test-meta-staged-layering-relayer.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ set -x
# This test suite enables the libostree staging feature, and executes
# the layering-relayer tests.
Copy link
Member

Choose a reason for hiding this comment

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

Seems like these comments should be updated now for s/enables/disables?


export VMCHECK_FLAGS=stage-deployments
export VMCHECK_FLAGS=not-stage-deployments
vm_cmd 'echo "[Experimental]" >> /etc/rpm-ostreed.conf'
vm_cmd 'echo StageDeployments=true >> /etc/rpm-ostreed.conf'
vm_cmd 'echo StageDeployments=false >> /etc/rpm-ostreed.conf'
vm_rpmostree reload

${dn}/test-layering-relayer.sh
Expand Down
4 changes: 2 additions & 2 deletions tests/vmcheck/test-meta-staged-upgrades.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ set -x
# This test suite enables the libostree staging feature, and executes
# the upgrade and layering-relayer tests.

export VMCHECK_FLAGS=stage-deployments
export VMCHECK_FLAGS=not-stage-deployments
vm_cmd 'echo "[Experimental]" >> /etc/rpm-ostreed.conf'
vm_cmd 'echo StageDeployments=true >> /etc/rpm-ostreed.conf'
vm_cmd 'echo StageDeployments=false >> /etc/rpm-ostreed.conf'
vm_rpmostree reload

${dn}/test-upgrades.sh
Expand Down
4 changes: 2 additions & 2 deletions tests/vmcheck/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ fi
if vm_cmd test -f /usr/etc/rpm-ostreed.conf; then
vm_cmd cp -f /usr/etc/rpm-ostreed.conf /etc
# Unless we're doing overrides
if [[ "${VMCHECK_FLAGS:-}" =~ "stage-deployments" ]]; then
if [[ "${VMCHECK_FLAGS:-}" =~ "not-stage-deployments" ]]; then
vm_cmd 'echo "[Experimental]" >> /etc/rpm-ostreed.conf'
vm_cmd 'echo StageDeployments=true >> /etc/rpm-ostreed.conf'
vm_cmd 'echo StageDeployments=false >> /etc/rpm-ostreed.conf'
fi
fi

Expand Down