On Thu, Nov 21, 2024 at 4:13 AM Zhang, Peng (Paul) (CN) <
[email protected]> wrote:

>
> On 11/21/2024 12:34 PM, Bruce Ashfield wrote:
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender
> and know the content is safe.
> >
> > In message: [meta-virtualization][PATCH] cri-o: enable ptest
> > on 15/11/2024 Zhang, Peng (Paul) (CN) via lists.yoctoproject.org wrote:
> >
> >> From: Zhang Peng <[email protected]>
> >>
> >> The ptest build for cri-o was previously disabled due to issues
> >> introduced with Go 1.11, which borken the build process. With the
> >> current Go version, these issues no longer occur, and the ptest build is
> >> now functional.
> >> This commit enables ptest support and resolves the "TMPDIR
> >> [buildpaths]" issue encountered during the ptest build process.
> > Can you include a log of the ptest running in the commit log ?
> > That way the baseline pass/fail is understood and captured.
> The CRI-O ptest suite consists of a total of 382 tests. Including the
> entire test results in the commit message would make it overly
> cumbersome and difficult to navigate. Instead, I propose including only
> the list of failed and skipped tests in the commit message, as these
> provide the most critical information.
>

That's acceptable for me. If we just put the total number of tests,
a general overview of the types of testing and then anything
notable (notable here being the skipped or failed tests), then we
have a good reference for the initial state.


> Alternatively, the full test results could be stored in a README or
> another documentation file. This approach ensures the comprehensive test
> results are preserved as a baseline without bloating the commit message.
> Would you prefer the abbreviated results in the commit message, or a
> detailed log stored in a separate file?
>

See above. Just the summary in the commit, and a README with
the initial status of the tests stored in a log file.

>
> >> Signed-off-by: Zhang Peng <[email protected]>
> >> ---
> >>   recipes-containers/cri-o/cri-o_git.bb         | 37
> ++++++++++++++++++-
> >>   .../0001-Add-trimpath-to-build-nri.test.patch | 31 ++++++++++++++++
> >>   recipes-containers/cri-o/files/run-ptest      | 11 ++++++
> >>   3 files changed, 77 insertions(+), 2 deletions(-)
> >>   create mode 100644
> recipes-containers/cri-o/files/0001-Add-trimpath-to-build-nri.test.patch
> >>   create mode 100644 recipes-containers/cri-o/files/run-ptest
> >>
> >> diff --git a/recipes-containers/cri-o/cri-o_git.bb
> b/recipes-containers/cri-o/cri-o_git.bb
> >> index efc86fbe..596a43e2 100644
> >> --- a/recipes-containers/cri-o/cri-o_git.bb
> >> +++ b/recipes-containers/cri-o/cri-o_git.bb
> >> @@ -17,7 +17,9 @@ At a high level, we expect the scope of cri-o to be
> restricted to the following
> >>   SRCREV_cri-o = "20c06a19cb395445620c31730c0f1a0a1922eaae"
> >>   SRC_URI = "\
> >>        git://
> github.com/kubernetes-sigs/cri-o.git;branch=release-1.31;name=cri-o;protocol=https;destsuffix=${GO_SRCURI_DESTSUFFIX}
> <http://github.com/kubernetes-sigs/cri-o.git;branch=release-1.31;name=cri-o;protocol=https;destsuffix=$%7BGO_SRCURI_DESTSUFFIX%7D>
> \
> >> +        file://0001-Add-trimpath-to-build-nri.test.patch \
> >>           file://crio.conf \
> >> +        file://run-ptest \
> >>        "
> >>
> >>   # Apache-2.0 for docker
> >> @@ -28,7 +30,7 @@ GO_IMPORT = "import"
> >>
> >>   PV = "1.31.0+git${SRCREV_cri-o}"
> >>
> >> -inherit features_check
> >> +inherit features_check ptest
> >>   REQUIRED_DISTRO_FEATURES ?= "seccomp"
> >>
> >>   DEPENDS = " \
> >> @@ -69,6 +71,13 @@ do_compile() {
> >>        oe_runmake binaries
> >>   }
> >>
> >> +do_compile_ptest() {
> >> +    set +e
> >> +
> >> +    cd ${S}/src/import
> >> +
> >> +    oe_runmake test-binaries
> >> +}
> >>   SYSTEMD_PACKAGES =
> "${@bb.utils.contains('DISTRO_FEATURES','systemd','${PN}','',d)}"
> >>   SYSTEMD_SERVICE:${PN} =
> "${@bb.utils.contains('DISTRO_FEATURES','systemd','crio.service','',d)}"
> >>   SYSTEMD_AUTO_ENABLE:${PN} = "enable"
> >> @@ -100,6 +109,14 @@ do_install() {
> >>       install -d ${D}${localstatedir}/lib/crio
> >>   }
> >>
> >> +do_install_ptest() {
> >> +    install -d ${D}${PTEST_PATH}/test
> >> +    install -d ${D}${PTEST_PATH}/bin
> >> +    install -d ${D}${PTEST_PATH}/vendor
> >> +    cp -rf ${S}/src/import/test ${D}${PTEST_PATH}
> >> +    cp -rf ${S}/src/import/bin ${D}${PTEST_PATH}
> >> +    cp -rf ${S}/src/import/vendor ${D}${PTEST_PATH}
> >> +}
> > I'm curious. Why does the vendor directory need to be
> > copied / installed for the ptest ?
> The ctr_seccomp.bats test requires a JSON file located at
> vendor/github.com/containers/common/pkg/seccomp/seccomp.json. This file
> is accessed via the CONTAINER_SECCOMP_PROFILE environment variable,
> which defaults to the path within the vendor directory. Although this
> variable can be overridden with a custom-defined path, the default
> configuration relies on the vendor directory.
> This dependency contributes to only two sub-tests passing within the
> suite. I share your concern about whether this folder should be
> included. Another option is to allow ctr_seccomp.bats to fail by default
> if the vendor directory is not installed.
> Would you recommend keeping the directory or accept the tests failed to
> avoid requiring the vendor directory?
>

What else is in the vendor/ subdirectory (I haven't gone to check) ?
I'm ok with files being installed into a default location that the test
will check, but I want to be sure that it is only those files.

So I'd want just the files we need from vendor/ installed, and a comment
above the install line indicating that "test < ... > requires these files.

>
> >>   FILES:${PN}-config = "${sysconfdir}/crio/config/*"
> >>   FILES:${PN} += "${systemd_unitdir}/system/*"
> >>   FILES:${PN} += "/usr/local/bin/*"
> >> @@ -109,7 +126,23 @@ FILES:${PN} += "/usr/share/containers/oci/hooks.d"
> >>   ALLOW_EMPTY:${PN} = "1"
> >>
> >>   INSANE_SKIP:${PN} += "ldflags already-stripped textrel"
> >> +INSANE_SKIP:${PN}-ptest += "textrel"
> >>
> >> -deltask compile_ptest_base
> >> +RDEPENDS:${PN}-ptest += " \
> >> +    bash \
> >> +    bats \
> >> +    cni \
> >> +    crictl \
> >> +    bind-utils \
> >> +    coreutils \
> >> +    dbus-daemon-proxy \
> >> +    iproute2 \
> >> +    util-linux-unshare \
> >> +    jq \
> >> +    ipcalc \
> >> +    slirp4netns \
> >> +    parallel \
> >> +    podman \
> >> +"
> > This is an extensive list of rdepends for a ptest.
> > What exactly is it testing ? I think we need to document
> > in a README what exactly is being tested, since this
> > dependency list tells me that it is not trivial and it
> > will be prone to breakage.
> Given the suite's complexity, documenting the detailed purpose of each
> dependency would be challenging and potentially incomplete, as many
> dependencies are used across multiple tests.  However, I agree that a
> README summarizing the primary focus areas of the tests (e.g.,
> networking, security, etc.) could provide clarity and help maintainability.
>
> Let me know if you’d prefer a high-level summary of the tests or
> additional details in the README.
>

Just a summary of the high level goals and focus of the tests will
be enough for me. I agree that anything overly detailed will just get
out of date, and be a maintenance burden.

That list of dependencies basically says "This needs an complete
distribution to tests" .. and that is fine since it is contained to the
ptests. I just want to document what we know about those
dependencies. (since I will forget).

Bruce


>
> //Peng
>
> > Bruce
> >
> >>   COMPATIBLE_HOST = "^(?!(qemu)?mips).*"
> >> diff --git
> a/recipes-containers/cri-o/files/0001-Add-trimpath-to-build-nri.test.patch
> b/recipes-containers/cri-o/files/0001-Add-trimpath-to-build-nri.test.patch
> >> new file mode 100644
> >> index 00000000..c6be41f0
> >> --- /dev/null
> >> +++
> b/recipes-containers/cri-o/files/0001-Add-trimpath-to-build-nri.test.patch
> >> @@ -0,0 +1,31 @@
> >> +From 0bf230f59d211044e7993543e010b0d7f9dcead3 Mon Sep 17 00:00:00 2001
> >> +From: Peng Zhang <[email protected]>
> >> +Date: Fri, 25 Oct 2024 10:42:02 +0800
> >> +Subject: [PATCH] Add --trimpath to build nri.test
> >> +
> >> +when build test-binary, TMPDIR[buildpaths] error found in nri.test
> >> +to fix this error, add "--trimpath" option to build nri.test.
> >> +
> >> +Upstream-Status: Inappropriate [oe specific]
> >> +
> >> +Signed-off-by: Peng Zhang <[email protected]>
> >> +---
> >> + Makefile | 2 +-
> >> + 1 file changed, 1 insertion(+), 1 deletion(-)
> >> +
> >> +Index:
> cri-o-1.31.0+git20c06a19cb395445620c31730c0f1a0a1922eaae/src/import/Makefile
> >> +===================================================================
> >> +---
> cri-o-1.31.0+git20c06a19cb395445620c31730c0f1a0a1922eaae.orig/src/import/Makefile
> >> ++++
> cri-o-1.31.0+git20c06a19cb395445620c31730c0f1a0a1922eaae/src/import/Makefile
> >> +@@ -169,7 +169,7 @@ test/checkcriu/checkcriu: $(GO_FILES)
> >> +     $(GO_BUILD) $(GCFLAGS) $(GO_LDFLAGS) -tags "$(BUILDTAGS)" -o $@
> ./test/checkcriu
> >> +
> >> + test/nri/nri.test: $(wildcard test/nri/*.go)
> >> +-    $(GO) test --tags "test $(BUILDTAGS)" -c ./test/nri -o $@
> >> ++    $(GO) test --tags "test $(BUILDTAGS)" -c ./test/nri -o $@
> ${TRIMPATH}
> >> +
> >> + bin/crio: $(GO_FILES)
> >> +     $(GO_BUILD) $(GCFLAGS) $(GO_LDFLAGS) -tags "$(BUILDTAGS)" -o $@
> ./cmd/crio
> >> +--
> >> +2.34.1
> >> +
> >> diff --git a/recipes-containers/cri-o/files/run-ptest
> b/recipes-containers/cri-o/files/run-ptest
> >> new file mode 100644
> >> index 00000000..62abe959
> >> --- /dev/null
> >> +++ b/recipes-containers/cri-o/files/run-ptest
> >> @@ -0,0 +1,11 @@
> >> +#!/bin/sh
> >> +
> >> +./test/test_runner.sh | while IFS= read -r line; do
> >> +     if [[ $line =~ ^not\ ok ]]; then
> >> +             echo "FAIL: ${line#not ok }"
> >> +     elif [[ $line =~ ^ok && ! $line =~ \#\ skip ]]; then
> >> +             echo "PASS: ${line#ok }"
> >> +     elif [[ $line =~ ^ok.*#\ skip ]]; then
> >> +             echo "SKIP: ${line#ok }"
> >> +     fi
> >> +done
> >> --
> >> 2.35.5
> >>
> >> 
> >>
>


-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#8995): 
https://lists.yoctoproject.org/g/meta-virtualization/message/8995
Mute This Topic: https://lists.yoctoproject.org/mt/109593003/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/meta-virtualization/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to