Build and publish skupper-router image #44
Conversation
| connectors = infer() | ||
| else: | ||
| raise Exception("Invalid value for QDROUTERD_AUTO_MESH_DISCOVERY, expected 'QUERY' or 'INFER'") | ||
| get_config(sys.argv[1]).append_connectors(connectors) |
There was a problem hiding this comment.
Should this feature be (eventually) moved into skuperrouter itself?
There was a problem hiding this comment.
It is possible. After the router is started, Skupper already maintains connectors properly.
However these scripts are used only to prepare the config for the router startup, before qdrouterd is executed.
There was a problem hiding this comment.
Since skupper-router is going to be used exclusively in skupper, then making the option 'QDROUTERD_AUTO_MESH_DISCOVERY' be qdrouterd's option sounds sensible to me. It's an enhancement idea, however; not relevant for this PR right now.
There was a problem hiding this comment.
This is certainly something we can discuss and re-evaluate.
fgiorgetti
force-pushed
the
container-image
branch
2 times, most recently
from
0768ad8 to
c853d2d
Compare
.github/scripts/Dockerfile
Outdated
| # Enable additional package repositories for CentOS | ||
| # note: PowerTools is called CodeReady Linux Builder in RHEL 8 | ||
| # RUN dnf -y install epel-release && dnf config-manager --set-enabled powertools && dnf -y install gcc gcc-c++ make cmake cyrus-sasl-devel cyrus-sasl-plain cyrus-sasl-gssapi cyrus-sasl-md5 openssl-devel libuuid-devel nodejs swig wget patch findutils git valgrind libwebsockets-devel python3-devel libnghttp2-devel && dnf clean all -y | ||
| RUN dnf -y install gcc gcc-c++ make cmake cyrus-sasl-devel cyrus-sasl-plain cyrus-sasl-gssapi cyrus-sasl-md5 openssl-devel libuuid-devel nodejs swig wget patch findutils git valgrind libwebsockets-devel python3-devel libnghttp2-devel && dnf clean all -y |
There was a problem hiding this comment.
cyrus-sasl-plain cyrus-sasl-gssapi cyrus-sasl-md5? These are needed for running tests, should not be needed for build.
nodejs? Not needed, we don't have webconsole anymore.
valgrind? Not needed here.
There was a problem hiding this comment.
Those cyrus-sasl-* were useful in the former upstream image, but I believe we can get rid of them now.
Same for nodejs, since there is no more console here.
There was a problem hiding this comment.
The cyrus sasl plugins could be useful in the image, but I agree there is no need to have them in the builder.
There was a problem hiding this comment.
Actually at least cyrus-sasl-plain is required at present.
There was a problem hiding this comment.
Do you mean in the builder? The console has been removed, so do you think it should be kept in the final image?
There was a problem hiding this comment.
If the console is gone then you are right, the existing use case is gone. We can always add it when needed.
| RUN .github/scripts/compile.sh | ||
|
|
||
|
|
||
| FROM fedora:34 |
There was a problem hiding this comment.
@kgiusti @ganeshmurthy Is there any reason why our libwebsockets (in downstream) is built with LWS_WITH_LIBUV=ON and LWS_WITH_HTTP2=OFF? Thing is, libwebsockets has multiple backend options, libuv is only one of multiple event loops possible. Why was this one chosen?
(I tried asking for libwebsockets be added to ubi8 image, but I got rebuffed, since the library is not part of standard rhel8 either. https://bugzilla.redhat.com/show_bug.cgi?id=2051603)
fgiorgetti
changed the title
fgiorgetti
force-pushed
the
container-image
branch
from
33883e6 to
a642a8c
Compare
Closes #39