yetus: Install dependencies inside Yetus container

[rene]

go-libzfs module depends on libzfs to be built. However, the package libzfslinux-dev is not available inside the Yetus container, leading to the following error:

cmd/zfsmanager/handlediskconfig.go:12:9: could not import github.com/bicomsystems/go-libzfs

This commit introduces the script tools/run-yetus.sh that allows to install and manage all dependencies inside the container before start to execute all the tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 19.75%. Comparing base (16a8071) to head (0d559e2).

❗ Current head 0d559e2 differs from pull request most recent head 40faabb. Consider uploading reports for the commit 40faabb to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3798   +/-   ##
=======================================
  Coverage   19.74%   19.75%           
=======================================
  Files         235      235           
  Lines       51716    51716           
=======================================
+ Hits        10209    10214    +5     
+ Misses      40765    40761    -4     
+ Partials      742      741    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rene]

Converted to draft because the libzfs installed is now leading to another errors (I think due to its version).... need to check more deeply....

[rene]

@eriknordmark , @deitch , build libzfs from sources it works, the issues reported in the PR are gone. However, this PR solves the problem when running make yetus locally, I'm not sure how we could integrate it to GH actions without too much changes.... would like to hear your thoughts about it....

[deitch]

Every run that invokes yetus - which is every CI job ever invoked on EVE - is going to clone openzfs and build it? Will that not slow down every run? And as you wrote, this doesn't solve the CI issue, where we just use the yetus action (which we should, to keep it simple). This also hacks the startup of the container, which makes it brittle, sensitive to changes in yetus.

How does it work outside of yetus, i.e. normal EVE build? Is it because we install it as part of the eve-build-<user> container based on the Dockerfile here?

The source to the yetus action is this repo. It is a composite action that then calls a single docker container this line.

What is the standard yetus way of, "I need these dependencies installed before you do your run"? There should be a way to do that.

[rene]

Every run that invokes yetus - which is every CI job ever invoked on EVE - is going to clone openzfs and build it? Will that not slow down every run? And as you wrote, this doesn't solve the CI issue, where we just use the yetus action (which we should, to keep it simple). This also hacks the startup of the container, which makes it brittle, sensitive to changes in yetus.

I understand and agree, my first option was use the binary package, but versions don't match, that's why the approach was to build from sources... regarding the script, entrypoint.sh from yetus container does the check if it's being called with parameters for test-patch or a third party program, which is this case will be executed with bash, so I don't think it's a hack: https://github.com/apache/yetus/blob/main/entrypoint.sh#L18

How does it work outside of yetus, i.e. normal EVE build? Is it because we install it as part of the eve-build-<user> container based on the Dockerfile here?

The userspace tools are built in the dom0-ztools package: https://github.com/lf-edge/eve/blob/master/pkg/dom0-ztools/Dockerfile#L11
AFAIK this is the right version to use... which matches the ZFS module built in our kernel...

The source to the yetus action is this repo. It is a composite action that then calls a single docker container this line.

Nice, thanks, will take a look....

What is the standard yetus way of, "I need these dependencies installed before you do your run"? There should be a way to do that.

Well, I don't know if there is a standard way to do that... they provide the container, in my view the right way would be extend yetus container to a custom one which would include all the dependencies... I was trying to avoid that so we would need to also maintain this custom container, but perhaps is the best approach....

[eriknordmark]

@rene #3780 is failing unit tests (and yetus) workflows. Does this PR have any impact on those?

[deitch]

I don't think it's a hack:

I didn't mean that in a negative way. The ENTRYPOINT and CMD of a container is an integral part of its shipping, and assumptions about how it runs. We are overriding that somewhat, by injecting something in between its normal CMD and what we want. It isn't bad, but it does change it somewhat.

The userspace tools are built in the dom0-ztools package

Isn't the problem with yetus occurring in the pillar container? How does dom0-ztools (a different package built separately) help with that?

in my view the right way would be extend yetus container to a custom one which would include all the dependencies... I was trying to avoid that so we would need to also maintain this custom container, but perhaps is the best approach..

I think you are correct to shy away from it for those reasons. We really do not need "yet another customized thing" to maintain.

Trying to think this through and be helpful.

1. We have to be able to run go commands (like vet or lint) on pkg/pillar (and elsewhere) from within the yetus container
2. Some of pillar depends on external libraries, specifically github.com/bicomsystems/go-libzfs. Normal go tools have no problem with those, as they download them automatically, and in any case, it all is vendored in pillar.
3. github.com/bicomsystems/go-libzfs depends on CGO, and more importantly, relies on the openzfs libraries. go tools do not know how to get those.
4. For our normal build, we do it inside the eve-build-* container, which includes the packages zfs-dev, zfs-libs
5. For yetus build, we do it inside the yetus container, which we do not control, and hence is missing libraries

From a yetus perspective, the module version doesn't even really matter. It just is checking if the code is buildable and passes vet and lint and so on. It just needs some version that satisfies dependencies.

I see a few ways we could do this.

• inject starting script which builds zfs - what you did, we could try something similar with the actual yetus container image. That would mean bypassing the official action, but it still would use the official container
• inject starting script which installs pre-built zfs - similar to what you did, but rather than git clone and build, run package install. ghcr.io/apache/yetus is based on Ubuntu 22.04, so apt update && apt install -y libzfs4linux should do it. This is a lot quicker.
• As you said, extend the container. This is the recommended way to do it in yetus, and is simple:

FROM ghcr.io/apache/yetus:main
RUN apt update && apt install -y libzfs4linux

• use the plugins functionality to create a custom plugin to install libzfs4linux, see advanced yetus
• get away from dependency on lib-zfs.

If we need to bypass the official yetus action, it isn't a big deal. All the action does pretty much is wrap running a container, see here.

My favourite is the last one, as it makes pillar much easier to build, works on any system, and moves us towards our goals of making it easier to build and test each part of pillar independently. That was the reason we implemented http://github.com/packetcap/go-pcap/ , to avoid CGO and library and header dependency problems.

How hard is it to do?

The library we use, http://github.com/bicomsystems/go-libzfs, has not been updated in 2 years (since Petr and I put in small PRs). All it does is wrap ioctl calls, which is easy to do in go. There is one native implementation that I found https://github.com/lorenz/go-zfs .

We use the current library in 5 files. Most have a few individual uses, one has a large number, but mostly types, with a few calls. At first blush it looks straightforward to do. Calls are different, but should not be hard.

I do not know, however, how hard it is to test it. if someone could confirm that they could test it sanely, I would be willing to try a cut at converting it. I would give it one day. If we can convert it, great; if not, let it go.

[andrewd-zededa]

The original lint error starting this conversation is from our zfs 2.1.2/2.1.12 versions. With PRs #3779 and #3780 I'm moving us over to a forked copy https://github.com/andrewd-zededa/go-libzfs which I've updated for zfs 2.2.2 (https://github.com/andrewd-zededa/go-libzfs/tree/andrewd-zfs-2.2.2-0.4.0).

@deitch @rene Can I suggest a couple more options?

• check in the libzfs header files under the eve repo (not great)
• add yetus ignore config to my go-libzfs fork

[eriknordmark]

@deitch @rene Can I suggest a couple more options?

• check in the libzfs header files under the eve repo (not great)
• add yetus ignore config to my go-libzfs fork

It seems like both the unit tests and yetus fails because it can't compile anything. Thus a yetus ignore for some files wouldn't help - we'd have to disable all of yetus and the unit tests. This is clearly a non-starter.

What does it take to do make test work in your workspace?

[andrewd-zededa]

@eriknordmark here is a change to get 'make test' running here. It pulls in the correct zfs version headers without the build time implications of building user space openzfs each time.

diff --git a/build-tools/src/scripts/Dockerfile b/build-tools/src/scripts/Dockerfile
index 5c4f0365d..39f1efcf6 100644
--- a/build-tools/src/scripts/Dockerfile
+++ b/build-tools/src/scripts/Dockerfile
@@ -12,7 +12,7 @@ ARG all_proxy
 RUN apk add --no-cache openssh-client git gcc linux-headers libc-dev util-linux libpcap-dev bash vim make protobuf protobuf-dev sudo tar curl graphviz ttf-freefont patch dnsmasq
 # we need updated libraries, here we use the same version as for eve/alpine
 # hadolint ignore=DL3018
-RUN apk --no-cache --repository https://dl-cdn.alpinelinux.org/alpine/v3.16/main add -U --upgrade zfs-dev zfs-libs
+RUN apk --no-cache --repository https://dl-cdn.alpinelinux.org/alpine/v3.19/main add -U --upgrade zfs-dev=2.2.2-r0 zfs-libs=2.2.2-r0
 RUN deluser ${USER} ; delgroup ${GROUP} || :
 RUN sed -ie /:${UID}:/d /etc/passwd /etc/shadow ; sed -ie /:${GID}:/d /etc/group || :
 RUN addgroup -g ${GID} ${GROUP} && adduser -h /home/${USER} -G ${GROUP} -D -H -u ${UID} ${USER}

Unfortunately this zfs package version is not in the 3.16 alpine repos.

[andrewd-zededa]

I can't find zfs 2.2.2 version of the packages in ubuntu 22.04/jammy repos.

@deitch Another go mod option looks like https://github.com/mistifyio/go-zfs which seems to be used fairly widely.

[deitch]

check in the libzfs header files under the eve repo (not great

Also doesn't provide the need .so libraries.

[deitch]

this zfs package version is not in the 3.16 alpine repos

We still are running on 3.16?
We should fix that.

[deitch]

Another go mod option looks like https://github.com/mistifyio/go-zfs which seems to be used fairly widely

It does not depend on CGO or libraries. It is a wrapper around the zfs CLI tools, so we could do that. It would require having the zfs tools installed, but is that so bad? I don't think so.

[rene]

I can't find zfs 2.2.2 version of the packages in ubuntu 22.04/jammy repos.

@deitch Another go mod option looks like https://github.com/mistifyio/go-zfs which seems to be used fairly widely.

I think it's a feasible solution, but TBH I'm afraid that we are just postponing a problem that eventually will happen again in the future.... now we change the library only to make Yetus running successfully... but sooner or later we might have another library that will require to be built as well... and then we will need to find another alternative again..... @deitch listed good approaches that can be followed in order to get the library binaries inside Yetus' container... I think we should move towards one of these approaches instead of replace the library....

Let's keep this PR as Draft for now only as a point of discussion until we take some decision, then I can change it or close it....

[deitch]

TBH I'm afraid that we are just postponing a problem

That is the real problem, isn't it? As long as we can build these things with OS-level dependencies that have to be installed, we eventually will hit the issue (although we should be willing to take reasonable solutions that buy us a year or two).

yetus runs inside a container. All build dependencies must be available inside that container. That means that everything must be one of:

• mountable from the source directory (like the code)
• part of standard tooling (i.e. go-native, which means either of the 2 alternate libraries listed above)

If not those, then we have to accept that we have to extend yetus.

But we need to decide, as a principle, which of these two we do.

[rene]

@eriknordmark , @christoph-zededa, @andrewd-zededa and I had a discussion about this issue. The priority for now is to fix the unit tests and make it work with the required libzfs library. @andrewd-zededa will work on that and update his PR. Meanwhile we will think in a better approach to mitigate the yetus issue, if possible, without extending the yetus container and forking the yetus GH action. I'm closing this PR. Thanks everybody for the useful comments.