From f371ebc02c994555ee303da1e2c1bdd3b2e30bc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Leszczy=C5=84ski?= <2000michal@wp.pl> Date: Tue, 17 Dec 2024 14:47:47 +0100 Subject: [PATCH 1/3] fix(backup_test): add missing 'Integration' suffix to tests Some tests were missing the Integration suffix in their names. This resulted in not including them in the 'make pkg-integration-test' command used when running tests on gh actions. --- pkg/service/backup/service_backup_integration_test.go | 4 ++-- pkg/service/backup/service_deduplicate_integration_test.go | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/service/backup/service_backup_integration_test.go b/pkg/service/backup/service_backup_integration_test.go index e9146dca0..ed0b8f4c2 100644 --- a/pkg/service/backup/service_backup_integration_test.go +++ b/pkg/service/backup/service_backup_integration_test.go @@ -2462,7 +2462,7 @@ func TestBackupAlternatorIntegration(t *testing.T) { } } -func TestBackupViews(t *testing.T) { +func TestBackupViewsIntegration(t *testing.T) { const ( testBucket = "backuptest-views" testKeyspace = "backuptest_views" @@ -2547,7 +2547,7 @@ func TestBackupViews(t *testing.T) { } } -func TestBackupSkipSchema(t *testing.T) { +func TestBackupSkipSchemaIntegration(t *testing.T) { const ( testBucket = "backuptest-skip-schema" testKeyspace = "backuptest_skip_schema" diff --git a/pkg/service/backup/service_deduplicate_integration_test.go b/pkg/service/backup/service_deduplicate_integration_test.go index 80abc9e73..ed3ffedb9 100644 --- a/pkg/service/backup/service_deduplicate_integration_test.go +++ b/pkg/service/backup/service_deduplicate_integration_test.go @@ -19,7 +19,7 @@ import ( "github.com/scylladb/scylla-manager/v3/pkg/util/uuid" ) -func TestBackupPauseResumeOnDeduplicationStage(t *testing.T) { +func TestBackupPauseResumeOnDeduplicationStageIntegration(t *testing.T) { const ( testBucket = "backuptest-deduplication" testKeyspace = "backuptest_deduplication" @@ -160,5 +160,4 @@ func TestBackupPauseResumeOnDeduplicationStage(t *testing.T) { t.Fatalf("Expected uploaded 0 bytes on delta 0 backup, but was %v", totalUploaded) } }() - } From 47bbcf17f7a37d72967716b528bd40e448a39c8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Leszczy=C5=84ski?= <2000michal@wp.pl> Date: Tue, 17 Dec 2024 15:35:09 +0100 Subject: [PATCH 2/3] refactor(testutils): export CheckAnyConstraint It is also useful for backup svc tests. --- .../restore/helper_integration_test.go | 18 ----------- .../restore/restore_integration_test.go | 6 ++-- .../service_restore_integration_test.go | 14 ++++---- pkg/testutils/version.go | 32 +++++++++++++++++++ 4 files changed, 42 insertions(+), 28 deletions(-) create mode 100644 pkg/testutils/version.go diff --git a/pkg/service/restore/helper_integration_test.go b/pkg/service/restore/helper_integration_test.go index 648e86933..6a032eb5b 100644 --- a/pkg/service/restore/helper_integration_test.go +++ b/pkg/service/restore/helper_integration_test.go @@ -20,7 +20,6 @@ import ( "github.com/scylladb/gocqlx/v2" "github.com/scylladb/gocqlx/v2/qb" "github.com/scylladb/scylla-manager/v3/pkg/service/cluster" - "github.com/scylladb/scylla-manager/v3/pkg/util/version" "go.uber.org/zap/zapcore" "github.com/scylladb/scylla-manager/v3/pkg/metrics" @@ -465,23 +464,6 @@ func validateCompleteProgress(t *testing.T, pr Progress, tables []table) { } } -func checkAnyConstraint(t *testing.T, client *scyllaclient.Client, constraints ...string) bool { - ni, err := client.AnyNodeInfo(context.Background()) - if err != nil { - t.Fatal(err) - } - for _, c := range constraints { - ok, err := version.CheckConstraint(ni.ScyllaVersion, c) - if err != nil { - t.Fatal(err) - } - if ok { - return true - } - } - return false -} - func createTable(t *testing.T, session gocqlx.Session, keyspace string, tables ...string) { for _, tab := range tables { ExecStmt(t, session, fmt.Sprintf("CREATE TABLE %q.%q (id int PRIMARY KEY, data int)", keyspace, tab)) diff --git a/pkg/service/restore/restore_integration_test.go b/pkg/service/restore/restore_integration_test.go index f0d37eaa4..90e584c4b 100644 --- a/pkg/service/restore/restore_integration_test.go +++ b/pkg/service/restore/restore_integration_test.go @@ -36,7 +36,7 @@ import ( func TestRestoreTablesUserIntegration(t *testing.T) { h := newTestHelper(t, ManagedSecondClusterHosts(), ManagedClusterHosts()) - if checkAnyConstraint(t, h.dstCluster.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if CheckAnyConstraint(t, h.dstCluster.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { t.Skip("Auth restore is not supported in Scylla 6.0. It requires core side support that is aimed at 6.1 release") } @@ -123,7 +123,7 @@ func TestRestoreSchemaRoundtripIntegration(t *testing.T) { h := newTestHelper(t, ManagedSecondClusterHosts(), ManagedClusterHosts()) hRev := newTestHelper(t, ManagedClusterHosts(), ManagedSecondClusterHosts()) - if !checkAnyConstraint(t, h.dstCluster.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if !CheckAnyConstraint(t, h.dstCluster.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { t.Skip("This test assumes that schema is backed up and restored via DESCRIBE SCHEMA WITH INTERNALS") } @@ -237,7 +237,7 @@ func TestRestoreSchemaRoundtripIntegration(t *testing.T) { func TestRestoreSchemaDropAddColumnIntegration(t *testing.T) { h := newTestHelper(t, ManagedSecondClusterHosts(), ManagedClusterHosts()) - if !checkAnyConstraint(t, h.dstCluster.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if !CheckAnyConstraint(t, h.dstCluster.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { t.Skip("This test is the reason why SM needs to restore schema by DESCRIBE SCHEMA WITH INTERNALS") } diff --git a/pkg/service/restore/service_restore_integration_test.go b/pkg/service/restore/service_restore_integration_test.go index 6b3b6b5c4..5aeaebe3f 100644 --- a/pkg/service/restore/service_restore_integration_test.go +++ b/pkg/service/restore/service_restore_integration_test.go @@ -308,7 +308,7 @@ func TestRestoreGetTargetUnitsViewsIntegration(t *testing.T) { } var ignoreTarget []string - if checkAnyConstraint(t, h.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if CheckAnyConstraint(t, h.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { ignoreTarget = []string{ "!system_auth.*", "!system_distributed.service_levels", @@ -325,10 +325,10 @@ func TestRestoreGetTargetUnitsViewsIntegration(t *testing.T) { } var ignoreUnits []string - if checkAnyConstraint(t, h.Client, "< 1000") { + if CheckAnyConstraint(t, h.Client, "< 1000") { ignoreUnits = append(ignoreUnits, "system_replicated_keys") } - if checkAnyConstraint(t, h.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if CheckAnyConstraint(t, h.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { ignoreUnits = append(ignoreUnits, "system_auth", "service_levels", @@ -1407,7 +1407,7 @@ func restoreAllTables(t *testing.T, schemaTarget, tablesTarget Target, keyspace {ks: "system_traces", tab: "sessions"}, {ks: "system_traces", tab: "sessions_time_idx"}, } - if !checkAnyConstraint(t, dstH.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if !CheckAnyConstraint(t, dstH.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { toValidate = append(toValidate, table{ks: "system_auth", tab: "role_attributes"}, table{ks: "system_auth", tab: "role_members"}, @@ -1480,7 +1480,7 @@ func restoreAlternator(t *testing.T, schemaTarget, tablesTarget Target, testKeys ) dstH.shouldSkipTest(schemaTarget, tablesTarget) - if checkAnyConstraint(t, dstH.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if CheckAnyConstraint(t, dstH.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { t.Skip("See https://github.com/scylladb/scylladb/issues/19112") } @@ -1531,7 +1531,7 @@ func (h *restoreTestHelper) validateRestoreSuccess(dstSession, srcSession gocqlx Print("Then: validate restore result") if target.RestoreSchema { - if !checkAnyConstraint(h.T, h.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if !CheckAnyConstraint(h.T, h.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { // Schema restart is required only for older Scylla versions h.restartScylla() } @@ -1828,7 +1828,7 @@ func getBucketKeyspaceUser(t *testing.T) (string, string, string) { func (h *restoreTestHelper) shouldSkipTest(targets ...Target) { for _, target := range targets { if target.RestoreSchema { - if err := IsRestoreSchemaFromSSTablesSupported(context.Background(), h.Client); err != nil && !checkAnyConstraint(h.T, h.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { + if err := IsRestoreSchemaFromSSTablesSupported(context.Background(), h.Client); err != nil && !CheckAnyConstraint(h.T, h.Client, ">= 6.0, < 2000", ">= 2024.2, > 1000") { h.T.Skip(err) } } diff --git a/pkg/testutils/version.go b/pkg/testutils/version.go new file mode 100644 index 000000000..d9d0843c7 --- /dev/null +++ b/pkg/testutils/version.go @@ -0,0 +1,32 @@ +// Copyright (C) 2024 ScyllaDB + +package testutils + +import ( + "context" + "testing" + + "github.com/scylladb/scylla-manager/v3/pkg/scyllaclient" + "github.com/scylladb/scylla-manager/v3/pkg/util/version" +) + +// CheckAnyConstraint checks if any of the passed version constraints are satisfied. +// Can be used for skipping tests which make sense only for certain Scylla versions. +func CheckAnyConstraint(t *testing.T, client *scyllaclient.Client, constraints ...string) bool { + t.Helper() + + ni, err := client.AnyNodeInfo(context.Background()) + if err != nil { + t.Fatal(err) + } + for _, c := range constraints { + ok, err := version.CheckConstraint(ni.ScyllaVersion, c) + if err != nil { + t.Fatal(err) + } + if ok { + return true + } + } + return false +} From 9743cbb6d4e1d7403bf1a0d04e3a7d3c730a29b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Leszczy=C5=84ski?= <2000michal@wp.pl> Date: Tue, 17 Dec 2024 15:39:49 +0100 Subject: [PATCH 3/3] fix(backup_test): skip TestBackupSkipSchemaIntegration for older Scylla versions --- pkg/service/backup/service_backup_integration_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/service/backup/service_backup_integration_test.go b/pkg/service/backup/service_backup_integration_test.go index ed0b8f4c2..8cd5d4485 100644 --- a/pkg/service/backup/service_backup_integration_test.go +++ b/pkg/service/backup/service_backup_integration_test.go @@ -2562,6 +2562,11 @@ func TestBackupSkipSchemaIntegration(t *testing.T) { clusterSession = CreateSessionAndDropAllKeyspaces(t, h.Client) ) + if CheckAnyConstraint(h.T, h.Client, "< 6.0", "< 2024.2, > 1000") { + t.Skip("CQL credentials are not needed for the backup with this Scylla version, " + + "so the --skip-schema flag is not needed there") + } + Print("And: simple table to back up") WriteData(t, clusterSession, testKeyspace, 1)