From 356522241c9e67db80ff3a607221d58735e0bdb9 Mon Sep 17 00:00:00 2001 From: Gonzalo Reyero Ferreras Date: Wed, 7 Feb 2024 04:04:06 -0600 Subject: [PATCH 1/2] Fix for platform-alteration-base-image test case. The original workaround to use a custom podman for this test case stopped working when we switched our debug pod's image from ubi8 to ubi9 due to libc missing in RHEL8.x based nodes (OCP 4.12.z and lower). This pre-built podman binary was used in this tc no matter the OCP version of the cluster. This commit will make the test case to use our ubi8-based podman only if the test suite is checking CNFs on OCPs 4.12.z or lower. For newer versions of OCP, the podman that's presinstalled in the nodes will be used, as it seems to work well again. We should remove this ugly workaround when 4.12 reaches EOL. The fix to build podman with ubi8 again is already merged in the partner repo here: https://github.com/test-network-function/cnf-certification-test-partner/pull/394 --- .../platform/cnffsdiff/fsdiff.go | 97 ++++++++--- .../platform/cnffsdiff/fsdiff_test.go | 152 +++++++++++++++++- cnf-certification-test/platform/suite.go | 2 +- 3 files changed, 219 insertions(+), 32 deletions(-) diff --git a/cnf-certification-test/platform/cnffsdiff/fsdiff.go b/cnf-certification-test/platform/cnffsdiff/fsdiff.go index a7a38b6d6..335657f4a 100644 --- a/cnf-certification-test/platform/cnffsdiff/fsdiff.go +++ b/cnf-certification-test/platform/cnffsdiff/fsdiff.go @@ -21,8 +21,9 @@ import ( "errors" "fmt" + "github.com/Masterminds/semver/v3" "github.com/test-network-function/cnf-certification-test/internal/clientsholder" - "github.com/test-network-function/cnf-certification-test/internal/log" + "github.com/test-network-function/cnf-certification-test/pkg/checksdb" "github.com/test-network-function/cnf-certification-test/pkg/stringhelper" "github.com/test-network-function/cnf-certification-test/pkg/testhelper" ) @@ -65,9 +66,11 @@ type fsDiffJSON struct { } type FsDiff struct { - result int - clientHolder clientsholder.Command - ctxt clientsholder.Context + check *checksdb.Check + result int + clientHolder clientsholder.Command + ctxt clientsholder.Context + useCustomPodman bool DeletedFolders []string ChangedFolders []string @@ -79,19 +82,54 @@ type FsDiffFuncs interface { GetResults() int } -func NewFsDiffTester(client clientsholder.Command, ctxt clientsholder.Context) *FsDiff { +func NewFsDiffTester(check *checksdb.Check, client clientsholder.Command, ctxt clientsholder.Context, ocpVersion string) *FsDiff { + useCustomPodman := shouldUseCustomPodman(check, ocpVersion) + check.LogDebug("Using custom podman: %v.", useCustomPodman) + return &FsDiff{ - clientHolder: client, - ctxt: ctxt, - result: testhelper.ERROR, + check: check, + clientHolder: client, + ctxt: ctxt, + result: testhelper.ERROR, + useCustomPodman: useCustomPodman, + } +} + +// Helper function that is used to check whether we should use the podman that comes preinstalled +// on each ocp node or the one that we've (custom) precompiled inside the debug pods that can only work in +// RHEL 8.x based ocp versions (4.12.z and lower). For ocp >= 4.13.0 this workaround should not be +// necessary. +func shouldUseCustomPodman(check *checksdb.Check, ocpVersion string) bool { + const ( + ocpForPreinstalledPodmanMajor = 4 + ocpForPreinstalledPodmanMinor = 13 + ) + + version, err := semver.NewVersion(ocpVersion) + if err != nil { + check.LogError("Failed to parse Openshift version %q. Using preinstalled podman.", ocpVersion) + // Use podman preinstalled in nodes as failover. + return false + } + + // Major versions > 4, use podman preinstalled in nodes. + if version.Major() > ocpForPreinstalledPodmanMajor { + return false + } + + if version.Major() == ocpForPreinstalledPodmanMajor { + return version.Minor() < ocpForPreinstalledPodmanMinor } + + // For older versions (< 3.), use podman preinstalled in nodes. + return false } -func intersectTargetFolders(src []string) []string { +func (f *FsDiff) intersectTargetFolders(src []string) []string { var dst []string for _, folder := range src { if stringhelper.StringInSlice(targetFolders, folder, false) { - log.Warn("Container's folder %q is altered.", folder) + f.check.LogWarn("Container's folder %q is altered.", folder) dst = append(dst, folder) } } @@ -99,7 +137,12 @@ func intersectTargetFolders(src []string) []string { } func (f *FsDiff) runPodmanDiff(containerUID string) (string, error) { - output, outerr, err := f.clientHolder.ExecCommandContainer(f.ctxt, fmt.Sprintf("chroot /host %s/podman diff --format json %s", tmpMountDestFolder, containerUID)) + podmanPath := "podman" + if f.useCustomPodman { + podmanPath = fmt.Sprintf("%s/podman", tmpMountDestFolder) + } + + output, outerr, err := f.clientHolder.ExecCommandContainer(f.ctxt, fmt.Sprintf("chroot /host %s diff --format json %s", podmanPath, containerUID)) if err != nil { return "", fmt.Errorf("can not execute command on container: %w", err) } @@ -110,16 +153,18 @@ func (f *FsDiff) runPodmanDiff(containerUID string) (string, error) { } func (f *FsDiff) RunTest(containerUID string) { - err := f.installCustomPodman() - if err != nil { - f.Error = err - f.result = testhelper.ERROR - return - } + if f.useCustomPodman { + err := f.installCustomPodman() + if err != nil { + f.Error = err + f.result = testhelper.ERROR + return + } - defer f.unmountCustomPodman() + defer f.unmountCustomPodman() + } - log.Info("Running \"podman diff\" for container id %s", containerUID) + f.check.LogInfo("Running \"podman diff\" for container id %s", containerUID) output, err := f.runPodmanDiff(containerUID) if err != nil { f.Error = err @@ -128,7 +173,7 @@ func (f *FsDiff) RunTest(containerUID string) { } // see if there's a match in the output - log.Debug("Podman diff output is %s", output) + f.check.LogDebug("Podman diff output is %s", output) diff := fsDiffJSON{} err = json.Unmarshal([]byte(output), &diff) @@ -137,8 +182,8 @@ func (f *FsDiff) RunTest(containerUID string) { f.result = testhelper.ERROR return } - f.DeletedFolders = intersectTargetFolders(diff.Deleted) - f.ChangedFolders = intersectTargetFolders(diff.Changed) + f.DeletedFolders = f.intersectTargetFolders(diff.Deleted) + f.ChangedFolders = f.intersectTargetFolders(diff.Changed) if len(f.ChangedFolders) != 0 || len(f.DeletedFolders) != 0 { f.result = testhelper.FAILURE } else { @@ -184,13 +229,13 @@ func (f *FsDiff) unmountDebugPartnerPodmanFolder() error { func (f *FsDiff) installCustomPodman() error { // We need to create the destination folder first. - log.Info("Creating temp folder %s", nodeTmpMountFolder) + f.check.LogInfo("Creating temp folder %s", nodeTmpMountFolder) if err := f.createNodeFolder(); err != nil { return err } // Mount podman from partner debug pod into /host/tmp/... - log.Info("Mouting %s into %s", partnerPodmanFolder, nodeTmpMountFolder) + f.check.LogInfo("Mouting %s into %s", partnerPodmanFolder, nodeTmpMountFolder) if mountErr := f.mountDebugPartnerPodmanFolder(); mountErr != nil { // We need to delete the temp folder previously created as mount point. if deleteErr := f.deleteNodeFolder(); deleteErr != nil { @@ -206,7 +251,7 @@ func (f *FsDiff) installCustomPodman() error { func (f *FsDiff) unmountCustomPodman() { // Unmount podman folder from host. - log.Info("Unmounting folder %s", nodeTmpMountFolder) + f.check.LogInfo("Unmounting folder %s", nodeTmpMountFolder) if err := f.unmountDebugPartnerPodmanFolder(); err != nil { // Here, there's no point on trying to remove the temp folder used as mount point, as // that probably won't work either. @@ -215,7 +260,7 @@ func (f *FsDiff) unmountCustomPodman() { return } - log.Info("Deleting folder %s", nodeTmpMountFolder) + f.check.LogInfo("Deleting folder %s", nodeTmpMountFolder) if err := f.deleteNodeFolder(); err != nil { f.Error = err f.result = testhelper.ERROR diff --git a/cnf-certification-test/platform/cnffsdiff/fsdiff_test.go b/cnf-certification-test/platform/cnffsdiff/fsdiff_test.go index e1fcc9478..0275920c2 100644 --- a/cnf-certification-test/platform/cnffsdiff/fsdiff_test.go +++ b/cnf-certification-test/platform/cnffsdiff/fsdiff_test.go @@ -14,7 +14,7 @@ // with this program; if not, write to the Free Software Foundation, Inc., // 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -package cnffsdiff_test +package cnffsdiff import ( "errors" @@ -23,8 +23,8 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/test-network-function/cnf-certification-test/cnf-certification-test/platform/cnffsdiff" "github.com/test-network-function/cnf-certification-test/internal/clientsholder" + "github.com/test-network-function/cnf-certification-test/pkg/checksdb" "github.com/test-network-function/cnf-certification-test/pkg/testhelper" ) @@ -98,6 +98,9 @@ func TestRunTest(t *testing.T) { }, } + check := &checksdb.Check{} + ocpVersion := "4.13.0" + for _, tc := range testCases { chm := &ClientHoldersMock{ stdout: tc.clientStdOut, @@ -105,7 +108,7 @@ func TestRunTest(t *testing.T) { err: tc.clientErr, } - fsdiff := cnffsdiff.NewFsDiffTester(chm, clientsholder.Context{}) + fsdiff := NewFsDiffTester(check, chm, clientsholder.Context{}, ocpVersion) fsdiff.RunTest("fakeUID") assert.Equal(t, tc.expectedResult, fsdiff.GetResults()) } @@ -180,8 +183,11 @@ func TestRunTestMountFolderErrors(t *testing.T) { }, } + check := &checksdb.Check{} + ocpVersion := "4.13.0" + for _, tc := range testCases { - fsdiff := cnffsdiff.NewFsDiffTester(tc.mockedClientshHolder, clientsholder.Context{}) + fsdiff := NewFsDiffTester(check, tc.mockedClientshHolder, clientsholder.Context{}, ocpVersion) fsdiff.RunTest("fakeUID") assert.Equal(t, testhelper.ERROR, fsdiff.GetResults()) assert.Equal(t, fsdiff.Error.Error(), tc.expectedError) @@ -262,10 +268,146 @@ func TestRunTestUnmountFolderErrors(t *testing.T) { }, } + check := &checksdb.Check{} + ocpVersion := "4.13.0" + for _, tc := range testCases { - fsdiff := cnffsdiff.NewFsDiffTester(tc.mockedClientshHolder, clientsholder.Context{}) + fsdiff := NewFsDiffTester(check, tc.mockedClientshHolder, clientsholder.Context{}, ocpVersion) fsdiff.RunTest("fakeUID") assert.Equal(t, testhelper.ERROR, fsdiff.GetResults()) assert.Equal(t, fsdiff.Error.Error(), tc.expectedError) } } + +func Test_shouldUseCustomPodman(t *testing.T) { + type args struct { + check *checksdb.Check + ocpVersion string + } + tests := []struct { + name string + args args + expected bool + }{ + { + name: "empty ocp version", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "", + }, + expected: false, + }, + { + name: "invalid ocp version", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "asdf.asdf", + }, + expected: false, + }, + { + name: "ocp version 4.11", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.11", + }, + expected: true, + }, + { + name: "ocp version 4.11.0", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.11.0", + }, + expected: true, + }, + { + name: "ocp version 4.11.53", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.11.53", + }, + expected: true, + }, + { + name: "ocp version 4.12", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.12", + }, + expected: true, + }, + { + name: "ocp version 4.12.87", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.12.87", + }, + expected: true, + }, + { + name: "ocp version 4.13", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.13", + }, + expected: false, + }, + { + name: "ocp version 4.13.0", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.13.0", + }, + expected: false, + }, + { + name: "ocp version 4.13.873", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.13.873", + }, + expected: false, + }, + { + name: "ocp version 4.15", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.15", + }, + expected: false, + }, + { + name: "ocp version 5.0", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "5.0.0", + }, + expected: false, + }, + // Nightlies are a bit special due to that z-stream num which is always "0-0" in openshift... + { + name: "ocp 4.12 nightly version 4.12.0-0.nightly-2024-01-22-220616", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.12.0-0.nightly-2024-01-22-220616", + }, + expected: true, + }, + { + name: "ocp 4.13 nightly version 4.13.0-0.nightly-2024-01-22-220616", + args: args{ + check: &checksdb.Check{}, + ocpVersion: "4.13.0-0.nightly-2024-01-22-220616", + }, + expected: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := shouldUseCustomPodman(tt.args.check, tt.args.ocpVersion); got != tt.expected { + t.Errorf("shouldUseCustomPodman() = %v, expected %v", got, tt.expected) + } + }) + } +} diff --git a/cnf-certification-test/platform/suite.go b/cnf-certification-test/platform/suite.go index 970e42b21..bdae8d4de 100644 --- a/cnf-certification-test/platform/suite.go +++ b/cnf-certification-test/platform/suite.go @@ -222,7 +222,7 @@ func testContainersFsDiff(check *checksdb.Check, env *provider.TestEnvironment) debugPod := env.DebugPods[cut.NodeName] ctxt := clientsholder.NewContext(debugPod.Namespace, debugPod.Name, debugPod.Spec.Containers[0].Name) - fsDiffTester := cnffsdiff.NewFsDiffTester(clientsholder.GetClientsHolder(), ctxt) + fsDiffTester := cnffsdiff.NewFsDiffTester(check, clientsholder.GetClientsHolder(), ctxt, env.OpenshiftVersion) fsDiffTester.RunTest(cut.UID) switch fsDiffTester.GetResults() { case testhelper.SUCCESS: From 7634fadba898814f7f2b7fcaa686e3917ee9af9f Mon Sep 17 00:00:00 2001 From: Gonzalo Reyero Ferreras Date: Wed, 7 Feb 2024 04:49:52 -0600 Subject: [PATCH 2/2] UT fixes: adjusted OCP version. The UT that test the folder creation/mount issues of podman need OCP ver 4.12 or lower to run. --- cnf-certification-test/platform/cnffsdiff/fsdiff_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cnf-certification-test/platform/cnffsdiff/fsdiff_test.go b/cnf-certification-test/platform/cnffsdiff/fsdiff_test.go index 0275920c2..3a1c75f0c 100644 --- a/cnf-certification-test/platform/cnffsdiff/fsdiff_test.go +++ b/cnf-certification-test/platform/cnffsdiff/fsdiff_test.go @@ -184,7 +184,7 @@ func TestRunTestMountFolderErrors(t *testing.T) { } check := &checksdb.Check{} - ocpVersion := "4.13.0" + ocpVersion := "4.12.0" for _, tc := range testCases { fsdiff := NewFsDiffTester(check, tc.mockedClientshHolder, clientsholder.Context{}, ocpVersion) @@ -269,7 +269,7 @@ func TestRunTestUnmountFolderErrors(t *testing.T) { } check := &checksdb.Check{} - ocpVersion := "4.13.0" + ocpVersion := "4.12.0" for _, tc := range testCases { fsdiff := NewFsDiffTester(check, tc.mockedClientshHolder, clientsholder.Context{}, ocpVersion)