[pve-devel] [PATCH v5 pve-storage 03/11] test: cephconfig: add regression test for Ceph config parser & writer

[Max Carrara <m.carrara@proxmox.com>]

Building on the previously declared cases, this test constructs a
config hash akin to the one the parser returns by invoking the
`ceph-conf` command.

The cases which `ceph-conf` cannot handle are marked with a special
key. If the key is provided, invocatinos to `ceph-conf` are expected
to fail. Furthermore, our parser is expected to behave differently for
those cases for simplicity's sake.

Should `ceph-conf` suddenly succeed when it parses a case where it is
expected to fail, or should it fail to parse a case where it is
expected to succeed, the test itself will fail. This way we can detect
any upstream changes to Ceph's config format and act accordingly.

Naturally, this requires the "ceph-common" package to be available
during builds, which is added to "debian/control" as build dep.

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Changes v4 --> v5:
  * new

 debian/control                             |   1 +
 src/PVE/test/ceph_conf_parse_write_test.pl | 241 +++++++++++++++++++++
 2 files changed, 242 insertions(+)

diff --git a/debian/control b/debian/control
index d0b1d95..90a895e 100644
--- a/debian/control
+++ b/debian/control
@@ -3,6 +3,7 @@ Section: perl
 Priority: optional
 Maintainer: Proxmox Support Team <support@proxmox.com>
 Build-Depends: debhelper-compat (= 13),
+               ceph-common (>= 12.2~),
                libfile-chdir-perl,
                libposix-strptime-perl,
                libpve-access-control,
diff --git a/src/PVE/test/ceph_conf_parse_write_test.pl b/src/PVE/test/ceph_conf_parse_write_test.pl
index 1d5e506..e3116c6 100755
--- a/src/PVE/test/ceph_conf_parse_write_test.pl
+++ b/src/PVE/test/ceph_conf_parse_write_test.pl
@@ -8,13 +8,182 @@ use lib qw(../..);
 use Test::More;
 
 use PVE::CephConfig;
+use PVE::Tools qw(run_command file_set_contents);
 
 
+sub with_tmp_ceph_conf_file {
+    my ($raw_content, $func) = @_;
+
+    my $cfg_filename = "/tmp/ceph-test-$$.conf";
+    my $perm = 0600;
+
+    my $rval = eval {
+	file_set_contents($cfg_filename, $raw_content, $perm);
+	return $func->($cfg_filename);
+    };
+
+    my $err = $@;
+
+    unlink $cfg_filename or warn "Failed to unlink temporary file '$cfg_filename' - $!\n";
+
+    die $err if $err;
+
+    return $rval;
+}
+
+# In an ideal world, `ceph-conf` would be able to just generate some JSON output
+# via the '--format JSON' flag, but that unfortunately doesn't do anything.
+# Whe therefore have to invoke `ceph-conf` for each key in each section here.
+#
+# The returned hash contains two keys:
+#   cfg => The Ceph config hash that is constructed by invoking the CLI
+#   cmd_infos => Information of the command invoked for each key of each
+#		 section, in the form of:
+#	{
+#	    section => {
+#		key => {
+#		    cmd => The invoked command,
+#		    exit_code => The command's exit code,
+#		    stdout => Its stdout,
+#		    stderr => Its stderr,
+#		},
+#		[...]
+#	    },
+#	    [...]
+#	}
+sub query_cfg_via_ceph_conf_cli {
+    my ($cfg_filename, $expected_cfg) = @_;
+
+    my $queried_cfg = {};
+
+    my $section_cmd_info = invoke_ceph_conf_cli_for_sections($cfg_filename);
+    for my $section (split(/\n/, $section_cmd_info->{stdout})) {
+	$queried_cfg->{$section} = {};
+    }
+
+    my $key_cmd_infos = {};
+
+    for my $section (sort keys $expected_cfg->%*) {
+	for my $key (sort keys $expected_cfg->{$section}->%*) {
+	    my $value = $expected_cfg->{$section}->{$key};
+
+	    my $output = invoke_ceph_conf_cli_for_key($cfg_filename, $section, $key);
+
+	    $queried_cfg->{$section}->{$key} = $output->{stdout};
+	    $key_cmd_infos->{$section}->{$key} = $output;
+	}
+    }
+
+    return {
+	cfg => $queried_cfg,
+	section_cmd_info => $section_cmd_info,
+	key_cmd_infos => $key_cmd_infos,
+    };
+}
+
+sub invoke_ceph_conf_cli_for_sections {
+    my ($cfg_filename) = @_;
+
+    my $cmd_info = {
+	cmd => ['ceph-conf', '-c', $cfg_filename, '--list-all-sections'],
+	exit_code => -1,
+	stdout => '',
+	stderr => '',
+    };
+
+    return invoke_ceph_conf_cli($cmd_info);
+}
+
+sub invoke_ceph_conf_cli_for_key {
+    my ($cfg_filename, $section, $key) = @_;
+
+    # Need to escape (not escaped) comment literals here, otherwise the CLI
+    # will fail to look up the key
+    $key =~ s/(?<!\\)([;#])/\\$1/g;
+
+    my $cmd_info = {
+	cmd => ['ceph-conf', '-c', $cfg_filename, '-s', $section, '--lookup', $key],
+	exit_code => -1,
+	stdout => '',
+	stderr => '',
+    };
+
+    return invoke_ceph_conf_cli($cmd_info);
+}
+
+sub invoke_ceph_conf_cli {
+    my ($cmd_info) = @_;
+
+    my $exit_code = eval {
+	run_command(
+	    $cmd_info->{cmd},
+	    noerr => 1,
+	    outfunc => sub { $cmd_info->{stdout} .= "$_[0]\n"; },
+	    errfunc => sub { $cmd_info->{stderr} .= "$_[0]\n"; },
+	)
+    };
+    $cmd_info->{exit_code} = $exit_code;
+
+    if ($@) {
+	my $errmsg = "Fatal error while invoking 'ceph-conf' (exit code: $exit_code) - $@\n\n";
+
+	if ($cmd_info->{stdout}) {
+	    $errmsg .= "stdout:\n" . $cmd_info->{stdout};
+	}
+
+	if ($cmd_info->{stderr}) {
+	    $errmsg .= "stderr:\n" . $cmd_info->{stderr};
+	}
+
+	die "$errmsg\n";
+    }
+
+    # remove trailing newlines for easier comparison later
+    $cmd_info->{stdout} =~ s/\n$//;
+    $cmd_info->{stderr} =~ s/\n$//;
+
+    return $cmd_info;
+}
+
+# Because `ceph-conf` doesn't have a separate exit code for parse failures,
+# this checks if all invocations failed and printed something to stderr.
+sub has_parse_failure {
+    my ($query_result) = @_;
+
+    my $section_cmd_info = $query_result->{section_cmd_info};
+
+    if ($section_cmd_info->{stderr} eq '' || $section_cmd_info->{exit_code} == 0) {
+	return 0;
+    }
+
+    my $key_cmd_infos = $query_result->{key_cmd_infos};
+
+    for my $section (keys $key_cmd_infos->%*) {
+	for my $key (keys $key_cmd_infos->{$section}->%*) {
+	    my $cmd_info = $key_cmd_infos->{$section}->{$key};
+	    my $failed = $cmd_info->{stderr} ne '' && $cmd_info->{exit_code} != 0;
+
+	    return 0 if !$failed;
+	}
+    }
+
+    return 1;
+}
+
 # An array of test cases.
 # Each test case is comprised of the following keys:
 #   description	  => to identify a single test
 #   expected_cfg  => the hash that parse_ceph_config should return
 #   raw	          => the raw content of the file to test
+#
+# Some cases also contain the optional 'expect_cli_parse_fail' key.
+# If supplied, this marks the case to be expected to fail for Ceph's
+# `ceph-conf` CLI interface that is used to check for regressions.
+#
+# Should a case that's expected to fail suddenly pass, or should
+# it fail when it's supposed to pass, the `ceph-conf` CLI test will
+# fail. This means that the upstream config grammar has most likely
+# changed.
 my $tests = [
     {
 	description => 'empty file',
@@ -24,6 +193,7 @@ my $tests = [
     },
     {
 	description => 'file without section',
+	expect_cli_parse_fail => 1,
 	expected_cfg => {},
 	raw => <<~EOF,
 	While Ceph's format doesn't allow this, we do, because it makes things simpler
@@ -68,6 +238,7 @@ my $tests = [
     },
     {
 	description => 'single section, section header with preceding whitespace',
+	expect_cli_parse_fail => 1,
 	expected_cfg => {
 	    foo => {
 		bar => 'baz',
@@ -93,6 +264,7 @@ my $tests = [
     {
 	description => 'single section, section header ' .
 	    'with preceding whitespace and comment',
+	expect_cli_parse_fail => 1,
 	expected_cfg => {
 	    foo => {
 		bar => 'baz',
@@ -105,6 +277,7 @@ my $tests = [
     },
     {
 	description => 'single section, arbitrary text before section',
+	expect_cli_parse_fail => 1,
 	expected_cfg => {
 	    foo => {
 		bar => 'baz',
@@ -120,6 +293,7 @@ my $tests = [
     },
     {
 	description => 'single section, invalid key-value pairs',
+	expect_cli_parse_fail => 1,
 	expected_cfg => {
 	    foo => {
 		bar => 'baz',
@@ -232,6 +406,7 @@ my $tests = [
     {
 	description => 'single section, keys with quoted values, '
 	    . 'comment literals within quotes',
+	expect_cli_parse_fail => 1,
 	expected_cfg => {
 	    foo => {},
 	},
@@ -717,6 +892,71 @@ my $tests = [
     },
 ];
 
+sub test_ceph_cli {
+    my ($case) = @_;
+
+    my $desc = "ceph-conf cli: $case->{description}";
+
+    # subtest descriptions
+    my $desc_format = "ceph-conf cli (format): $case->{description}";
+    my $desc_comparison = "ceph-conf cli (comparison): $case->{description}";
+
+    my $query_result = eval {
+	return with_tmp_ceph_conf_file($case->{raw}, sub {
+	    my ($cfg_filename) = @_;
+	    return query_cfg_via_ceph_conf_cli($cfg_filename, $case->{expected_cfg});
+	});
+    };
+    die "failed to query test ceph.conf file: $@\n" if $@;
+
+    my $cli_may_fail = defined($case->{expect_cli_parse_fail}) && $case->{expect_cli_parse_fail};
+
+    subtest $desc => sub {
+	plan(tests => 2);
+
+	my $did_fail = has_parse_failure($query_result);
+
+	if ($cli_may_fail) {
+	    if ($did_fail) {
+		pass($desc_format);
+		pass($desc_comparison);
+	    } else {
+		fail($desc_format);
+
+		diag("=== Call to ceph-conf succeeded unexpectedly - did upstream change? ===");
+		diag(explain($query_result));
+
+		fail($desc_comparison);
+	    }
+
+	    return;
+	}
+
+	if ($did_fail) {
+	    fail($desc_format);
+
+	    diag("=== Call to ceph-conf failed unexpectedly - did upstream change? ===");
+	    diag(explain($query_result));
+
+	    fail($desc_comparison);
+
+	    return;
+	}
+
+	pass($desc_format);
+
+	if (!is_deeply($query_result->{cfg}, $case->{expected_cfg}, $desc_comparison)) {
+	    diag("=== Expected ===");
+	    diag(explain($case->{expected_cfg}));
+	    diag("=== Got ===");
+	    diag(explain($query_result->{cfg}));
+	    diag("=== Diagnostic Output ===");
+	    diag(explain($query_result->{section_cmd_info}));
+	    diag(explain($query_result->{key_cmd_infos}));
+	}
+    };
+}
+
 sub test_parse_ceph_config {
     my ($case) = @_;
 
@@ -771,6 +1011,7 @@ sub test_write_ceph_config {
 
 sub main {
     my $test_subs = [
+	\&test_ceph_cli,
 	\&test_parse_ceph_config,
 	\&test_write_ceph_config,
     ];
-- 
2.39.2