Add tool to bootstrap openQA with only one command #1936
Conversation
| @@ -0,0 +1,61 @@ | |||
| #!/bin/bash -x | |||
There was a problem hiding this comment.
can you make that #!/bin/sh -e? By default I would not make that "-x". We can leave that to the user, don't you think?
There was a problem hiding this comment.
It would give the user more info on what is happening.
There was a problem hiding this comment.
I did some tests and it seems that sh -e won't work because zypper --root installation will return 107 and also because of some potential error (that my script actually deals with) when starting the container.
There was a problem hiding this comment.
I suggest to explicitly catch these, e.g. zypper … ||: to explicitly ignore the exit code then
There was a problem hiding this comment.
Good point. I use -e in almost all my scripts and either do what oliver suggests or use the command as an if condition and actually handle the error. I would keep the -x, too.
| fi | ||
| /usr/share/openqa/script/fetchneedles | ||
|
|
||
| if ping -c1 gitlab.suse.de. ; then |
There was a problem hiding this comment.
In this case I would just call https://gitlab.suse.de/openqa/scripts/blob/master/fetchneedles-sles instead
There was a problem hiding this comment.
I tried the script and it didn't work.
There was a problem hiding this comment.
It did nothing for the sle needles.
Also I don't see why I should need an additional script for a simple git clone …; chown ….
There was a problem hiding this comment.
It's not just that as it also does the normal "fetchneedles" call
|
I already said in the progress ticket that I like it. Did you test that the RPM actually builds? |
No - I just tested the scripts on w3. |
Codecov Report
@@ Coverage Diff @@
## master #1936 +/- ##
==========================================
+ Coverage 88.64% 89.11% +0.46%
==========================================
Files 154 154
Lines 10360 10360
==========================================
+ Hits 9184 9232 +48
+ Misses 1176 1128 -48
Continue to review full report at Codecov.
|
I just tested building and installing an RPM and it worked fine. |
asdil12
force-pushed
the
bootstrap
branch
2 times, most recently
from
5eb786e
to
92a2fc7
Compare
|
Maybe mention the script in the setup guide (you have already written https://w3.suse.de/~dheidler/openqa-container.html anyways)? And note that is has only been tested under Tumbleweed so far (if that's the case). |
asdil12
force-pushed
the
bootstrap
branch
4 times, most recently
from
3d43332
to
cba5f57
Compare
|
@asdil12 I suggest to add either an automatic test or at least cover how to use/test it within documentation. Can you do that? |
|
@okurz I am planning to test this using openQA. I will add some documentation, though. |
asdil12
force-pushed
the
bootstrap
branch
3 times, most recently
from
c051d65
to
3eb5f26
Compare
| [source,sh] | ||
| ------------------------------------------------------------------------------- | ||
| zypper in openQA-bootstrap | ||
| /usr/share/openqa/script/openqa-bootstrap |
There was a problem hiding this comment.
Could you call this script as post-install action of command specific subpackages?
There was a problem hiding this comment.
No, because this will just result in an rpm that takes forever to install without giving user feedback as rpm caches the script output until the script returns.
Also this script adds some repos and installs some packages. This wouldn't work due to the already locked rpm database.
| zypper in openQA-bootstrap | ||
| /usr/share/openqa/script/openqa-bootstrap-container | ||
|
|
||
| systemd-run -tM openqa1 /bin/bash # start a shell in the container |
There was a problem hiding this comment.
Why would I want /need that or is it just an additional example?
There was a problem hiding this comment.
You need this eg. to run openqa-clone-job or openqa-client
| %files bootstrap | ||
| %{_datadir}/openqa/script/openqa-bootstrap | ||
| %{_datadir}/openqa/script/openqa-bootstrap-container | ||
|
|
There was a problem hiding this comment.
How about moving these to /usr/bin on install as they already have an openqa-prefix?
There was a problem hiding this comment.
Having them in this special dir makes it harder to accidentally execute this scripts.
| # add extra repos for leap | ||
| source /etc/os-release | ||
| if $NAME == "openSUSE Leap" ; then | ||
| zypper -n addrepo obs://devel:openQA devel:openQA |
There was a problem hiding this comment.
I suggest the addrepo parameters "-f -p 105"
There was a problem hiding this comment.
-f is already the default.
There was a problem hiding this comment.
Does -p 105 increase or decrease the priority?
There was a problem hiding this comment.
It decreases priority which should be fine for "devel:openQA" itself, meaning: Only if we can not find the package in the main repos we install packages from here. However, thinking about it, depending on the needed features we might want to prefer "devel:openQA" so -p 95 would be more suitable
| # setup database | ||
| systemctl enable --now postgresql | ||
| su postgres -c "createuser -D geekotest" | ||
| su postgres -c "createdb -O geekotest openqa" |
There was a problem hiding this comment.
Can you rely on the openqa-setup-db systemd service here to prevent duplication?
There was a problem hiding this comment.
No, because consider it as very ugly to use a systemd job for a task, that is only executed once.
This is the job of an sh script - not of a systemd.service file.
Also it will help users to understand, what this script does and therefore act as some kind of setup documentation.
There was a problem hiding this comment.
Well, we should be consistent within this repo still. We can suggest to replace the existing openqa-setup-db service with a script instead as well
|
I added more comments but actually I am fine when anyone merges in the current state. We can always improve later still |
Ticket: https://progress.opensuse.org/issues/45365