From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id E6D2090798 for ; Tue, 2 Apr 2024 16:55:55 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D0735810B for ; Tue, 2 Apr 2024 16:55:55 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 2 Apr 2024 16:55:54 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id AF9DF4476F for ; Tue, 2 Apr 2024 16:55:54 +0200 (CEST) From: Max Carrara To: pve-devel@lists.proxmox.com Date: Tue, 2 Apr 2024 16:55:15 +0200 Message-Id: <20240402145523.683008-4-m.carrara@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240402145523.683008-1-m.carrara@proxmox.com> References: <20240402145523.683008-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.022 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH v5 pve-storage 03/11] test: cephconfig: add regression test for Ceph config parser & writer X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Apr 2024 14:55:56 -0000 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 Suggested-by: Fabian Grünbichler --- 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 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/(? ['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