README: add coreos-assembler bash function, overhaul docs #290

dustymabe · 2019-01-21T20:45:13Z

This adds the cosa bash function, which brings support for various
environment variables to tweak functionality at runtime. Having this
functionality allows us to overhaul the rest of the documentation to
use it instead of adding new aliases in various places.

dustymabe · 2019-01-21T21:07:21Z

example of executing using the function:

$ export COREOS_ASSEMBLER_GIT=/var/b/shared/code/github.com/coreos/coreos-assembler/
$ export COREOS_ASSEMBLER_CONFIG_GIT=/var/b/shared/code/github.com/coreos/fedora-coreos-config/
$ export COREOS_ASSEMBLER_PRIVILEGED=
$ export COREOS_ASSEMBLER_CONTAINER=localhost/coreos-assembler
$ export COREOS_ASSEMBLER_CONTAINER_RUNTIME_ARGS=
$
$ cosa() {
>     env | grep COREOS_ASSEMBLER
>     set -x # so we can see what command gets run
>     sudo podman run --rm -ti -v ${PWD}:/srv/ --userns=host --device /dev/kvm --name cosa            \
>                ${COREOS_ASSEMBLER_PRIVILEGED:+--privileged}                                         \
>                ${COREOS_ASSEMBLER_CONFIG_GIT:+-v $COREOS_ASSEMBLER_CONFIG_GIT:/srv/src/config/:ro}  \
>                ${COREOS_ASSEMBLER_GIT:+-v $COREOS_ASSEMBLER_GIT/src/:/usr/lib/coreos-assembler/:ro} \
>                ${COREOS_ASSEMBLER_CONTAINER_RUNTIME_ARGS}                                           \
>                ${COREOS_ASSEMBLER_CONTAINER:-quay.io/coreos-assembler/coreos-assembler:latest} $@
>     set +x
> }
$
$ cosa fetch
COREOS_ASSEMBLER_CONTAINER=localhost/coreos-assembler
COREOS_ASSEMBLER_PRIVILEGED=
COREOS_ASSEMBLER_CONTAINER_RUNTIME_ARGS=
COREOS_ASSEMBLER_GIT=/var/b/shared/code/github.com/coreos/coreos-assembler/
COREOS_ASSEMBLER_CONFIG_GIT=/var/b/shared/code/github.com/coreos/fedora-coreos-config/
+ sudo podman run --rm -ti -v /var/b/shared/fedora-assembler:/srv/ --userns=host --device /dev/kvm --name cosa -v /var/b/shared/code/github.com/coreos/fedora-coreos-config/:/srv/src/config/:ro -v /var/b/shared/code/github.com/coreos/coreos-assembler//src/:/usr/lib/coreos-assembler/:ro -e 'COREOS_ASSEMBLER*' localhost/coreos-assembler fetch
info: Missing CAP_SYS_ADMIN; using virt
Using manifest: /srv/src/config/manifest.yaml
Running: rpm-ostree compose tree --repo=/srv/repo --cachedir=/srv/cache --touch-if-changed /srv/tmp/treecompose.changed --unified-core /srv/src/config/manifest.yaml --download-only

jlebon · 2019-01-21T21:55:06Z

Isn't this what the bottlecap script is supposed to wrap?

cgwalters · 2019-01-21T22:01:13Z

This would break the symmetry that today one can invoke e.g. coreos-assembler build also from inside the container via coreos-assembler shell. Today one can use most of the commands in README.md both ways (though I do feel we should be encouraging people to run most commands from inside the container).

Why specifically did you choose cosa? I feel like there has to be a well-understood rationale to have the binary name not match the project name. At least currently coreos-<TAB> auto-completes here. (But as soon as some other project was introduced that was coreos-X and lived in /usr/bin the argument for shortening or changing coreos-assembler would be stronger)

ashcrow

👍 this is REALLY helpful!

README.md

ashcrow · 2019-01-21T22:03:34Z

Isn't this what the bottlecap script is supposed to wrap?

I actually never checked that file until this comment. It does look like it.

ajeddeloh · 2019-01-21T22:05:01Z

@jlebon yeah, although it's somewhat orthogonal; there's nothing that says you should use bottlecap and nothing that says you shouldn't. Really we should have two sections, one with bottlecap and one without. There are some difference with this approach (bottlecap runs make && make check && make install rather than binding over the /usr/lib/coreos-assembler directory. This means you'll catch shellcheck errors as you develop and if we added scripts/helpers requiring compilation it would automatically compile them.

README.md

dustymabe · 2019-01-21T22:23:21Z

@cgwalters
Why specifically did you choose cosa? I feel like there has to be a well-understood rationale to have the binary name not match the project name. At least currently coreos-<TAB> auto-completes here. (But as soon as some other project was introduced that was coreos-X and lived in /usr/bin the argument for shortening or changing coreos-assembler would be stronger)

I've always used an alias since coreos-assembler is a pain to type. I had been using cass until recently I've seen people abbreviate this as COSA and I liked that better. In this situation we can:

change the readme back to coreos-assembler everywhere - i'm cool with this as it was kind of a late addition
add a helper inside the container that points /usr/bin/cosa -> /usr/bin/coreos-assembler to help the symmetry problem. If we do that we should probably open a new issue and have people agree on the 4 letter word that we use.

WDYT?

dustymabe · 2019-01-21T22:27:39Z

pushed a fixup for the minor things

cgwalters · 2019-01-22T14:23:09Z

change the readme back to coreos-assembler everywhere - i'm cool with this as it was kind of a late addition

I'd vote for that and circle back to the naming/typing issue as a separate PR. It's a bit ironic because I find rpm-ostree annoying to type and have been advocating a rename partially based on that but you're on the other side there 😉

(I'm fine with cosa though it does open questions around whether we're renaming the github project too, how far it permeates into the docs...)

dustymabe · 2019-01-22T15:56:42Z

I'd vote for that and circle back to the naming/typing issue as a separate PR.

pushed fixup ^^

dustymabe · 2019-01-22T18:15:31Z

I'd vote for that and circle back to the naming/typing issue as a separate PR.

opened #291 to discuss abbreviation. working on the dir permissions change now.

README.md

dustymabe · 2019-01-22T22:20:14Z

ok should have addressed all comments now and rebased/squashed everything so it should be ready for merge if everything looks good.

side note: if someone could answer this question that would be great.

cgwalters · 2019-01-27T18:16:35Z

Overall this looks good...I just wanted to get a chance to test it myself and haven't been at a place with good enough Internet to pull the latest cosa container. I should be able to do so tomorrow; if someone else beats me to it and tests this they should feel free to merge too!

cgwalters · 2019-01-27T18:17:45Z

This replaces #263 right?

dustymabe · 2019-01-28T02:56:43Z

.I just wanted to get a chance to test it myself and haven't been at a place with good enough Internet to pull the latest cosa container.

👍 - no rush - i want some of the other people who had comments to sign off too

This replaces #263 right?

Not exactly. I think @jlebon was trying to document how to use rootless podman to run a build. While we technically can run in a rootless container (assuming you provide /dev/kvm) I haven't found the rootless podman builds/runs to work for me reliably yet, so I don't think we should document that flow. All of the commands in this documentation still use sudo.

README.md

rfairley · 2019-01-29T23:07:59Z

Tried the Setup section, and works well overall! Ran into a couple of problems, which could be specific to my environment.

I needed to set COREOS_ASSEMBLER_PRIVILEGED=true for coreos-assembler run to work on my end (after doing init, fetch, and build unpriv successfully). The avc denial doing run with unprivileged was (see Details):

$ coreos-assembler run
...
+ exec qemu-kvm -name coreos -m 2048 -nographic -netdev user,id=eth0,hostname=coreos -device virtio-net-pci,netdev=eth0 -object rng-random,filename=/dev/urandom,id=rng0 -device virtio-rng-4
qemu-system-x86_64: -drive if=virtio,cache=unsafe,file=builds/29/fedora-coreos-29-qemu.qcow2: Could not open 'builds/29/fedora-coreos-29-qemu.qcow2': Permission denied
+ rc=1
+ set +x

$ sudo ausearch -m avc --start 17:14:55
----
time->Tue Jan 29 17:14:55 2019
type=AVC msg=audit(1548800095.064:1972): avc:  denied  { read } for  pid=13784 comm="qemu-system-x86" name="fedora-coreos-29-qemu.qcow2" dev="dm-0" ino=528736 scontext=system_u:system_r:co0
----
time->Tue Jan 29 17:14:55 2019
type=AVC msg=audit(1548800095.064:1973): avc:  denied  { read } for  pid=13784 comm="qemu-system-x86" name="fedora-coreos-29-qemu.qcow2" dev="dm-0" ino=528736 scontext=system_u:system_r:co0

$ sudo ausearch -m avc --start 17:14:55 | audit2allow


#============= container_t ==============

#!!!! This avc is a constraint violation.  You would need to modify the attributes of either the source or target types to allow this access.
#Constraint rule: 
#	mlsconstrain file { ioctl read lock execute execute_no_trans } ((h1 dom h2 -Fail-)  or (t1 != mcs_constrained_type -Fail-) ); Constraint DENIED
mlsconstrain file { write setattr append unlink link rename } ((h1 dom h2 -Fail-)  or (t1 != mcs_constrained_type -Fail-) ); Constraint DENIED
mlsconstrain file { create relabelto } ((h1 dom h2 -Fail-)  and (l2 eq h2)  or (t1 != mcs_constrained_type -Fail-) ); Constraint DENIED
mlsconstrain file { relabelfrom } ((h1 dom h2 -Fail-)  or (t1 != mcs_constrained_type -Fail-) ); Constraint DENIED

#	Possible cause is the source level (s0:c585,c1005) and target level (s0:c174,c429) are different.
allow container_t container_file_t:file read;

rfairley · 2019-01-29T23:08:45Z

One other thing, after export COREOS_ASSEMBLER_PRIVILEGED=true and then coreos-assembler run, the MOTD (with the QEMU monitor and ICMP messages) didn't show nor was the unit to generate it found:

Fedora 29 (CoreOS preview)
Kernel 4.20.4-200.fc29.x86_64 on an x86_64 (ttyS0)

This is coreos (Linux x86_64 4.20.4-200.fc29.x86_64) 22:51:45
SSH host key: SHA256:gUQbUlB4v1PLnKmKuE7JsjKMZw+YYuygwfyBBEGLcLk (ED25519)
SSH host key: SHA256:3uBQrrfqn8E0cz7bDei6q83uk9CKvrSzzls2kZCt9yY (ECDSA)
SSH host key: SHA256:C4FXCZFel7m4ooXwsVfRxQYudUDPyPEhdCL0VrC1+68 (RSA)
eth0: 10.0.2.15 fec0::5054:ff:fe12:3456

coreos login: core (automatic login)

Fedora CoreOS (preview)
https://github.com/coreos/fedora-coreos-tracker
WARNING: All aspects subject to change, highly experimental
---

[core@coreos ~]$ systemctl status serial-getty-motd
Unit serial-getty-motd.service could not be found.

Not sure if the unit not getting in could be related to running unpriv then priv, or if it's something with my environment - will check on laptop too tomorrow.

README.md

dustymabe · 2019-01-30T00:02:54Z

@rfairley
The avc denial doing run with unprivileged was

I've seen this before and am attempting to fix this in #294

@rfairley
the MOTD (with the QEMU monitor and ICMP messages) didn't show nor was the unit to generate it found

haven't seen that myself - might have to do some investigation after this merges if you still see it.

rfairley · 2019-01-30T15:04:56Z

@dustymabe

Just to follow up on the second issue (MOTD), coreos-assembler run worked on my laptop (a new install of Silverblue), so it does seem like just something on the desktop (F29 where some networking config has been changed by me at some point). Will check later to see if there is any log from Ignition indicating a problem.

cgwalters · 2019-01-30T16:45:06Z

README.md

+    set -x # so we can see what command gets run
+    sudo podman run --rm -ti -v ${PWD}:/srv/ --userns=host --device /dev/kvm --name coreos-assembler \
+               ${COREOS_ASSEMBLER_PRIVILEGED:+--privileged}                                          \
+               ${COREOS_ASSEMBLER_CONFIG_GIT:+-v $COREOS_ASSEMBLER_CONFIG_GIT:/srv/src/config/:ro}   \


It'd be a bit nicer if we did something like auto-detect the case where this is a symlink and did a the bind mount over it or so. But not necessary to do in this change.

It'd be a bit nicer if we did something like auto-detect the case where this is a symlink and did a the bind mount over it or so.

yeah, hadn't thought of that before. now that we're using a function and not an alias we could do that

But not necessary to do in this change.

is that a +1 for merge?

dustymabe · 2019-01-30T19:46:20Z

anything else outstanding for this review?

rfairley · 2019-01-30T19:50:24Z

Just one minor thing I commented above

jlebon

This looks good to me! (Though I haven't tested it).

README.md

This adds a bash function, which brings support for various environment variables to tweak functionality at runtime. Having this functionality allows us to overhaul the rest of the documentation to use it instead of adding new aliases in various places.

Determined in code review that it wasn't necessary any longer. #290 (comment)

dustymabe · 2019-01-31T16:38:17Z

Just one minor thing I commented above

Thanks. I removed the podman push section in a 2nd commit.

rfairley

LGTM

A minor detail, just seeing this now: should the paths /srv/coreos be updated to ./srv-coreos? There are 3 instances under Initializing (https://github.com/coreos/coreos-assembler/blob/99be52d3491c35bbf6e5ebe8032c001332e24a6d/README.md#initializing) and Performing a build.

dustymabe · 2019-01-31T17:06:59Z

A minor detail, just seeing this now: should the paths /srv/coreos be updated to ./srv-coreos?

fixed this in a new commit, I changed the paths to be relative to the host directory PWD

README.md

Rather than reference the absolute path in the assembler container let's reference the relative path to PWD on the host.

rfairley · 2019-01-31T17:19:27Z

👍 all LGTM, good to merge.

dustymabe · 2019-01-31T17:49:43Z

ok going to merge this now since I've got three 👍 .

I would appreciate comments/votes over in #291 regarding the convenience discussion (could be a naming discussion depending on how you look at it).

dustymabe requested review from ashcrow, jlebon and ajeddeloh January 21, 2019 20:45

ashcrow reviewed Jan 21, 2019

View reviewed changes

cgwalters reviewed Jan 21, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

jlebon reviewed Jan 21, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

dustymabe changed the title ~~README: add cosa bash function, overhaul docs~~ README: add coreos-assembler bash function, overhaul docs Jan 22, 2019

cgwalters reviewed Jan 22, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

cgwalters reviewed Jan 29, 2019

View reviewed changes

README.md Show resolved Hide resolved

rfairley reviewed Jan 29, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

cgwalters reviewed Jan 30, 2019

View reviewed changes

jlebon approved these changes Jan 31, 2019

View reviewed changes

README.md Show resolved Hide resolved

dustymabe added 2 commits January 31, 2019 11:23

rfairley approved these changes Jan 31, 2019

View reviewed changes

ashcrow approved these changes Jan 31, 2019

View reviewed changes

rfairley reviewed Jan 31, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

dustymabe merged commit 7405f50 into coreos:master Jan 31, 2019

dustymabe mentioned this pull request Feb 12, 2019

README: Use cosa in examples #343

Merged

dustymabe deleted the dusty branch February 15, 2019 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

README: add coreos-assembler bash function, overhaul docs #290

README: add coreos-assembler bash function, overhaul docs #290

dustymabe commented Jan 21, 2019

dustymabe commented Jan 21, 2019 •

edited

Loading

jlebon commented Jan 21, 2019

cgwalters commented Jan 21, 2019

ashcrow left a comment

ashcrow commented Jan 21, 2019

ajeddeloh commented Jan 21, 2019

dustymabe commented Jan 21, 2019

dustymabe commented Jan 21, 2019

cgwalters commented Jan 22, 2019

dustymabe commented Jan 22, 2019

dustymabe commented Jan 22, 2019

dustymabe commented Jan 22, 2019

cgwalters commented Jan 27, 2019

cgwalters commented Jan 27, 2019

dustymabe commented Jan 28, 2019

rfairley commented Jan 29, 2019 •

edited

Loading

rfairley commented Jan 29, 2019

dustymabe commented Jan 30, 2019

rfairley commented Jan 30, 2019

cgwalters Jan 30, 2019

dustymabe Jan 30, 2019

dustymabe commented Jan 30, 2019

rfairley commented Jan 30, 2019

jlebon left a comment

dustymabe commented Jan 31, 2019

rfairley left a comment

dustymabe commented Jan 31, 2019

rfairley commented Jan 31, 2019

dustymabe commented Jan 31, 2019

README: add coreos-assembler bash function, overhaul docs #290

README: add coreos-assembler bash function, overhaul docs #290

Conversation

dustymabe commented Jan 21, 2019

dustymabe commented Jan 21, 2019 • edited Loading

jlebon commented Jan 21, 2019

cgwalters commented Jan 21, 2019

ashcrow left a comment

Choose a reason for hiding this comment

ashcrow commented Jan 21, 2019

ajeddeloh commented Jan 21, 2019

dustymabe commented Jan 21, 2019

dustymabe commented Jan 21, 2019

cgwalters commented Jan 22, 2019

dustymabe commented Jan 22, 2019

dustymabe commented Jan 22, 2019

dustymabe commented Jan 22, 2019

cgwalters commented Jan 27, 2019

cgwalters commented Jan 27, 2019

dustymabe commented Jan 28, 2019

rfairley commented Jan 29, 2019 • edited Loading

rfairley commented Jan 29, 2019

dustymabe commented Jan 30, 2019

rfairley commented Jan 30, 2019

cgwalters Jan 30, 2019

Choose a reason for hiding this comment

dustymabe Jan 30, 2019

Choose a reason for hiding this comment

dustymabe commented Jan 30, 2019

rfairley commented Jan 30, 2019

jlebon left a comment

Choose a reason for hiding this comment

dustymabe commented Jan 31, 2019

rfairley left a comment

Choose a reason for hiding this comment

dustymabe commented Jan 31, 2019

rfairley commented Jan 31, 2019

dustymabe commented Jan 31, 2019

dustymabe commented Jan 21, 2019 •

edited

Loading

rfairley commented Jan 29, 2019 •

edited

Loading