-
Notifications
You must be signed in to change notification settings - Fork 51
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
feat(24.04): add rabbitmq slices #258
base: ubuntu-24.04
Are you sure you want to change the base?
feat(24.04): add rabbitmq slices #258
Conversation
Diff of dependencies: slices/erlang-base.yaml@@ -1,8 +1,6 @@
-adduser
libc6
libgcc-s1
libstdc++6
libsystemd0
libtinfo6
-procps
zlib1g slices/libpam-runtime.yaml@@ -1,4 +0,0 @@
-cdebconf
-debconf
-debconf-2.0
-libpam-modules slices/librabbitmq4.yaml@@ -1,2 +1,2 @@
libc6
-libssl3
+libssl3t64 slices/rabbitmq-server.yaml@@ -1,19 +1,13 @@
-adduser
-erlang-base
-erlang-base-hipe
-erlang-crypto
-erlang-eldap
-erlang-inets
-erlang-mnesia
-erlang-os-mon
-erlang-parsetools
-erlang-public-key
-erlang-runtime-tools
-erlang-ssl
-erlang-syntax-tools
-erlang-tools
-erlang-xmerl
-logrotate
-openssl
-python3
-socat
+base-files
+base-passwd
+bash
+coreutils
+grep
+libc-bin
+libtinfo6
+login
+netbase
+passwd
+python3.12
+sed
+util-linux |
5488646
to
92121b6
Compare
13360d1
to
e00a8bd
Compare
Manual testing of the slice in a rock: rockcraft.yaml name: rabbitmq
base: [email protected]
version: '0.1'
summary: RabbitMQ rock
description: |
This is a RabbitMQ Server rock.
platforms:
amd64:
services:
rabbitmq-service:
override: replace
startup: enabled
command: /usr/sbin/rabbitmq-server
parts:
build-context:
plugin: nil
source: chisel-releases/
source-type: local
override-build: |
set -x
ERL_COOKIE=${CRAFT_PART_INSTALL}/var/lib/rabbitmq/.erlang.cookie
chisel cut --release ./ --root ${CRAFT_PART_INSTALL} rabbitmq-server_bins
groupadd -r -g 90 -R ${CRAFT_PART_INSTALL} rabbitmq
useradd -d /var/lib/rabbitmq -M -u 90 -g 90 -r -s /usr/sbin/nologin -R ${CRAFT_PART_INSTALL} rabbitmq
chown 90:90 ${CRAFT_PART_INSTALL}/etc/rabbitmq
chown -R 90:90 ${CRAFT_PART_INSTALL}/var/lib/rabbitmq
chown -R 90:90 ${CRAFT_PART_INSTALL}/var/log/rabbitmq
openssl rand -base64 -out ${ERL_COOKIE} 42
chown 90:90 ${ERL_COOKIE}
chmod 400 ${ERL_COOKIE} Running the rock: $ docker run --rm rabbitmq --verbose
2024-07-01T12:27:18.585Z [pebble] Started daemon.
2024-07-01T12:27:18.595Z [pebble] POST /v1/services 5.384266ms 202
2024-07-01T12:27:18.599Z [pebble] Service "rabbitmq-service" starting: /usr/sbin/rabbitmq-server
2024-07-01T12:27:19.611Z [pebble] GET /v1/changes/1/wait 1.01580796s 200
2024-07-01T12:27:19.612Z [pebble] Started default services with change 1.
2024-07-01T12:27:20.687Z [rabbitmq-service] 2024-07-01 12:27:20.683100+00:00 [notice] <0.44.0> Application syslog exited with reason: stopped
2024-07-01T12:27:20.687Z [rabbitmq-service] 2024-07-01 12:27:20.687486+00:00 [notice] <0.230.0> Logging: switching to configured handler(s); following messages may not be visible in this log output
2024-07-01T12:27:20.998Z [rabbitmq-service]
2024-07-01T12:27:20.998Z [rabbitmq-service] ## ## RabbitMQ 3.12.1
2024-07-01T12:27:20.998Z [rabbitmq-service] ## ##
2024-07-01T12:27:20.998Z [rabbitmq-service] ########## Copyright (c) 2007-2023 VMware, Inc. or its affiliates.
2024-07-01T12:27:20.998Z [rabbitmq-service] ###### ##
2024-07-01T12:27:20.998Z [rabbitmq-service] ########## Licensed under the MPL 2.0. Website: https://rabbitmq.com
2024-07-01T12:27:20.998Z [rabbitmq-service]
2024-07-01T12:27:20.998Z [rabbitmq-service] Erlang: 25.3.2.8 [jit]
2024-07-01T12:27:20.998Z [rabbitmq-service] TLS Library: OpenSSL - OpenSSL 3.0.13 30 Jan 2024
2024-07-01T12:27:20.998Z [rabbitmq-service] Release series support status: supported
2024-07-01T12:27:20.998Z [rabbitmq-service]
2024-07-01T12:27:20.998Z [rabbitmq-service] Doc guides: https://rabbitmq.com/documentation.html
2024-07-01T12:27:20.998Z [rabbitmq-service] Support: https://rabbitmq.com/contact.html
2024-07-01T12:27:20.998Z [rabbitmq-service] Tutorials: https://rabbitmq.com/getstarted.html
2024-07-01T12:27:20.998Z [rabbitmq-service] Monitoring: https://rabbitmq.com/monitoring.html
2024-07-01T12:27:20.998Z [rabbitmq-service]
2024-07-01T12:27:20.998Z [rabbitmq-service] Logs: /var/log/rabbitmq/[email protected]
2024-07-01T12:27:20.998Z [rabbitmq-service] <stdout>
2024-07-01T12:27:20.998Z [rabbitmq-service]
2024-07-01T12:27:20.998Z [rabbitmq-service] Config file(s): (none)
2024-07-01T12:27:20.998Z [rabbitmq-service]
2024-07-01T12:27:20.998Z [rabbitmq-service] Starting broker... completed with 0 plugins. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this.
can you please explain the rationale for this diff:
@@ -1,19 +1,5 @@
adduser
-erlang-base
-erlang-base-hipe
-erlang-crypto
-erlang-eldap
-erlang-inets
-erlang-mnesia
-erlang-os-mon
-erlang-parsetools
-erlang-public-key
-erlang-runtime-tools
-erlang-ssl
-erlang-syntax-tools
-erlang-tools
-erlang-xmerl
-logrotate
+base-files
openssl
-python3
+python3.12
socat
Also, thanks for the test rock. Can you demonstrate a simple message exchange?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must admit I do not know much about most of the packages. This is a general review. I noticed a few issues which are present in a lot of places:
- Let's please omit version numbers which might change once the package is updated. Let's utilise the wildcards for that.
- I noticed that
erlang-base_bins
is a dep in a lot of places. I am wondering whether it should have beenerlang-base_libs
instead. I don't know much about it of course. Let me know if I am wrong! - While including an entire directory with everything inside using
**
at the end, let's please be careful. If careful patterns can be crafted, let's do so. For example, if we are looking for all the.beam
files, let's add that to the pattern as well e.g.<path>/**.beam
. - Inclusion of slices (mostly
bins
) as deps because the maintainer scripts use them is nice, but unnecessary if we are not replicating the scripts. Except for reasonable scenarios, let's please not include those as deps.
Again, please note that I don't know much about these packages so some of my comments may be silly or just an ask for further clarity.
All these said, amazing job on slicing this huge list of weird packages. Bravo!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more!
erlang* and logrotate packages are not available for i386 arch, so they have been commented out. |
1060b4e
to
c81c4ac
Compare
Rafid's review:
|
and the additions? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something's off with the shared libraries cause I don't seem to be able to run any binary inside this chiselled rootfs.
Also, and cause of that, this is an ideal candidate to have the spread tests included
080e585
to
d8421b0
Compare
ad157d1
to
2c32139
Compare
2c32139
to
7f19702
Compare
@cjdcordeiro @rebornplusplus This has been rebased with the lastest upstream changes so it's ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
In general there are some comments that need addressing, as well as missing tests.
P.S. since it's a big PR, please avoid force-pushing so we can easily identify the changes between reviews
slices/debconf.yaml
Outdated
/etc/apt/apt.conf.d/70debconf: | ||
/etc/debconf.conf: | ||
|
||
modules: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this slice has a mix of actual modules, with helper scripts (e.g. /usr/share/debconf/confmodule), obsolete scripts (e.g. /usr/share/debconf/confmodule.sh), configurations (e.g. /usr/share/debconf/frontend), etc.
For the sake of getting this PR in asap:
- do we actually need
debconf
anywhere? I don't see it as a dependency for anything else at quick glance. If we don't, let's leave it out and address it in a separate PR - if we do, do we need these modules? all of them? (same for binaries)
- ultimately, these modules seem to be a mixed pot of things. So can we split it further?
Finally, if this SDF really needs to go in, it also needs tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The debconf slice isn't needed anymore after adding the mutate script in libpam-runtime, so I'll remove it.
/usr/bin/epmd: | ||
/usr/bin/erl: | ||
/usr/bin/erl_call: | ||
/usr/bin/erlc: | ||
/usr/bin/escript: | ||
/usr/bin/run_erl: | ||
/usr/bin/start_embedded: | ||
/usr/bin/to_erl: | ||
/usr/lib/erlang/bin/**: | ||
/usr/lib/erlang/erts-*/bin/**: | ||
/usr/lib/erlang/lib/erl_interface-*/bin/erl_call: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing tests
- rabbitmq-server_libs | ||
- sed_bins | ||
- util-linux_bins | ||
contents: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need all of these? and again, some sound like scripts and not binaries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, most of those are bash scripts or just hardlinkes, but yeah they are needed. I tried for instance removing the rabbitmq-plugins script to create a smaller image out of that slice, but that broke rabbitmq-server / rabbitmqctl.
/usr/sbin/wipefs: | ||
/usr/sbin/zramctl: | ||
|
||
generated: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why generated
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ported from #330. That PR is almost ready, so even if it got changed afterwards it shouldn't affect this PR much.
889f864
to
abc8b60
Compare
abc8b60
to
f4c1a2a
Compare
Erlang slices were moved to another separate PR #384. |
Proposed changes
Add
rabbitmq-server
andlibrabbitmq4
slices to 24.04.Note that erlang* and logrotate packages are not available for i386 arch, so they have been commented out.
Related issues/PRs
N/A
Forward porting
N/A
Testing
Checklist
Additional Context
erlang-base
and other erlang related packages don't exist for that archpython3.12
slice was used instead ofpython
systemd
slices were not added because they are not necessary