Skip to content
This repository has been archived by the owner on Apr 19, 2019. It is now read-only.

test build with meta-swupd and new images #25

Closed
wants to merge 194 commits into from

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Mar 23, 2016

This imports meta-swupd, enables swupd support, and builds ostro-image-reference and ostro-image-qa instead of ostro-image and ostro-image-dev.

@pohly
Copy link
Contributor Author

pohly commented Mar 23, 2016

The PR depends on a CI change that enables changing the build targets as described in b773681

The server-side of that change hasn't been enabled yet.

@pohly
Copy link
Contributor Author

pohly commented Mar 23, 2016

retest this please

CI change in place now, to be tested by this PR.

@pohly
Copy link
Contributor Author

pohly commented Mar 26, 2016

retest this please

@pohly pohly force-pushed the meta-swupd3 branch 7 times, most recently from 95e0526 to dc2e20a Compare March 28, 2016 13:09
# Following targets would be used to perform default build task:
OSTROPROJECT_CI_BUILD_TARGETS="${DISTRO}-image ${DISTRO}-image-dev"
# Following targets would be used to perform default build task.
OSTROPROJECT_CI_BUILD_TARGETS="ostro-image ostro-image-qa ostro-image-reference"
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to have ${DISTRO} here instead of hardcoded ostro.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Tue, 2016-03-29 at 01:21 -0700, Alexander D. Kanevskiy wrote:

In meta-ostro/conf/distro/include/ostroproject-ci.inc:

@@ -44,14 +48,25 @@ OSTRO_VM_IMAGE_TYPES = "dsk.xz dsk.zip dsk.vdi.zip"

which must contain only alphanumeric symbols,'-' and '_'.

Any other symbols would be skipped in parser.

-# Following targets would be used to perform default build task:
-OSTROPROJECT_CI_BUILD_TARGETS="${DISTRO}-image ${DISTRO}-image-dev"
+# Following targets would be used to perform default build task.
+OSTROPROJECT_CI_BUILD_TARGETS="ostro-image ostro-image-qa ostro-image-reference"

I prefer to have ${DISTRO} here instead of hardcoded ostro.

That would have the drawback of not being able to grep for image names
when looking for them. For example, when I more recently replaced
ostro-image-qa with ostro-image-dev, I could use grep and be reasonably
sure I covered it.

I could imagine that someone might want to fork Ostro OS and then the
DISTRO variable would make some sense, but this is our Ostro OS CI
configuration - I just don't see that advantage outweighing the
disadvantage mentioned above.

Copy link
Contributor

Choose a reason for hiding this comment

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

git grep '${DISTRO}-image'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Wed, 2016-03-30 at 01:43 -0700, Alexander D. Kanevskiy wrote:

In meta-ostro/conf/distro/include/ostroproject-ci.inc:

@@ -44,14 +48,25 @@ OSTRO_VM_IMAGE_TYPES = "dsk.xz dsk.zip dsk.vdi.zip"

which must contain only alphanumeric symbols,'-' and '_'.

Any other symbols would be skipped in parser.

-# Following targets would be used to perform default build task:
-OSTROPROJECT_CI_BUILD_TARGETS="${DISTRO}-image ${DISTRO}-image-dev"
+# Following targets would be used to perform default build task.
+OSTROPROJECT_CI_BUILD_TARGETS="ostro-image ostro-image-qa ostro-image-reference"

git grep '${DISTRO}-image'

Someone who doesn't know about ostroproject-ci.inc will also not know
that he has to grep with that term.

@pohly pohly force-pushed the meta-swupd3 branch 2 times, most recently from 64a7495 to 99c1761 Compare March 30, 2016 06:17
@pohly
Copy link
Contributor Author

pohly commented Mar 30, 2016

retest this please

1 similar comment
@pohly
Copy link
Contributor Author

pohly commented Mar 30, 2016

retest this please

@pohly
Copy link
Contributor Author

pohly commented Mar 30, 2016

retest this please

CI is currently unreliable due to loss of connections between Jenking master and slaves, hence the need to retrigger testing.

@pohly
Copy link
Contributor Author

pohly commented Mar 30, 2016

@incandescant @rojkov I've merged the BSD tar patches and on top of that added some more patches for meta-swupd:

c353a2c swupdbundle.bbclass: support -dev bundles
735e66e swupd-image.bbclass: remove assumption about OS version numbering
270fee3 swupd-server: work around pseudo xattr hardlink bug

@incandescant please review those and get ready to merge everything that is pending for meta-swupd. CI builds are still unreliable as far as I can tell, so it is uncertain whether we'll get CI results soonish.

@incandescant
Copy link
Contributor

@pohly I'm very happy with the meta-swupd changes and will be happy to land them upstream once those minor changes are done.

Note: I pushed a similar fix (almost identical, bar the commit message) to 15c47f9 upstream before I saw you'd fixed here — sorry.

@pohly
Copy link
Contributor Author

pohly commented Mar 30, 2016

@incandescant I fixed the d.appendVar() and added lnr as suggested. Force-pushed the updated commits.

@incandescant
Copy link
Contributor

meta-swupd has been updated. I think I got all of the patches from this series?

http://git.yoctoproject.org/cgit/cgit.cgi/meta-swupd

@pohly
Copy link
Contributor Author

pohly commented Mar 31, 2016

retest this please

@pohly
Copy link
Contributor Author

pohly commented Mar 31, 2016

@incandescant @rojkov I changed the way how OS_VERSION gets handled. I wanted to ensure that local builds do not suffer from constant rebuilds when using the default Ostro value for OS_VERSION, which is derived from DATETIME and thus constantly changes.

New commits are: ffa7c21 fdde7d6 5da446a

require conf/distro/include/ostro-os-development.inc
OSTRO_DEVELOPMENT_EXTRA_FEATURES = ""
OSTRO_DEVELOPMENT_EXTRA_INSTALL = ""

Copy link
Contributor

Choose a reason for hiding this comment

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

This creates extra divergence from default images built by user. Not good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you checked my proposal for building non-swupd and swupd images from the same build configuration? See my mail from today in the "integrating swupd support into image creation" mail thread.

That solution will remove this CI config change.

pohly and others added 27 commits April 7, 2016 20:43
BUILD_ID is only useful in the CI. Setting OS_VERSION based on it
therefore belongs into ostroproject-ci.inc.

During a local build, we can explicitly use ${DATETIME} to have a
reasonable default if developers do not set OS_VERSION manually. The
default is to use a modified OS_VERSION if something else triggers an
image build, but a OS_VERSION change itself won't trigger an image and
bundle build.

If a rebuild is desired without other changes, a developer can enforce
it by setting VERSION_ID in local.conf.

Signed-off-by: Patrick Ohly <[email protected]>
As with BUILD_ID, OS_VERSION must not depend on DATETIME to avoid task hash calculation problems.

Signed-off-by: Patrick Ohly <[email protected]>
Both systemd and the efi_combo_updater have problems when
"mount" is provided by busybox: systemd fails to remount
the rootfs read/write and the updater segfaults because
it does not parse the output correctly.

For now avoid these problems by sticking to the traditional
mount utilities from util-linux.

Signed-off-by: Patrick Ohly <[email protected]>
Including BUILD_ID only makes sense in the CI, because during local
builds it is set to a basically unpredictable value (${DATETIME}) and
does not get updated in os-release during later builds because of the
implications (constant rebuilds, inconsistencies between different
machines).

This change also documents what the BUILD_ID in our CI system is.

Signed-off-by: Patrick Ohly <[email protected]>
According to review feedback from Sasha, BUILD_ID not
set by the CI system and thus cannot be relied to stay
as it is now (?).

Thus moved the explanation of what it is expected to contain
to the OS_VERSION variable.

Signed-off-by: Patrick Ohly <[email protected]>
…me configuration

This makes it possible to build both in the CI system and simplifies
usage during local builds, because developers can switch from one kind
of image to another without having to change their local.conf.

The new ostro-image-swupd.bb is the base image for the virtual
ostro-image-swupd-reference and ostro-image-swupd-dev where swupd is
enabled. ostro-image-swupd does not produce image files, because those
would not be useful (too minimal).

Most of the code from ostro-image.bb was moved to ostro-image.bbclass
to allow sharing it with ostro-image-swupd.bb. Renaming ostro-image.bb
to something like ostro-image-swupd.bb would make sense, but cannot be
done because code elsewhere (for example, in meta-iotqa) still depends
on an image recipe called "ostro-image".

The "ostro-image" base name is currently reserved by the CI testing
system for the image which is to be used for automated
testing. Therefore and for the sake of consistency with the intended
naming, IMAGE_BASENAME gets overridden when building "ostro-image".
"ostro-image-noswupd" is also possible because of an alias set
via PROVIDES.

The CI gets configured to build ostro-image-noswupd,
ostro-image-swupd-reference and ostro-image-swupd-dev. CI sanity
testing happens on ostro-image-swupd-dev.

Signed-off-by: Patrick Ohly <[email protected]>
Recording volatile information in the os-release package is
conceptually problematic: either the package gets updated during each
build and thus causes constant image rebuilds, or it does not get
updated and then old, incorrect information gets copied into images.

In Ostro OS, we do not have to keep packaged files unmodified during
image creation, so the solution is to record static strings in the
os-release package and replace that with the actual, current values
during image creation.

This now works in the CI and in local builds, therefore recording
BUILD_ID can be enabled for both.

Signed-off-by: Patrick Ohly <[email protected]>
There is a standard OE-core variable for the first part of the file
name used for symlinks to the latest image file. The dsk image code
should use that instead of hard-coding ${BPN}-${MACHINE}, because then
it automatically does the right thing in image recipes which modify
IMAGE_BASENAME.

Signed-off-by: Patrick Ohly <[email protected]>
…mage creation

The previous approach depended on recompiling packages when
Ostro-specific update-alternative priorities had to be changed. That
breaks sstate re-usage. The new solution avoids that by introducing
new shell variables into all packages using
update-alternatives.bbclass and then setting these shell variables
during image creation.

Signed-off-by: Patrick Ohly <[email protected]>
We prefer utils-linux over Toybox/Busybox. That implies that we must
have it installed in the os-core, to avoid dangling symlinks when only
additional bundles pull it in.

Done unconditionally also in non-swupd images for the sake of
consistency.

Signed-off-by: Patrick Ohly <[email protected]>
The EFI combo update works only on dsk images. Therefore it should only get
installed and activated on architectures using the dsk image format, and that
choice should be made in a central place like ostro.conf.

For this to work, the .service file which hooks the efi combo updater
into cannot be in the allarch oe-swupd-helpers, because that must have
the same content regardless of the architecture. It seems simpler to
do it all in a single .bbappend anyway, so now
efi-combo-trigger.service is part of the swupd-client.bbappend, just
like the efi-combo-trigger binary itself.

A side effect is that developers can override that choice by setting
the new OSTRO_USE_DSK_IMAGES, which may be useful for debugging.

Signed-off-by: Patrick Ohly <[email protected]>
Builds are too slow in the CI to be useful (> 3 hours). Performance
enhancements are possible and planned, but not available yet. As the
initial goal is primarily about whole image update with bundle support
as stretch goal, reducing the number of bundles to just those that are
needed to build the example images is acceptable and will reduce
build times again.

Signed-off-by: Patrick Ohly <[email protected]>
The meta-swupd swupd-client does not depend on /bin/sh -> bash anymore,
so we can remove our workaround.

Signed-off-by: Patrick Ohly <[email protected]>
It's up to user to decide if `swupd check-update` needs to
run periodically. By default the periodic job should be disabled.

Signed-off-by: Dmitry Rozhkov <[email protected]>
The efi-combo-update.service must be explicitly enabled (was disabled
in the base recipe). For that to work we must remove the base recipe's
check-update.timer/service completely.

Signed-off-by: Patrick Ohly <[email protected]>
Command line tools were all referenced by absolute paths instead of
relying on PATH. Ostro OS does no specify where exactly each tool is
located, and in the case of sed that changes depending on whether it
comes from Busybox or Toybox, so better rely on searching via PATH.

Signed-off-by: Patrick Ohly <[email protected]>
When replacing busybox with toybox (to be committed separately),
several commands will disappear from the base image (ping, top, ps,
vi, etc.).

This is acceptable for a base image, but not for something that people
use interactively or the QA test scripts. Therefore a new
"tools-interactive" image feature gets introduced:
- it is active in all image variants at the moment
- the corresponding package group installs commands not provided
  anymore by toybox, choosing the corresponding individual
  packages instead
- bash is installed but not used as /bin/sh, which always points
  to dash

The goal is to ensure consistent behavior, regardless whether
coreutils, busybox or toybox are installed.

Signed-off-by: Patrick Ohly <[email protected]>
The initial commit of packagegroup-tools-interactive.bb only had the
essentials. To make interactive use more convenient, we can and should
also include additional commands that may be expected, like
successor(s) of top and the corresponding tools for TCP and I/O.

Also restores sort order in PNWHITELIST_openembedded-layer.

Signed-off-by: Patrick Ohly <[email protected]>
…/rsync)

wget is required for QA tests, the other two might be expected by
users.

Signed-off-by: Patrick Ohly <[email protected]>
Unless explicitly requested, tools like ps, wget, curl and thus also
their dependencies like ca-certificates are not installed in the
"ostro-image-swupd-reference" and thus also not in
"ostro-image-noswup".

As the reference image is expected to be used interactively,
better add the corresponding feature.

Signed-off-by: Patrick Ohly <[email protected]>
The normal log level only puts progress messages into the log file
where is is not visible unless developers know where to look. In a CI
system the output is not accessible at all.

Using bbplain for progress messages is a bit unusual and thus not done
by default in meta-swupd, but okay (they just do not get formatted as
NOTES). A dedicated logging level for progress notifications would be
better, but does not exist in bitbake at the moment.

Signed-off-by: Patrick Ohly <[email protected]>
Dangling symlinks have been a problem in particular in swupd-based
images, because it can happen that the actual file that gets pointed
to is only part of a bundle, while the link is part of the os-core.

This new check covers this. It gets applied to all images, whether
they are based on swupd or not, because we also do not want dangling
symlinks in non-swupd images.

Signed-off-by: Patrick Ohly <[email protected]>
The priorities of Toybox/Busybox must be higher than those of any
alternatives. procps uses 200, so we start at 300, to avoid
dangling symlinks for ps and friends in the os-core.

Signed-off-by: Patrick Ohly <[email protected]>
I had only tested with stateless Ostro and missed some symlinks in
/etc that are okay and need to be whitelisted.

Signed-off-by: Patrick Ohly <[email protected]>
These changes were made as part of cleaning up the meta-ostro
changes, which are now here:
https://github.com/pohly/meta-ostro/tree/meta-swupd

The only functional changes are:
- ??= instead of ?= to allow an image recipe to provide
  a default with ?= that a user then can override in local.conf
  with =
- move the Edison image archive workaround into meta-ostro,
  where it was combined with the CI testing workaround;
  it's better to have it in one place, because we will want
  to revert it at some point.

Signed-off-by: Patrick Ohly <[email protected]>
When configuring the image to be tested, symlinks are no longer
needed.

Signed-off-by: Patrick Ohly <[email protected]>
meta-swupd already has a OS_VERSION default, so using ??= in ostro.conf
is too weak.

Signed-off-by: Patrick Ohly <[email protected]>
@pohly
Copy link
Contributor Author

pohly commented Apr 9, 2016

Merged via ostroproject/meta-ostro#95

@pohly pohly closed this Apr 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.