On 11/21/2024 10:04 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.


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
    
<https://urldefense.com/v3/__http://lists.yoctoproject.org__;!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOaoUePcw$>
    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
    
<https://urldefense.com/v3/__http://cri-o_git.bb__;!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOQ5BxFg1$>
           | 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
    
<https://urldefense.com/v3/__http://cri-o_git.bb__;!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOQ5BxFg1$>
    b/recipes-containers/cri-o/cri-o_git.bb
    
<https://urldefense.com/v3/__http://cri-o_git.bb__;!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOQ5BxFg1$>
    >> index efc86fbe..596a43e2 100644
    >> --- a/recipes-containers/cri-o/cri-o_git.bb
    
<https://urldefense.com/v3/__http://cri-o_git.bb__;!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOQ5BxFg1$>
    >> +++ b/recipes-containers/cri-o/cri-o_git.bb
    
<https://urldefense.com/v3/__http://cri-o_git.bb__;!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOQ5BxFg1$>
    >> @@ -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}
    
<https://urldefense.com/v3/__http://github.com/kubernetes-sigs/cri-o.git;branch=release-1.31;name=cri-o;protocol=https;destsuffix=$*7BGO_SRCURI_DESTSUFFIX*7D__;JSU!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOfiTZ4sE$>
    \
    >> + 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
    
<https://urldefense.com/v3/__http://github.com/containers/common/pkg/seccomp/seccomp.json__;!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOf5GEmOu$>.
    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.
After reviewing the contents of the vendor/ directory and examining the code, it turns out that the only file required for the test is seccomp.json. This file is needed specifically for the ctr_seccomp.bats test, and its presence allows two sub-test cases to pass. The rest of the files in the vendor/ directory (amounting to approximately 110MB of code) are not relevant to the ptest suite. Therefore, I have decided not to install the entire vendor/ directory. Instead, I have documented this requirement in the README file. For users who wish to run the tests involving seccomp.json, they can manually add the file to the appropriate location and set the CONTAINER_SECCOMP_PROFILE environment variable to point to it.
the coming v2 will update the code

    >
    >>   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
    
<https://urldefense.com/v3/__http://test_runner.sh__;!!AjveYdw8EvQ!dcMJ_RyaY86e9xQF11g_sARozs9Xt8SSpHkb4w4wTRRTx3r-KoAo6OgkM7W2dNBjr4uIRv3_a6xuE9I2eX1AUfzkOR_JEp0q$>
    | 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 (#9016): 
https://lists.yoctoproject.org/g/meta-virtualization/message/9016
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