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

Add an experimental option to use libostree's "staging" API #1352

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

Now that infrastructure for this has landed in libostree,
let's make it easy for people to opt-in to testing it. This is a distinct first
step for adding it as an update policy.

@cgwalters
Copy link
Member Author

My test case:

$ make && env VMCHECK_FLAGS=stage-deployments make vmcheck TESTS=layering-relayer

@cgwalters
Copy link
Member Author

Right now though that option isn't enabled by default...one thing I was thinking is to pick one or two of the tests and turn that option on by default or something? layering-relayer is probably as good as any?

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Right now though that option isn't enabled by default...one thing I was thinking is to pick one or two of the tests and turn that option on by default or something?

Ahh I see; turning it on for all the tests won't work for those that check for /etc files etc... Hmm, though there can't be that many of those either.

Maybe let's start with upgrades, layering-basic, and layering-relayer?

@@ -5,3 +5,6 @@
[Daemon]
#AutomaticUpdatePolicy=none
#IdleExitTimeout=60

#[Experimental]
#StageDeployments=false
Copy link
Member

Choose a reason for hiding this comment

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

Do we even want to have it commented out in the baked config? My initial thoughts were to keep it hidden, though I'm open to document it this way.

gboolean is_staged = FALSE;
g_variant_dict_lookup (dict, "staged", "b", &is_staged);

if (opt_verbose && (is_staged || first))
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, let's drop the || first part? That way we can tell more easily if StageDeployments=true is active.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see how that'd help. For that we'd need to pass the boolean status from the daemon to the client, right?

The way the logic is now, if there is a pending deployment we'll always see whether or not it's staged, which was the main idea.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, right. I somehow read it as always printing yes. That makes sense.

Now that infrastructure for this has landed in libostree,
let's make it easy for people to opt-in to testing it.  This is a distinct first
step for adding it as an update policy.
@cgwalters
Copy link
Member Author

OK reworked this a bit and added a test that just does layering-relayer for now. Will look at extending to the rest of the suite over time.

@jlebon
Copy link
Member

jlebon commented May 2, 2018

Just gave it a spin here and it's working nicely! One thing that took me by surprise was that status showed three deployments before rebooting since the rollback was still there. I think that's what we want, though we'll need to remember to mention this in the release notes (and maybe a blog post) when we stabilize.

@rh-atomic-bot r+ bab391c

@rh-atomic-bot
Copy link

⚡ Test exempted: merge already tested.

cgwalters added a commit to jlebon/rpm-ostree that referenced this pull request May 4, 2018
Following up to coreos#1352
AKA 506910d
which added an experimental flag to globally enable deployment
staging, let's add an `ex-stage` automatic update policy.

I chose to create a new `test-autoupdate-stage.sh` and rename
the previous one to `test-autoupdate-check.sh` in going with
the previous theme of smaller test files; it's
way faster to iterate on new tests when it's a new file. And adding
staging at the top would have been weird.

This was all quite straightforward, just plumbing through lots
of layers.
cgwalters added a commit to cgwalters/ostree that referenced this pull request May 9, 2018
I feel like I'm drowning in a pile of experimental-but-almost-stable
features...

Anyways, since we made the feature opt-in in rpm-ostree in
coreos/rpm-ostree#1352
let's mirror that a bit here with an environment variable so people
can play with it more easily.

The tests needed some tweaks; specifically we need to reload the
status fact after making changes.  I'm still a bit uncertain
about the Ansible-as-tests.

But we add an upgrade test that uses the new environment variable.
cgwalters added a commit to cgwalters/ostree that referenced this pull request May 9, 2018
I feel like I'm drowning in a pile of experimental-but-almost-stable
features...

Anyways, since we made the feature opt-in in rpm-ostree in
coreos/rpm-ostree#1352
let's mirror that a bit here with an environment variable so people
can play with it more easily.

The tests needed some tweaks; specifically we need to reload the
status fact after making changes.  I'm still a bit uncertain
about the Ansible-as-tests.

But we add an upgrade test that uses the new environment variable.
rh-atomic-bot pushed a commit to ostreedev/ostree that referenced this pull request May 9, 2018
I feel like I'm drowning in a pile of experimental-but-almost-stable
features...

Anyways, since we made the feature opt-in in rpm-ostree in
coreos/rpm-ostree#1352
let's mirror that a bit here with an environment variable so people
can play with it more easily.

The tests needed some tweaks; specifically we need to reload the
status fact after making changes.  I'm still a bit uncertain
about the Ansible-as-tests.

But we add an upgrade test that uses the new environment variable.

Closes: #1583
Approved by: <try>
cgwalters added a commit to cgwalters/ostree that referenced this pull request May 10, 2018
I feel like I'm drowning in a pile of experimental-but-almost-stable
features...

Anyways, since we made the feature opt-in in rpm-ostree in
coreos/rpm-ostree#1352
let's mirror that a bit here with an environment variable so people
can play with it more easily.

The tests needed some tweaks; specifically we need to reload the
status fact after making changes.  I'm still a bit uncertain
about the Ansible-as-tests.

But we add an upgrade test that uses the new environment variable.
rh-atomic-bot pushed a commit to ostreedev/ostree that referenced this pull request May 11, 2018
I feel like I'm drowning in a pile of experimental-but-almost-stable
features...

Anyways, since we made the feature opt-in in rpm-ostree in
coreos/rpm-ostree#1352
let's mirror that a bit here with an environment variable so people
can play with it more easily.

The tests needed some tweaks; specifically we need to reload the
status fact after making changes.  I'm still a bit uncertain
about the Ansible-as-tests.

But we add an upgrade test that uses the new environment variable.

Closes: #1583
Approved by: jlebon
cgwalters added a commit to jlebon/rpm-ostree that referenced this pull request May 14, 2018
Following up to coreos#1352
AKA 506910d
which added an experimental flag to globally enable deployment
staging, let's add an `ex-stage` automatic update policy.

I chose to create a new `test-autoupdate-stage.sh` and rename
the previous one to `test-autoupdate-check.sh` in going with
the previous theme of smaller test files; it's
way faster to iterate on new tests when it's a new file. And adding
staging at the top would have been weird.

This was all quite straightforward, just plumbing through lots
of layers.
rh-atomic-bot pushed a commit that referenced this pull request May 14, 2018
Following up to #1352
AKA 506910d
which added an experimental flag to globally enable deployment
staging, let's add an `ex-stage` automatic update policy.

I chose to create a new `test-autoupdate-stage.sh` and rename
the previous one to `test-autoupdate-check.sh` in going with
the previous theme of smaller test files; it's
way faster to iterate on new tests when it's a new file. And adding
staging at the top would have been weird.

This was all quite straightforward, just plumbing through lots
of layers.

Closes: #1321
Approved by: jlebon
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.

None yet

3 participants