diff options
author | serg-belyakov <serg-belyakov@yandex-team.com> | 2023-08-22 15:03:44 +0300 |
---|---|---|
committer | serg-belyakov <serg-belyakov@yandex-team.com> | 2023-08-22 15:37:44 +0300 |
commit | 037360c096adbb72d4a50fb5de0db80368da03be (patch) | |
tree | a9cf0196e866746d0c08edd2108f4468757572ad | |
parent | 39b001b8899b48037e0d669b1bf6dad6c4ad8e48 (diff) | |
download | ydb-037360c096adbb72d4a50fb5de0db80368da03be.tar.gz |
Always use common +-1 major rule, KIKIMR-15989
Always use common compatibility rule
-rw-r--r-- | ydb/core/driver_lib/version/ut/version_ut.cpp | 39 | ||||
-rw-r--r-- | ydb/core/driver_lib/version/version.cpp | 123 |
2 files changed, 59 insertions, 103 deletions
diff --git a/ydb/core/driver_lib/version/ut/version_ut.cpp b/ydb/core/driver_lib/version/ut/version_ut.cpp index a6a79078f74..8dbeecbf569 100644 --- a/ydb/core/driver_lib/version/ut/version_ut.cpp +++ b/ydb/core/driver_lib/version/ut/version_ut.cpp @@ -14,6 +14,7 @@ Y_UNIT_TEST_SUITE(VersionParser) { } } +using TComponentId = NKikimrConfig::TCompatibilityRule::EComponentId; using EComponentId = NKikimrConfig::TCompatibilityRule; using TOldFormat = NActors::TInterconnectProxyCommon::TVersionInfo; using TVersion = TCompatibilityInfo::TProtoConstructor::TVersion; @@ -23,13 +24,14 @@ using TStoredCompatibilityInfo = TCompatibilityInfo::TProtoConstructor::TStoredC Y_UNIT_TEST_SUITE(YdbVersion) { - void Test(TCurrentCompatibilityInfo current, TCurrentCompatibilityInfo store, bool expected) { + void Test(TCurrentCompatibilityInfo current, TCurrentCompatibilityInfo store, bool expected, + TComponentId componentId = EComponentId::Test1) { TString errorReason; auto currentPB = current.ToPB(); auto storePB = store.ToPB(); - auto storedPB = CompatibilityInfo.MakeStored(EComponentId::Test1, &storePB); + auto storedPB = CompatibilityInfo.MakeStored(componentId, &storePB); UNIT_ASSERT_EQUAL_C(CompatibilityInfo.CheckCompatibility(¤tPB, &storedPB, - EComponentId::Test1, errorReason), expected, errorReason); + componentId, errorReason), expected, errorReason); } Y_UNIT_TEST(DefaultSameVersion) { @@ -609,6 +611,7 @@ Y_UNIT_TEST_SUITE(YdbVersion) { .Application = "nbs", .LowerLimit = TVersion{ .Year = 23, .Major = 3 }, .UpperLimit = TVersion{ .Year = 24, .Major = 2 }, + .ComponentId = EComponentId::Interconnect, }, }, .StoresReadableBy = { @@ -616,10 +619,12 @@ Y_UNIT_TEST_SUITE(YdbVersion) { .Application = "nbs", .LowerLimit = TVersion{ .Year = 23, .Major = 3 }, .UpperLimit = TVersion{ .Year = 24, .Major = 2 }, + .ComponentId = EComponentId::Interconnect, }, } }, - true + true, + EComponentId::Interconnect ); } @@ -665,32 +670,6 @@ Y_UNIT_TEST_SUITE(YdbVersion) { ); } - Y_UNIT_TEST(RestrictedCompatibilitySameApplication) { - Test( - TCurrentCompatibilityInfo{ - .Application = "ydb", - .Version = TVersion{ .Year = 24, .Major = 3, .Minor = 1, .Hotfix = 0 }, - .CanLoadFrom = { - TCompatibilityRule{ - .LowerLimit = TVersion{ .Year = 24, .Major = 2, .Minor = 4}, - .UpperLimit = TVersion{ .Year = 24, .Major = 3 }, - }, - }, - .StoresReadableBy = { - TCompatibilityRule{ - .LowerLimit = TVersion{ .Year = 24, .Major = 2, .Minor = 4}, - .UpperLimit = TVersion{ .Year = 24, .Major = 3 }, - }, - } - }, - TCurrentCompatibilityInfo{ - .Application = "ydb", - .Version = TVersion{ .Year = 24, .Major = 2, .Minor = 1, .Hotfix = 0 }, - }, - false - ); - } - Y_UNIT_TEST(CompatibleWithSelf) { auto stored = CompatibilityInfo.MakeStored(EComponentId::Test1); TString errorReason; diff --git a/ydb/core/driver_lib/version/version.cpp b/ydb/core/driver_lib/version/version.cpp index 439e6c96a08..31d39326ff0 100644 --- a/ydb/core/driver_lib/version/version.cpp +++ b/ydb/core/driver_lib/version/version.cpp @@ -95,12 +95,12 @@ TString PrintStoredAndCurrent(const TStored* stored, const TCurrent* current) { "Current CompatibilityInfo# { " << currentStr << " } "; } -TString PrintStoredAndCurrent(const TOldFormat& stored, const TCurrent* current) { +TString PrintStoredAndCurrent(const TOldFormat& peer, const TCurrent* current) { TStringStream str; - str << "Stored CompatibilityInfo# { "; - str << "Tag# " << stored.Tag; + str << "Peer CompatibilityInfo# { "; + str << "Tag# " << peer.Tag; str << "AcceptedTag# { "; - for (const TString& tag : stored.AcceptedTags) { + for (const TString& tag : peer.AcceptedTags) { str << tag << " "; } str << " } } "; @@ -119,8 +119,7 @@ TStored TCompatibilityInfo::MakeStored(TComponentId componentId, const TCurrent* stored.MutableVersion()->CopyFrom(current->GetVersion()); } - for (ui32 i = 0; i < current->StoresReadableBySize(); i++) { - auto rule = current->GetStoresReadableBy(i); + for (const auto& rule : current->GetStoresReadableBy()) { const auto ruleComponentId = TComponentId(rule.GetComponentId()); if (!rule.HasComponentId() || ruleComponentId == componentId || ruleComponentId == EComponentId::Any) { auto *newRule = stored.AddReadableBy(); @@ -132,7 +131,6 @@ TStored TCompatibilityInfo::MakeStored(TComponentId componentId, const TCurrent* newRule->SetForbidden(rule.GetForbidden()); } } - return stored; } @@ -279,18 +277,12 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TStor const auto* storedVersion = stored->HasVersion() ? &stored->GetVersion() : nullptr; bool permitted = false; - bool useDefault = true; - for (ui32 i = 0; i < current->CanLoadFromSize(); ++i) { - const auto rule = current->GetCanLoadFrom(i); + for (const auto& rule : current->GetCanLoadFrom()) { const auto ruleComponentId = TComponentId(rule.GetComponentId()); if (!rule.HasComponentId() || ruleComponentId == componentId || ruleComponentId == EComponentId::Any) { - bool isForbidding = rule.HasForbidden() && rule.GetForbidden(); - if ((!rule.HasApplication() || rule.GetApplication() == storedApplication) && !isForbidding) { - useDefault = false; - } if (CheckRule(storedApplication, storedVersion, rule)) { - if (isForbidding) { + if (rule.HasForbidden() && rule.GetForbidden()) { errorReason = "Stored version is explicitly prohibited, " + PrintStoredAndCurrent(stored, current); return false; } else { @@ -300,12 +292,10 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TStor } } - for (ui32 i = 0; i < stored->ReadableBySize(); ++i) { - const auto rule = stored->GetReadableBy(i); + for (const auto& rule : stored->GetReadableBy()) { const auto ruleComponentId = TComponentId(rule.GetComponentId()); if (!rule.HasComponentId() || ruleComponentId == componentId || ruleComponentId == EComponentId::Any) { if (CheckRule(currentApplication, currentVersion, rule)) { - useDefault = false; if (rule.HasForbidden() && rule.GetForbidden()) { errorReason = "Current version is explicitly prohibited, " + PrintStoredAndCurrent(stored, current); return false; @@ -319,16 +309,13 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TStor if (permitted) { return true; } else { - if (useDefault) { - if (CheckDefaultRules(currentApplication, currentVersion, storedApplication, storedVersion)) { - return true; - } else { - errorReason = "Versions are not compatible by default rules, " + PrintStoredAndCurrent(stored, current); - return false; - } + if (CheckDefaultRules(currentApplication, currentVersion, storedApplication, storedVersion)) { + return true; + } else { + errorReason = "Versions are not compatible neither by common rule nor by provided rule sets, " + + PrintStoredAndCurrent(stored, current); + return false; } - errorReason = "Versions are not compatible by given rule sets, " + PrintStoredAndCurrent(stored, current); - return false; } } @@ -552,6 +539,7 @@ void CheckVersionTag() { } bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TOldFormat& stored, TComponentId componentId, TString& errorReason) const { + // stored version is peer version in terms of Interconnect Y_VERIFY(current); std::optional<TString> storedApplication; @@ -566,18 +554,12 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TOldF } bool permitted = false; - bool useDefault = true; - for (ui32 i = 0; i < current->CanLoadFromSize(); ++i) { - const auto rule = current->GetCanLoadFrom(i); + for (const auto& rule : current->GetCanLoadFrom()) { const auto ruleComponentId = TComponentId(rule.GetComponentId()); if (!rule.HasComponentId() || ruleComponentId == componentId || ruleComponentId == EComponentId::Any) { - bool isForbidding = rule.HasForbidden() && rule.GetForbidden(); - if (!rule.HasApplication() && !isForbidding) { - useDefault = false; - } if (CheckRule(storedApplication, &*storedVersion, rule)) { - if (isForbidding) { + if (rule.HasForbidden() && rule.GetForbidden()) { errorReason = "Stored version is explicitly prohibited, " + PrintStoredAndCurrent(stored, current); return false; } else { @@ -591,60 +573,55 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TOldF return true; } - const auto* currentVersion = current->HasVersion() ? ¤t->GetVersion() : nullptr; for (const auto& tag : stored.AcceptedTags) { auto version = ParseVersionFromTag(tag); - if (storedVersion && currentVersion) { - if (version->GetYear() == currentVersion->GetYear() && - version->GetMajor() == currentVersion->GetMajor() && - version->GetMinor() == currentVersion->GetMinor() && - version->GetHotfix() == currentVersion->GetHotfix()) { + if (version && current->HasVersion()) { + if (CompareVersions(*version, current->GetVersion()) == 0) { return true; } - } else if (!storedVersion && !currentVersion) { + } else if (!version && !current->HasVersion()) { if (current->GetApplication() == tag) { return true; } } } - if (useDefault) { - if (current->HasVersion() && storedVersion) { - auto currentVersion = current->GetVersion(); - if (!currentVersion.HasYear() || !storedVersion->HasYear()) { - return true; - } - if (currentVersion.GetYear() != storedVersion->GetYear()) { - errorReason = "Default rules used, stored's and current's Year differ, " - + PrintStoredAndCurrent(stored, current); - return false; - } - if (!currentVersion.HasMajor() || !storedVersion->HasMajor()) { - return true; - } - if (std::abs((i32)currentVersion.GetMajor() - (i32)storedVersion->GetMajor()) <= 1) { - return true; - } else { - errorReason = "Default rules used, stored's and current's Major difference is more than 1, " - + PrintStoredAndCurrent(stored, current); - return false; - } - } else if (!current->HasVersion() && !storedVersion) { - if (*storedApplication == current->GetApplication()) { - return true; - } else { - errorReason = "Default rules used, both versions are non-stable, stored's and current's Application differ, " - + PrintStoredAndCurrent(stored, current); - return false; - } + // use common rule + if (current->HasVersion() && storedVersion) { + const auto& currentVersion = current->GetVersion(); + if (!currentVersion.HasYear() || !storedVersion->HasYear()) { + return true; + } + if (currentVersion.GetYear() != storedVersion->GetYear()) { + errorReason = "Incompatible by common rule: peer's and current's Year differ, " + + PrintStoredAndCurrent(stored, current); + return false; + } + if (!currentVersion.HasMajor() || !storedVersion->HasMajor()) { + return true; + } + if (std::abs((i32)currentVersion.GetMajor() - (i32)storedVersion->GetMajor()) <= 1) { + return true; } else { - errorReason = "Default rules used, stable and non-stable versions are incompatible, " + errorReason = "Incompatible by common rule: peer's and current's Major differ by more than 1, " + PrintStoredAndCurrent(stored, current); return false; } + } else if (!current->HasVersion() && !storedVersion) { + if (*storedApplication == current->GetApplication()) { + return true; + } else { + errorReason = "Incompatible by common rule: both versions are non-stable, peer's and current's Build differ, " + + PrintStoredAndCurrent(stored, current); + return false; + } + } else { + errorReason = "Incompatible by common rule: one tag is stable and other is non-stable, " + + PrintStoredAndCurrent(stored, current); + return false; } - errorReason = "Version tag doesn't match any current compatibility rule, current version is not in accepted tags list, " + errorReason = "Peer version tag doesn't match any current compatibility rule, current version is not in accepted tags list, " + PrintStoredAndCurrent(stored, current); return false; } |