Merge pull request #7686 from VersatusHPC/backport-7650-to-2.18

[Backport 2.18] fix(xcat-core): run every *.t test in the GitHub CI

github_action_xcat_test.pl

@@ -27,6 +27,26 @@ my $check_result_str="``CI CHECK RESULT`` : ";
 my $last_func_start = timelocal(localtime());
 my $GITHUB_API = "https://api.github.com";
 
+# The workflow starts us in the checked out source tree. The unit tests under
+# xCAT-test/unit resolve xCAT modules and fixture files relative to that tree
+# through FindBin, so they can only be run from a source tree -- but the tree
+# does not survive the build. build-ubunturepo sets
+#     local_core_repo_path="$curdir/../../xcat-core"
+# which, under the work/<repo>/<repo> layout GitHub checks out into, resolves
+# to the checkout's own parent, and it then rm -rf's that path to make room for
+# the apt repository. So take a copy of the tree before building and run the
+# unit tests out of the copy.
+my $srcdir = getcwd();
+my $unitsrc = ($ENV{'RUNNER_TEMP'} ? $ENV{'RUNNER_TEMP'} : "/tmp") . "/xcat-core-unitsrc";
+
+# Cases whose output is printed even when they pass. A passing case is normally
+# silent, which is the right default for 250 of them but makes it impossible to
+# tell from the log whether a case did real work or skipped everything -- a
+# distinction that matters for cases wrapping prove, since prove exits 0 either
+# way. Name a case here to see its output; empty the list for the quiet
+# behaviour.
+my @verbose_cases = qw(integration_tests);
+
 #--------------------------------------------------------
 # Fuction name: runcmd
 # Description:  run a command after 'cmd' label in one case
@@ -270,6 +290,29 @@ sub send_back_comment{
     }
 }
 
+#--------------------------------------------------------
+# Fuction name: preserve_source_tree
+# Description:  Copy the checkout aside before the build destroys it, so the
+#               unit tests still have a source tree to run against afterwards.
+#               Must be called before build_xcat_core().
+# Attributes:
+# Return code:  0 Success  1 Failed
+#--------------------------------------------------------
+sub preserve_source_tree{
+    my $cmd = "rm -rf $unitsrc && cp -a $srcdir $unitsrc";
+    print "[preserve_source_tree] running $cmd\n";
+    my @output = runcmd("$cmd");
+    if($::RUNCMD_RC){
+        print RED "[preserve_source_tree] $cmd ....[Failed]\n";
+        print Dumper \@output;
+        return 1;
+    }
+
+    @output = runcmd("ls $unitsrc/xCAT-test/unit/*.t | wc -l");
+    print "[preserve_source_tree] preserved $srcdir in $unitsrc ($output[0] unit tests)\n";
+    return 0;
+}
+
 #--------------------------------------------------------
 # Fuction name: build_xcat_core
 # Description:
@@ -384,6 +427,34 @@ sub install_xcat{
 }
 
 
+#--------------------------------------------------------
+# Fuction name: run_unit_tests
+# Description:  Run every Perl unit test under xCAT-test/unit with prove.
+#               Runs against the pre-build copy of the source tree taken by
+#               preserve_source_tree(): the tests reach for xCAT modules and
+#               fixture files through FindBin, so the installed copy under
+#               /opt/xcat/share/xcat/tools/autotest is not enough for them.
+# Attributes:
+# Return code:  0 all tests passed, 1 otherwise
+#--------------------------------------------------------
+sub run_unit_tests{
+    my $cmd = "cd $unitsrc && prove -r xCAT-test/unit";
+    print "[run_unit_tests] running $cmd\n";
+    my @output = runcmd("$cmd");
+    print Dumper \@output;
+    if($::RUNCMD_RC){
+        print RED "[run_unit_tests] $cmd ....[Failed]\n";
+        $check_result_str .= "> **UNIT TESTS Failed** : Please click ``Details`` label in ``Merge pull request`` box for detailed information\n";
+        print $check_result_str;
+        return 1;
+    }
+
+    print "[run_unit_tests] $cmd ....[Pass]\n";
+    $check_result_str .= "> **UNIT TESTS Successful**\n";
+    print $check_result_str;
+    return 0;
+}
+
 #--------------------------------------------------------
 # Fuction name: check_syntax
 # Description:
@@ -497,12 +568,16 @@ sub run_fast_regression_test{
         $cmd = "sudo bash -c '. /etc/profile.d/xcat.sh &&  xcattest -f $conf_file -t $case'";
         print "[run_fast_regression_test] run $x: $cmd\n";
         @output = runcmd("$cmd");
-        #print Dumper \@output;
+        my $verbose = grep { $_ eq $case } @verbose_cases;
+        if($verbose){
+            print "[run_fast_regression_test] output of $case (listed in \@verbose_cases):\n";
+            print Dumper \@output;
+        }
         for(my $i = $#output; $i>-1; --$i){
             if($output[$i] =~ /------END::(.+)::Failed/){
                 push @failcase, $1;
                 ++$failnum;
-                print Dumper \@output;
+                print Dumper \@output unless($verbose);
                 last;
              }elsif ($output[$i] =~ /------END::(.+)::Passed/){
                 ++$passnum;
@@ -566,6 +641,15 @@ print Dumper \@ipinfo;
 
 #Start to build xcat core
 
+#Save the source tree before the build deletes it, the unit tests need it later
+print GREEN "\n------ Preserving the source tree for the unit tests ------\n";
+$rst = preserve_source_tree();
+if($rst){
+    print RED "Preserving the source tree failed\n";
+    exit $rst;
+}
+mark_time("preserve_source_tree");
+
 print GREEN "\n------ Building xCAT core package ------\n";
 $rst = build_xcat_core();
 if($rst){
@@ -583,6 +667,16 @@ if($rst){
 }
 mark_time("install_xcat");
 
+#Run the xCAT-test unit tests. They need the perl dependencies xCAT pulls in
+#(Net::DNS, XML::Simple) and a usable xCAT database, so they run after install.
+print GREEN "\n------Running xCAT-test unit tests ------\n";
+$rst = run_unit_tests();
+if($rst){
+    print RED "Run of xCAT-test unit tests failed\n";
+    exit $rst;
+}
+mark_time("run_unit_tests");
+
 #Check the syntax of changing code
 print GREEN "\n------ Checking the syntax of changed code------\n";
 $rst = check_syntax();

xCAT-test/autotest/testcase/dhcp_unit/cases0

@@ -1,8 +0,0 @@
-start:dhcp_backend_unit_tests
-description:Run the DHCP backend Perl unit tests through xcattest
-os:Linux
-label:mn_only,ci_test,dhcp,unit,dhcp_unit
-cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT /opt/xcat/share/xcat/tools/autotest/unit/dhcp_*.t
-check:rc==0
-check:output=~All tests successful
-end

xCAT-test/autotest/testcase/integration/cases0

@@ -0,0 +1,8 @@
+start:integration_tests
+description:Run the xCAT-test integration tests (xCAT-test/integration) against the installed management node
+os:Linux
+label:mn_only,ci_test,integration
+cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT -r /opt/xcat/share/xcat/tools/autotest/integration
+check:rc==0
+check:output=~Files=3
+end

xCAT-test/autotest/testcase/ipmi_unit/cases0

@@ -1,8 +0,0 @@
-start:ipmi_rakp2_unit_tests
-description:Run the IPMI RAKP2 Perl unit tests through xcattest
-os:Linux
-label:mn_only,ci_test,ipmi,unit,ipmi_unit
-cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT /opt/xcat/share/xcat/tools/autotest/unit/ipmi_*.t
-check:rc==0
-check:output=~All tests successful
-end

xCAT-test/autotest/testcase/xcatprobe_unit/cases0

@@ -1,8 +0,0 @@
-start:xcatprobe_tcp_listener_unit_tests
-description:Run the xCAT probe TCP listener Perl unit tests through xcattest
-os:Linux
-label:mn_only,ci_test,xcatprobe,unit,xcatprobe_unit
-cmd:prove -I/opt/xcat/probe/lib/perl /opt/xcat/share/xcat/tools/autotest/unit/probe_utils_tcp_listener.t
-check:rc==0
-check:output=~All tests successful
-end

xCAT-test/debian/install

@@ -4,3 +4,4 @@ share/man/man1/* opt/xcat/share/man/man1
 share/doc/man1/* opt/xcat/share/doc/man1
 autotest opt/xcat/share/xcat/tools
 unit opt/xcat/share/xcat/tools/autotest
+integration opt/xcat/share/xcat/tools/autotest

xCAT-test/integration/README.md

@@ -0,0 +1,80 @@
+# xCAT-test/integration
+
+Integration tests. These run against an **installed management node** -- they need a
+real xCAT installation, and depending on the test a populated `/install`, a service
+binary they can execute, or a live daemon.
+
+They are driven by `xcattest` through the testcase in
+`../autotest/testcase/integration/`, which proves the copy installed by the
+`xcat-test` package:
+
+```
+prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT \
+      -r /opt/xcat/share/xcat/tools/autotest/integration
+```
+
+Run the case by hand on an MN with:
+
+```
+xcattest -f <cluster.conf> -t integration_tests
+```
+
+The case carries the `ci_test` label, so it also runs on every pull request: the
+`xcat_test` GitHub Actions workflow installs and configures xCAT on the runner, which
+makes that runner a (single node) management node, and then runs every `ci_test` case
+against it.
+
+Note the `-I` flags: unlike the unit tests these run from the installed location, so
+they pick up xCAT modules from `/opt/xcat/lib/perl` rather than from a source tree.
+
+The case checks `rc==0` and `output=~Files=3`. The second assertion is there because
+`prove` exits 0 both when tests pass and when they all skip, so `rc==0` alone would let
+the case report green having run nothing. Matching `Files=3` proves `prove` actually
+found all three files, which catches a packaging regression or a file being renamed
+without the count being updated here. **Add to that number when you add a test.** A
+missing directory is already caught by `rc==0` -- `prove -r` on a path that does not
+exist exits 2.
+
+Note also that `github_action_xcat_test.pl` invokes each case through `sudo`, so in CI
+these tests run as **root** while the unit tests run unprivileged. That is the right
+way round -- integration tests legitimately need to write to places like `/etc/kea`,
+whereas running the unit tests as root would let permission-related assertions pass
+for the wrong reason.
+
+## What belongs here
+
+A test belongs in `integration/` when it needs something the checkout cannot provide:
+
+| Test | Requires |
+| --- | --- |
+| `copycds_packages_integrity.t` | `/install` populated by a real `copycds` |
+| `dhcp_kea_config_validation.t` | a `kea-dhcp4` binary that can read the generated config |
+| `dhcp_kea_control_agent_smoke.t` | live `kea-dhcp4` and `kea-ctrl-agent`, root, and the Kea host-commands hook |
+
+## Environment guards
+
+Tests here still guard with `plan skip_all` so the case does not fail on a node that
+legitimately lacks the dependency -- an MN with no Kea installed should skip the Kea
+tests, not go red. A skip in this directory is therefore expected and normal, which is
+precisely why these tests do not belong alongside the unit tests.
+
+Which tests actually run consequently varies by node. On a GitHub runner, for example,
+`/install` is empty so `copycds_packages_integrity.t` skips, while
+`dhcp_kea_config_validation.t` does run because the case executes as root and can
+therefore validate from `/etc/kea`.
+
+`dhcp_kea_control_agent_smoke.t` is opt-in on top of that:
+
+```perl
+plan skip_all => 'set XCAT_KEA_LIVE_SMOKE=1 to run live Kea daemon smoke test'
+  unless $ENV{XCAT_KEA_LIVE_SMOKE};
+```
+
+It starts real Kea daemons, so it stays off unless asked for. Do not enable it on a
+node whose DHCP service is in use.
+
+## What does not belong here
+
+Anything that only needs the checkout. Those go in [`../unit`](../unit/README.md) and
+run on every pull request, which is much faster feedback than waiting for a cluster
+test.

xCAT-test/unit/README.md

@@ -0,0 +1,60 @@
+# xCAT-test/unit
+
+Unit tests. These run against the **source tree only** -- no xCAT installation, no
+running daemons, no management node.
+
+They are executed on every pull request by the `xcat_test` GitHub Actions workflow,
+which calls `run_unit_tests()` in `github_action_xcat_test.pl`:
+
+```
+prove -r xCAT-test/unit
+```
+
+You can run exactly the same thing from a clean checkout:
+
+```
+cd <xcat-core checkout>
+prove -r xCAT-test/unit
+```
+
+## What belongs here
+
+A test belongs in `unit/` when everything it needs is in the checkout: plugin and
+library sources, kickstart/preseed/subiquity templates, postscripts, packaging
+metadata. Such a test asserts on rendered output or module logic and reaches the
+repository root through `FindBin`:
+
+```perl
+use FindBin;
+use lib "$FindBin::Bin/../../perl-xCAT";
+use lib "$FindBin::Bin/../../xCAT-server/lib/perl";
+```
+
+Because of those `FindBin` paths the tests only work from a source tree. The copy
+installed under `/opt/xcat/share/xcat/tools/autotest/unit` is not a substitute --
+`../..` resolves to `/opt/xcat/share/xcat/tools` there and the tests die or silently
+skip. The CI takes a copy of the checkout before the build for this reason; see
+`preserve_source_tree()`.
+
+## What does not belong here
+
+Anything that needs an installed xCAT, a populated `/install`, a real service binary
+or a live daemon. Those go in [`../integration`](../integration/README.md) and run on
+a management node through `xcattest`. Both suites run on every pull request -- the
+workflow installs xCAT on the runner and then runs the `ci_test` cases against it --
+so putting a test in `integration/` does not cost it CI coverage. What differs is what
+each suite is allowed to depend on, and that unit tests also run standalone from a
+bare checkout with no xCAT at all.
+
+The distinction matters because a test that needs an absent environment does not fail
+-- it calls `plan skip_all` and reports as skipped. A handful of those in a suite of
+several hundred assertions is easy to stop reading. Keeping the two kinds in separate
+directories means a skip in `unit/` is a real signal rather than routine noise.
+
+Guarding on a *source* file, on the other hand, is fine and common here:
+
+```perl
+plan skip_all => "compute.subiquity.tmpl not found" unless -f $tmpl_path;
+```
+
+That guard never fires when the tree is intact.

xCAT-test/unit/ubuntu_subiquity_template.t

@@ -15,7 +15,7 @@ like($tmpl, qr/^#cloud-config/, 'template starts with #cloud-config');
 like($tmpl, qr/autoinstall:/, 'template has autoinstall: key');
 like($tmpl, qr/version:\s*1/, 'template has version: 1');
 
-unlike($tmpl, qr/^\s*identity:/m, 'template must not have identity section (use user-data instead)');
+like($tmpl, qr/^\s*identity:/m, 'template has an identity section so subiquity does not prompt');
 like($tmpl, qr/kernel:/, 'template has kernel section');
 like($tmpl, qr/package:\s*linux-generic/, 'template specifies linux-generic kernel');
 like($tmpl, qr/#UBUNTU_SUBIQUITY_APT_CONFIG#/, 'template renders apt section from osimage context');
@@ -38,7 +38,8 @@ like($tmpl, qr/printf ''%s\\n'' ''GRUB_CMDLINE_LINUX="#TABLEBLANKOKAY:bootparams
 like($tmpl, qr/\/target\/etc\/netplan\/00-xcat-install\.yaml/, 'template writes an xCAT-owned target netplan file');
 like($tmpl, qr/installnic="#TABLE:noderes:\$NODE:installnic#"/, 'target netplan uses node installnic');
 like($tmpl, qr/installmac="#TABLE:mac:\$NODE:mac#"/, 'target netplan uses node MAC');
-like($tmpl, qr/installmac="\$\(printf ''%s'' "\$\{installmac\}" \| tr ''A-F'' ''a-f''\)"/, 'target netplan normalizes MAC case');
+like($tmpl, qr/installmac="\$\(printf ''%s'' "\$\{installmac\}".*\| tr ''A-F'' ''a-f''\)"/, 'target netplan normalizes MAC case');
+like($tmpl, qr/installmac="\$\(printf ''%s'' "\$\{installmac\}" \| cut -d''\|'' -f1 \| cut -d''!'' -f1/, 'target netplan strips mac table suffixes before matching');
 like($tmpl, qr/printf ''%s\\n'' "network:" "  version: 2" "  ethernets:" "    xcat-install:" "      match:" "        macaddress: \\"\$\{installmac\}\\"" "      set-name: \$\{installnic\}" "      dhcp4: true" >\/target\/etc\/netplan\/00-xcat-install\.yaml;/, 'target netplan printf stays on one shell line');
 like($tmpl, qr/"        macaddress: \\"\$\{installmac\}\\""/, 'target netplan matches by MAC address');
 like($tmpl, qr/"      set-name: \$\{installnic\}"/, 'target netplan sets the expected installnic name');

xCAT-test/xCAT-test.spec

@@ -63,6 +63,7 @@ chmod 644 $RPM_BUILD_ROOT/%{prefix}/share/doc/man1/*
 
 cp -r autotest $RPM_BUILD_ROOT/%{prefix}/share/xcat/tools
 cp -r unit $RPM_BUILD_ROOT/%{prefix}/share/xcat/tools/autotest
+cp -r integration $RPM_BUILD_ROOT/%{prefix}/share/xcat/tools/autotest
 
 
 %clean