diff --git a/cluster-autoscaler/simulator/common/patch.go b/cluster-autoscaler/simulator/common/patch.go new file mode 100644 index 0000000000000000000000000000000000000000..dfd3923264a63101c7f9ebdc8c7eda7e7ed6953c --- /dev/null +++ b/cluster-autoscaler/simulator/common/patch.go @@ -0,0 +1,87 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package common + +// Patch represents a single layer of modifications (additions/updates) +// and deletions for a set of key-value pairs. It's used as a building +// block for the patchSet. +type Patch[K comparable, V any] struct { + modified map[K]V + deleted map[K]bool +} + +// Set marks a key-value pair as modified in the current patch. +// If the key was previously marked as deleted, the deletion mark is removed. +func (p *Patch[K, V]) Set(key K, value V) { + p.modified[key] = value + delete(p.deleted, key) +} + +// Delete marks a key as deleted in the current patch. +// If the key was previously marked as modified, the modification is removed. +func (p *Patch[K, V]) Delete(key K) { + delete(p.modified, key) + p.deleted[key] = true +} + +// Get retrieves the modified value for a key within this specific patch. +// It returns the value and true if the key was found in this patch +// or zero value and false otherwise. +func (p *Patch[K, V]) Get(key K) (value V, found bool) { + value, found = p.modified[key] + return value, found +} + +// IsDeleted checks if the key is marked as deleted within this specific patch. +func (p *Patch[K, V]) IsDeleted(key K) bool { + return p.deleted[key] +} + +// NewPatch creates a new, empty patch with no modifications or deletions. +func NewPatch[K comparable, V any]() *Patch[K, V] { + return &Patch[K, V]{ + modified: make(map[K]V), + deleted: make(map[K]bool), + } +} + +// NewPatchFromMap creates a new patch initialized with the data from the provided map +// the data supplied is recorded as modified in the patch. +func NewPatchFromMap[M ~map[K]V, K comparable, V any](source M) *Patch[K, V] { + if source == nil { + source = make(M) + } + + return &Patch[K, V]{ + modified: source, + deleted: make(map[K]bool), + } +} + +// mergePatchesInPlace merges two patches into one, while modifying patch a +// inplace taking records in b as a priority when overwrites are required +func mergePatchesInPlace[K comparable, V any](a *Patch[K, V], b *Patch[K, V]) *Patch[K, V] { + for key, value := range b.modified { + a.Set(key, value) + } + + for key := range b.deleted { + a.Delete(key) + } + + return a +} diff --git a/cluster-autoscaler/simulator/common/patch_test.go b/cluster-autoscaler/simulator/common/patch_test.go new file mode 100644 index 0000000000000000000000000000000000000000..30a6d778a5c8102423b77c2bfd9aeb88d7de8bd8 --- /dev/null +++ b/cluster-autoscaler/simulator/common/patch_test.go @@ -0,0 +1,324 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package common + +import ( + "maps" + "testing" +) + +func TestPatchOperations(t *testing.T) { + p := NewPatch[string, int]() + + // 1. Get and IsDeleted on non-existent key + if _, found := p.Get("a"); found { + t.Errorf("Get for 'a' should not find anything") + } + if p.IsDeleted("a") { + t.Errorf("IsDeleted for 'a' should be false") + } + + // 2. Set 'a' key + p.Set("a", 1) + if val, found := p.Get("a"); !found || val != 1 { + t.Errorf("Get('a') = %v,%v, expected 1,true", val, found) + } + if p.IsDeleted("a") { + t.Errorf("IsDeleted('a') = true, should be false") + } + + // 3. Overwrite 'a' key + p.Set("a", 2) + if val, found := p.Get("a"); !found || val != 2 { + t.Errorf("Get('a') = %v,%v, expected 2,true", val, found) + } + if p.IsDeleted("a") { + t.Errorf("IsDeleted('a') = true, should be false") + } + + // 4. Delete 'a' key + p.Delete("a") + if val, found := p.Get("a"); found { + t.Errorf("Get('a') = %v,%v, should not find anything after delete", val, found) + } + if !p.IsDeleted("a") { + t.Errorf("IsDeleted('a') = false, should be true") + } + + // 5. Set 'a' key again after deletion + p.Set("a", 3) + if val, found := p.Get("a"); !found || val != 3 { + t.Errorf("Get('a') = %v,%v, expected 3,true", val, found) + } + if p.IsDeleted("a") { + t.Errorf("IsDeleted('a') = true, should be false") + } + + // 6. Delete a non-existent key 'c' + p.Delete("c") + if val, found := p.Get("c"); found { + t.Errorf("Get('c') = %v, %v, should not find anything", val, found) + } + if !p.IsDeleted("c") { + t.Errorf("IsDeleted('c') = false, should be true") + } +} + +func TestCreatePatch(t *testing.T) { + tests := map[string]struct { + sourceMap map[string]int + addKeys map[string]int + deleteKeys []string + wantModified map[string]int + wantDeleted map[string]bool + }{ + "SourceMapOnlyNoModifications": { + sourceMap: map[string]int{"k1": 1, "k2": 2}, + addKeys: map[string]int{}, + deleteKeys: []string{}, + wantModified: map[string]int{"k1": 1, "k2": 2}, + wantDeleted: map[string]bool{}, + }, + "NilSourceMapAddAndDelete": { + sourceMap: nil, + addKeys: map[string]int{"a": 1, "b": 2}, + deleteKeys: []string{"b"}, + wantModified: map[string]int{"a": 1}, + wantDeleted: map[string]bool{"b": true}, + }, + "EmptySourceMapAddAndDelete": { + sourceMap: map[string]int{}, + addKeys: map[string]int{"x": 10}, + deleteKeys: []string{"y"}, + wantModified: map[string]int{"x": 10}, + wantDeleted: map[string]bool{"y": true}, + }, + "NonEmptySourceMapAddOverwriteDelete": { + sourceMap: map[string]int{"orig1": 100, "orig2": 200}, + addKeys: map[string]int{"new1": 300, "orig1": 101}, + deleteKeys: []string{"orig2", "new1"}, + wantModified: map[string]int{"orig1": 101}, + wantDeleted: map[string]bool{"orig2": true, "new1": true}, + }, + "DeleteKeyFromSourceMap": { + sourceMap: map[string]int{"key_to_delete": 70, "key_to_keep": 80}, + addKeys: map[string]int{}, + deleteKeys: []string{"key_to_delete"}, + wantModified: map[string]int{"key_to_keep": 80}, + wantDeleted: map[string]bool{"key_to_delete": true}, + }, + "AddOnlyNoSourceMap": { + sourceMap: nil, + addKeys: map[string]int{"add1": 10, "add2": 20}, + deleteKeys: []string{}, + wantModified: map[string]int{"add1": 10, "add2": 20}, + wantDeleted: map[string]bool{}, + }, + "DeleteOnlyNoSourceMap": { + sourceMap: nil, + addKeys: map[string]int{}, + deleteKeys: []string{"del1", "del2"}, + wantModified: map[string]int{}, + wantDeleted: map[string]bool{"del1": true, "del2": true}, + }, + "DeleteKeyNotPresentInSourceOrAdded": { + sourceMap: map[string]int{"a": 1}, + addKeys: map[string]int{"b": 2}, + deleteKeys: []string{"c"}, + wantModified: map[string]int{"a": 1, "b": 2}, + wantDeleted: map[string]bool{"c": true}, + }, + "AddKeyThenDeleteIt": { + sourceMap: map[string]int{"base": 100}, + addKeys: map[string]int{"temp": 50}, + deleteKeys: []string{"temp"}, + wantModified: map[string]int{"base": 100}, + wantDeleted: map[string]bool{"temp": true}, + }, + "AllOperations": { + sourceMap: map[string]int{"s1": 1, "s2": 2, "s3": 3}, + addKeys: map[string]int{"a1": 10, "s2": 22, "a2": 20}, + deleteKeys: []string{"s1", "a1", "nonexistent"}, + wantModified: map[string]int{"s2": 22, "s3": 3, "a2": 20}, + wantDeleted: map[string]bool{"s1": true, "a1": true, "nonexistent": true}, + }, + "NoOperationsEmptySource": { + sourceMap: map[string]int{}, + addKeys: map[string]int{}, + deleteKeys: []string{}, + wantModified: map[string]int{}, + wantDeleted: map[string]bool{}, + }, + } + + for testName, test := range tests { + t.Run(testName, func(t *testing.T) { + p := NewPatchFromMap(test.sourceMap) + + for k, v := range test.addKeys { + p.Set(k, v) + } + for _, k := range test.deleteKeys { + p.Delete(k) + } + + if !maps.Equal(p.modified, test.wantModified) { + t.Errorf("Modified map mismatch: got %v, want %v", p.modified, test.wantModified) + } + + if !maps.Equal(p.deleted, test.wantDeleted) { + t.Errorf("Deleted map mismatch: got %v, want %v", p.deleted, test.wantDeleted) + } + }) + } +} + +func TestMergePatchesInPlace(t *testing.T) { + tests := map[string]struct { + modifiedPatchA map[string]int + deletedPatchA map[string]bool + modifiedPatchB map[string]int + deletedPatchB map[string]bool + wantModifiedMerged map[string]int + wantDeletedMerged map[string]bool + }{ + "PatchBOverwritesValueInPatchA": { + modifiedPatchA: map[string]int{"a": 1, "b": 2}, + deletedPatchA: map[string]bool{}, + modifiedPatchB: map[string]int{"b": 22, "c": 3}, + deletedPatchB: map[string]bool{}, + wantModifiedMerged: map[string]int{"a": 1, "b": 22, "c": 3}, + wantDeletedMerged: map[string]bool{}, + }, + "PatchBDeletesValuePresentInPatchA": { + modifiedPatchA: map[string]int{"a": 1, "b": 2}, + deletedPatchA: map[string]bool{}, + modifiedPatchB: map[string]int{}, + deletedPatchB: map[string]bool{"a": true}, + wantModifiedMerged: map[string]int{"b": 2}, + wantDeletedMerged: map[string]bool{"a": true}, + }, + "PatchBDeletesValueAlreadyDeletedInPatchA": { + modifiedPatchA: map[string]int{}, + deletedPatchA: map[string]bool{"x": true}, + modifiedPatchB: map[string]int{}, + deletedPatchB: map[string]bool{"x": true, "y": true}, + wantModifiedMerged: map[string]int{}, + wantDeletedMerged: map[string]bool{"x": true, "y": true}, + }, + "PatchBAddsValuePreviouslyDeletedInPatchA": { + modifiedPatchA: map[string]int{}, + deletedPatchA: map[string]bool{"x": true}, + modifiedPatchB: map[string]int{"x": 10}, + deletedPatchB: map[string]bool{}, + wantModifiedMerged: map[string]int{"x": 10}, + wantDeletedMerged: map[string]bool{}, + }, + "MergeEmptyPatchBIntoNonEmptyPatchA": { + modifiedPatchA: map[string]int{"a": 1}, + deletedPatchA: map[string]bool{"b": true}, + modifiedPatchB: map[string]int{}, + deletedPatchB: map[string]bool{}, + wantModifiedMerged: map[string]int{"a": 1}, + wantDeletedMerged: map[string]bool{"b": true}, + }, + "MergeNonEmptyPatchBIntoEmptyPatchA": { + modifiedPatchA: map[string]int{}, + deletedPatchA: map[string]bool{}, + modifiedPatchB: map[string]int{"a": 1}, + deletedPatchB: map[string]bool{"b": true}, + wantModifiedMerged: map[string]int{"a": 1}, + wantDeletedMerged: map[string]bool{"b": true}, + }, + "MergeTwoEmptyPatches": { + modifiedPatchA: map[string]int{}, + deletedPatchA: map[string]bool{}, + modifiedPatchB: map[string]int{}, + deletedPatchB: map[string]bool{}, + wantModifiedMerged: map[string]int{}, + wantDeletedMerged: map[string]bool{}, + }, + "NoOverlapBetweenPatchAAndPatchBModifications": { + modifiedPatchA: map[string]int{"a1": 1, "a2": 2}, + deletedPatchA: map[string]bool{}, + modifiedPatchB: map[string]int{"b1": 10, "b2": 20}, + deletedPatchB: map[string]bool{}, + wantModifiedMerged: map[string]int{"a1": 1, "a2": 2, "b1": 10, "b2": 20}, + wantDeletedMerged: map[string]bool{}, + }, + "NoOverlapBetweenPatchAAndPatchBDeletions": { + modifiedPatchA: map[string]int{}, + deletedPatchA: map[string]bool{"a1": true, "a2": true}, + modifiedPatchB: map[string]int{}, + deletedPatchB: map[string]bool{"b1": true, "b2": true}, + wantModifiedMerged: map[string]int{}, + wantDeletedMerged: map[string]bool{"a1": true, "a2": true, "b1": true, "b2": true}, + }, + "PatchBOnlyAddsNewKeysPatchAUnchanged": { + modifiedPatchA: map[string]int{"orig": 5}, + modifiedPatchB: map[string]int{"new1": 100, "new2": 200}, + deletedPatchA: map[string]bool{}, + deletedPatchB: map[string]bool{}, + wantModifiedMerged: map[string]int{"orig": 5, "new1": 100, "new2": 200}, + wantDeletedMerged: map[string]bool{}, + }, + "PatchBOnlyDeletesNewKeysPatchAUnchanged": { + modifiedPatchA: map[string]int{"orig": 5}, + deletedPatchA: map[string]bool{}, + modifiedPatchB: map[string]int{}, + deletedPatchB: map[string]bool{"del1": true, "del2": true}, + wantModifiedMerged: map[string]int{"orig": 5}, + wantDeletedMerged: map[string]bool{"del1": true, "del2": true}, + }, + "AllInOne": { + modifiedPatchA: map[string]int{"k1": 1, "k2": 2, "k3": 3}, + deletedPatchA: map[string]bool{"d1": true, "d2": true}, + modifiedPatchB: map[string]int{"k2": 22, "k4": 4, "d1": 11}, + deletedPatchB: map[string]bool{"k3": true, "d2": true, "d3": true}, + wantModifiedMerged: map[string]int{"k1": 1, "k2": 22, "k4": 4, "d1": 11}, + wantDeletedMerged: map[string]bool{"k3": true, "d2": true, "d3": true}, + }, + } + + for testName, test := range tests { + t.Run(testName, func(t *testing.T) { + patchA := NewPatchFromMap(test.modifiedPatchA) + for k := range test.deletedPatchA { + patchA.Delete(k) + } + + patchB := NewPatchFromMap(test.modifiedPatchB) + for k := range test.deletedPatchB { + patchB.Delete(k) + } + + merged := mergePatchesInPlace(patchA, patchB) + + if merged != patchA { + t.Errorf("mergePatchesInPlace did not modify patchA inplace, references are different") + } + + if !maps.Equal(merged.modified, test.wantModifiedMerged) { + t.Errorf("Modified map mismatch: got %v, want %v", merged.modified, test.wantModifiedMerged) + } + + if !maps.Equal(merged.deleted, test.wantDeletedMerged) { + t.Errorf("Deleted map mismatch: got %v, want %v", merged.deleted, test.wantDeletedMerged) + } + }) + } +} diff --git a/cluster-autoscaler/simulator/dynamicresources/snapshot/patchset.go b/cluster-autoscaler/simulator/common/patchset.go similarity index 64% rename from cluster-autoscaler/simulator/dynamicresources/snapshot/patchset.go rename to cluster-autoscaler/simulator/common/patchset.go index 255ad3892cd9ede0a8dc39a32c5fce8a048f338e..c30ad06e93eeec60e5504c4b83fd27fcd2dfc569 100644 --- a/cluster-autoscaler/simulator/dynamicresources/snapshot/patchset.go +++ b/cluster-autoscaler/simulator/common/patchset.go @@ -14,116 +14,46 @@ See the License for the specific language governing permissions and limitations under the License. */ -package snapshot - -// patch represents a single layer of modifications (additions/updates) -// and deletions for a set of key-value pairs. It's used as a building -// block for the patchSet. -type patch[K comparable, V any] struct { - Modified map[K]V - Deleted map[K]bool -} - -// Set marks a key-value pair as modified in the current patch. -// If the key was previously marked as deleted, the deletion mark is removed. -func (p *patch[K, V]) Set(key K, value V) { - p.Modified[key] = value - delete(p.Deleted, key) -} - -// Delete marks a key as deleted in the current patch. -// If the key was previously marked as modified, the modification is removed. -func (p *patch[K, V]) Delete(key K) { - delete(p.Modified, key) - p.Deleted[key] = true -} - -// Get retrieves the modified value for a key within this specific patch. -// It returns the value and true if the key was found in this patch -// or zero value and false otherwise. -func (p *patch[K, V]) Get(key K) (value V, found bool) { - value, found = p.Modified[key] - return value, found -} - -// IsDeleted checks if the key is marked as deleted within this specific patch. -func (p *patch[K, V]) IsDeleted(key K) bool { - return p.Deleted[key] -} - -// newPatch creates a new, empty patch with no modifications or deletions. -func newPatch[K comparable, V any]() *patch[K, V] { - return &patch[K, V]{ - Modified: make(map[K]V), - Deleted: make(map[K]bool), - } -} - -// newPatchFromMap creates a new patch initialized with the data from the provided map -// the data supplied is recorded as modified in the patch. -func newPatchFromMap[M ~map[K]V, K comparable, V any](source M) *patch[K, V] { - if source == nil { - source = make(M) - } - - return &patch[K, V]{ - Modified: source, - Deleted: make(map[K]bool), - } -} +package common -// mergePatchesInPlace merges two patches into one, while modifying patch a -// inplace taking records in b as a priority when overwrites are required -func mergePatchesInPlace[K comparable, V any](a *patch[K, V], b *patch[K, V]) *patch[K, V] { - for key, value := range b.Modified { - a.Set(key, value) - } - - for key := range b.Deleted { - a.Delete(key) - } - - return a -} - -// patchSet manages a stack of patches, allowing for fork/revert/commit operations. +// PatchSet manages a stack of patches, allowing for fork/revert/commit operations. // It provides a view of the data as if all patches were applied sequentially. // // Time Complexities: // - Fork(): O(1). // - Commit(): O(P), where P is the number of modified/deleted entries -// in the current patch or no-op for patchSet with a single patch. +// in the current patch or no-op for PatchSet with a single patch. // - Revert(): O(P), where P is the number of modified/deleted entries -// in the topmost patch or no-op for patchSet with a single patch. +// in the topmost patch or no-op for PatchSet with a single patch. // - FindValue(key): O(1) for cached keys, O(N * P) - otherwise. // - AsMap(): O(N * P) as it needs to iterate through all the layers // modifications and deletions to get the latest state. Best case - -// if the cache is currently in sync with the patchSet data complexity becomes -// O(C), where C is the actual number key/value pairs in flattened patchSet. +// if the cache is currently in sync with the PatchSet data complexity becomes +// O(C), where C is the actual number key/value pairs in flattened PatchSet. // - SetCurrent(key, value): O(1). // - DeleteCurrent(key): O(1). // - InCurrentPatch(key): O(1). // // Variables used in complexity analysis: -// - N: The number of patch layers in the patchSet. +// - N: The number of patch layers in the PatchSet. // - P: The number of modified/deleted entries in a single patch layer // // Caching: // -// The patchSet employs a lazy caching mechanism to speed up access to data. When a specific item is requested, +// The PatchSet employs a lazy caching mechanism to speed up access to data. When a specific item is requested, // the cache is checked first. If the item isn't cached, its effective value is computed by traversing the layers // of patches from the most recent to the oldest, and the result is then stored in the cache. For operations that // require the entire dataset like AsMap, if a cache is fully up-to-date, the data is served directly from the cache. // Otherwise, the entire dataset is rebuilt by applying all patches, and this process fully populates the cache. // Direct modifications to the current patch update the specific item in the cache immediately. Reverting the latest // patch will clear affected items from the cache and mark the cache as potentially out-of-sync, as underlying values may -// no longer represent the patchSet as a whole. Committing and forking does not invalidate the cache, as the effective +// no longer represent the PatchSet as a whole. Committing and forking does not invalidate the cache, as the effective // values remain consistent from the perspective of read operations. -type patchSet[K comparable, V any] struct { +type PatchSet[K comparable, V any] struct { // patches is a stack of individual modification layers. The base data is // at index 0, and subsequent modifications are layered on top. // PatchSet should always contain at least a single patch. - patches []*patch[K, V] + patches []*Patch[K, V] // cache stores the computed effective value for keys that have been accessed. // A nil pointer indicates the key is effectively deleted or not present. @@ -136,9 +66,9 @@ type patchSet[K comparable, V any] struct { cacheInSync bool } -// newPatchSet creates a new patchSet, initializing it with the provided base patches. -func newPatchSet[K comparable, V any](patches ...*patch[K, V]) *patchSet[K, V] { - return &patchSet[K, V]{ +// NewPatchSet creates a new PatchSet, initializing it with the provided base patches. +func NewPatchSet[K comparable, V any](patches ...*Patch[K, V]) *PatchSet[K, V] { + return &PatchSet[K, V]{ patches: patches, cache: make(map[K]*V), cacheInSync: false, @@ -147,13 +77,13 @@ func newPatchSet[K comparable, V any](patches ...*patch[K, V]) *patchSet[K, V] { // Fork adds a new, empty patch layer to the top of the stack. // Subsequent modifications will be recorded in this new layer. -func (p *patchSet[K, V]) Fork() { - p.patches = append(p.patches, newPatch[K, V]()) +func (p *PatchSet[K, V]) Fork() { + p.patches = append(p.patches, NewPatch[K, V]()) } // Commit merges the topmost patch layer into the one below it. // If there's only one layer (or none), it's a no-op. -func (p *patchSet[K, V]) Commit() { +func (p *PatchSet[K, V]) Commit() { if len(p.patches) < 2 { return } @@ -166,7 +96,7 @@ func (p *patchSet[K, V]) Commit() { // Revert removes the topmost patch layer. // Any modifications or deletions recorded in that layer are discarded. -func (p *patchSet[K, V]) Revert() { +func (p *PatchSet[K, V]) Revert() { if len(p.patches) <= 1 { return } @@ -174,11 +104,11 @@ func (p *patchSet[K, V]) Revert() { currentPatch := p.patches[len(p.patches)-1] p.patches = p.patches[:len(p.patches)-1] - for key := range currentPatch.Modified { + for key := range currentPatch.modified { delete(p.cache, key) } - for key := range currentPatch.Deleted { + for key := range currentPatch.deleted { delete(p.cache, key) } @@ -188,7 +118,7 @@ func (p *patchSet[K, V]) Revert() { // FindValue searches for the effective value of a key by looking through the patches // from top to bottom. It returns the value and true if found, or the zero value and false // if the key is deleted or not found in any patch. -func (p *patchSet[K, V]) FindValue(key K) (value V, found bool) { +func (p *PatchSet[K, V]) FindValue(key K) (value V, found bool) { var zero V if cachedValue, cacheHit := p.cache[key]; cacheHit { @@ -227,7 +157,7 @@ func (p *patchSet[K, V]) FindValue(key K) (value V, found bool) { // AsMap merges all patches into a single map representing the current effective state. // It iterates through all patches from bottom to top, applying modifications and deletions. // The cache is populated with the results during this process. -func (p *patchSet[K, V]) AsMap() map[K]V { +func (p *PatchSet[K, V]) AsMap() map[K]V { if p.cacheInSync { patchSetMap := make(map[K]V, len(p.cache)) for key, value := range p.cache { @@ -242,11 +172,11 @@ func (p *patchSet[K, V]) AsMap() map[K]V { patchSetMap := make(map[K]V, keysCount) for _, patch := range p.patches { - for key, value := range patch.Modified { + for key, value := range patch.modified { patchSetMap[key] = value } - for key := range patch.Deleted { + for key := range patch.deleted { delete(patchSetMap, key) } } @@ -260,7 +190,7 @@ func (p *patchSet[K, V]) AsMap() map[K]V { } // SetCurrent adds or updates a key-value pair in the topmost patch layer. -func (p *patchSet[K, V]) SetCurrent(key K, value V) { +func (p *PatchSet[K, V]) SetCurrent(key K, value V) { if len(p.patches) == 0 { p.Fork() } @@ -271,7 +201,7 @@ func (p *patchSet[K, V]) SetCurrent(key K, value V) { } // DeleteCurrent marks a key as deleted in the topmost patch layer. -func (p *patchSet[K, V]) DeleteCurrent(key K) { +func (p *PatchSet[K, V]) DeleteCurrent(key K) { if len(p.patches) == 0 { p.Fork() } @@ -282,7 +212,7 @@ func (p *patchSet[K, V]) DeleteCurrent(key K) { } // InCurrentPatch checks if the key is available in the topmost patch layer. -func (p *patchSet[K, V]) InCurrentPatch(key K) bool { +func (p *PatchSet[K, V]) InCurrentPatch(key K) bool { if len(p.patches) == 0 { return false } @@ -296,11 +226,40 @@ func (p *patchSet[K, V]) InCurrentPatch(key K) bool { // pairs across all patches taking the highest number of records possible // this calculation does not consider deleted records - thus it is likely // to be not accurate -func (p *patchSet[K, V]) totalKeyCount() int { +func (p *PatchSet[K, V]) totalKeyCount() int { count := 0 for _, patch := range p.patches { - count += len(patch.Modified) + count += len(patch.modified) } return count } + +// ClonePatchSet creates a deep copy of a PatchSet object with the same patch layers +// structure, while copying keys and values using cloneKey and cloneValue functions +// provided. +// +// This function is intended for testing purposes only. +func ClonePatchSet[K comparable, V any](ps *PatchSet[K, V], cloneKey func(K) K, cloneValue func(V) V) *PatchSet[K, V] { + if ps == nil { + return nil + } + + cloned := NewPatchSet[K, V]() + for _, patch := range ps.patches { + clonedPatch := NewPatch[K, V]() + for key, value := range patch.modified { + clonedKey, clonedValue := cloneKey(key), cloneValue(value) + clonedPatch.Set(clonedKey, clonedValue) + } + + for key := range patch.deleted { + clonedKey := cloneKey(key) + clonedPatch.Delete(clonedKey) + } + + cloned.patches = append(cloned.patches, clonedPatch) + } + + return cloned +} diff --git a/cluster-autoscaler/simulator/dynamicresources/snapshot/patchset_test.go b/cluster-autoscaler/simulator/common/patchset_test.go similarity index 62% rename from cluster-autoscaler/simulator/dynamicresources/snapshot/patchset_test.go rename to cluster-autoscaler/simulator/common/patchset_test.go index 9032e605d7f763eae371168fd8ffafb681a65a85..e6571321bb08bac6c86eecc519463bfb55b35ccc 100644 --- a/cluster-autoscaler/simulator/dynamicresources/snapshot/patchset_test.go +++ b/cluster-autoscaler/simulator/common/patchset_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package snapshot +package common import ( "maps" @@ -23,308 +23,6 @@ import ( "k8s.io/utils/ptr" ) -func TestPatchOperations(t *testing.T) { - p := newPatch[string, int]() - - // 1. Get and IsDeleted on non-existent key - if _, found := p.Get("a"); found { - t.Errorf("Get for 'a' should not find anything") - } - if p.IsDeleted("a") { - t.Errorf("IsDeleted for 'a' should be false") - } - - // 2. Set 'a' key - p.Set("a", 1) - if val, found := p.Get("a"); !found || val != 1 { - t.Errorf("Get('a') = %v,%v, expected 1,true", val, found) - } - if p.IsDeleted("a") { - t.Errorf("IsDeleted('a') = true, should be false") - } - - // 3. Overwrite 'a' key - p.Set("a", 2) - if val, found := p.Get("a"); !found || val != 2 { - t.Errorf("Get('a') = %v,%v, expected 2,true", val, found) - } - if p.IsDeleted("a") { - t.Errorf("IsDeleted('a') = true, should be false") - } - - // 4. Delete 'a' key - p.Delete("a") - if val, found := p.Get("a"); found { - t.Errorf("Get('a') = %v,%v, should not find anything after delete", val, found) - } - if !p.IsDeleted("a") { - t.Errorf("IsDeleted('a') = false, should be true") - } - - // 5. Set 'a' key again after deletion - p.Set("a", 3) - if val, found := p.Get("a"); !found || val != 3 { - t.Errorf("Get('a') = %v,%v, expected 3,true", val, found) - } - if p.IsDeleted("a") { - t.Errorf("IsDeleted('a') = true, should be false") - } - - // 6. Delete a non-existent key 'c' - p.Delete("c") - if val, found := p.Get("c"); found { - t.Errorf("Get('c') = %v, %v, should not find anything", val, found) - } - if !p.IsDeleted("c") { - t.Errorf("IsDeleted('c') = false, should be true") - } -} - -func TestCreatePatch(t *testing.T) { - tests := map[string]struct { - sourceMap map[string]int - addKeys map[string]int - deleteKeys []string - wantModified map[string]int - wantDeleted map[string]bool - }{ - "SourceMapOnlyNoModifications": { - sourceMap: map[string]int{"k1": 1, "k2": 2}, - addKeys: map[string]int{}, - deleteKeys: []string{}, - wantModified: map[string]int{"k1": 1, "k2": 2}, - wantDeleted: map[string]bool{}, - }, - "NilSourceMapAddAndDelete": { - sourceMap: nil, - addKeys: map[string]int{"a": 1, "b": 2}, - deleteKeys: []string{"b"}, - wantModified: map[string]int{"a": 1}, - wantDeleted: map[string]bool{"b": true}, - }, - "EmptySourceMapAddAndDelete": { - sourceMap: map[string]int{}, - addKeys: map[string]int{"x": 10}, - deleteKeys: []string{"y"}, - wantModified: map[string]int{"x": 10}, - wantDeleted: map[string]bool{"y": true}, - }, - "NonEmptySourceMapAddOverwriteDelete": { - sourceMap: map[string]int{"orig1": 100, "orig2": 200}, - addKeys: map[string]int{"new1": 300, "orig1": 101}, - deleteKeys: []string{"orig2", "new1"}, - wantModified: map[string]int{"orig1": 101}, - wantDeleted: map[string]bool{"orig2": true, "new1": true}, - }, - "DeleteKeyFromSourceMap": { - sourceMap: map[string]int{"key_to_delete": 70, "key_to_keep": 80}, - addKeys: map[string]int{}, - deleteKeys: []string{"key_to_delete"}, - wantModified: map[string]int{"key_to_keep": 80}, - wantDeleted: map[string]bool{"key_to_delete": true}, - }, - "AddOnlyNoSourceMap": { - sourceMap: nil, - addKeys: map[string]int{"add1": 10, "add2": 20}, - deleteKeys: []string{}, - wantModified: map[string]int{"add1": 10, "add2": 20}, - wantDeleted: map[string]bool{}, - }, - "DeleteOnlyNoSourceMap": { - sourceMap: nil, - addKeys: map[string]int{}, - deleteKeys: []string{"del1", "del2"}, - wantModified: map[string]int{}, - wantDeleted: map[string]bool{"del1": true, "del2": true}, - }, - "DeleteKeyNotPresentInSourceOrAdded": { - sourceMap: map[string]int{"a": 1}, - addKeys: map[string]int{"b": 2}, - deleteKeys: []string{"c"}, - wantModified: map[string]int{"a": 1, "b": 2}, - wantDeleted: map[string]bool{"c": true}, - }, - "AddKeyThenDeleteIt": { - sourceMap: map[string]int{"base": 100}, - addKeys: map[string]int{"temp": 50}, - deleteKeys: []string{"temp"}, - wantModified: map[string]int{"base": 100}, - wantDeleted: map[string]bool{"temp": true}, - }, - "AllOperations": { - sourceMap: map[string]int{"s1": 1, "s2": 2, "s3": 3}, - addKeys: map[string]int{"a1": 10, "s2": 22, "a2": 20}, - deleteKeys: []string{"s1", "a1", "nonexistent"}, - wantModified: map[string]int{"s2": 22, "s3": 3, "a2": 20}, - wantDeleted: map[string]bool{"s1": true, "a1": true, "nonexistent": true}, - }, - "NoOperationsEmptySource": { - sourceMap: map[string]int{}, - addKeys: map[string]int{}, - deleteKeys: []string{}, - wantModified: map[string]int{}, - wantDeleted: map[string]bool{}, - }, - } - - for testName, test := range tests { - t.Run(testName, func(t *testing.T) { - p := newPatchFromMap(test.sourceMap) - - for k, v := range test.addKeys { - p.Set(k, v) - } - for _, k := range test.deleteKeys { - p.Delete(k) - } - - if !maps.Equal(p.Modified, test.wantModified) { - t.Errorf("Modified map mismatch: got %v, want %v", p.Modified, test.wantModified) - } - - if !maps.Equal(p.Deleted, test.wantDeleted) { - t.Errorf("Deleted map mismatch: got %v, want %v", p.Deleted, test.wantDeleted) - } - }) - } -} - -func TestMergePatchesInPlace(t *testing.T) { - tests := map[string]struct { - modifiedPatchA map[string]int - deletedPatchA map[string]bool - modifiedPatchB map[string]int - deletedPatchB map[string]bool - wantModifiedMerged map[string]int - wantDeletedMerged map[string]bool - }{ - "PatchBOverwritesValueInPatchA": { - modifiedPatchA: map[string]int{"a": 1, "b": 2}, - deletedPatchA: map[string]bool{}, - modifiedPatchB: map[string]int{"b": 22, "c": 3}, - deletedPatchB: map[string]bool{}, - wantModifiedMerged: map[string]int{"a": 1, "b": 22, "c": 3}, - wantDeletedMerged: map[string]bool{}, - }, - "PatchBDeletesValuePresentInPatchA": { - modifiedPatchA: map[string]int{"a": 1, "b": 2}, - deletedPatchA: map[string]bool{}, - modifiedPatchB: map[string]int{}, - deletedPatchB: map[string]bool{"a": true}, - wantModifiedMerged: map[string]int{"b": 2}, - wantDeletedMerged: map[string]bool{"a": true}, - }, - "PatchBDeletesValueAlreadyDeletedInPatchA": { - modifiedPatchA: map[string]int{}, - deletedPatchA: map[string]bool{"x": true}, - modifiedPatchB: map[string]int{}, - deletedPatchB: map[string]bool{"x": true, "y": true}, - wantModifiedMerged: map[string]int{}, - wantDeletedMerged: map[string]bool{"x": true, "y": true}, - }, - "PatchBAddsValuePreviouslyDeletedInPatchA": { - modifiedPatchA: map[string]int{}, - deletedPatchA: map[string]bool{"x": true}, - modifiedPatchB: map[string]int{"x": 10}, - deletedPatchB: map[string]bool{}, - wantModifiedMerged: map[string]int{"x": 10}, - wantDeletedMerged: map[string]bool{}, - }, - "MergeEmptyPatchBIntoNonEmptyPatchA": { - modifiedPatchA: map[string]int{"a": 1}, - deletedPatchA: map[string]bool{"b": true}, - modifiedPatchB: map[string]int{}, - deletedPatchB: map[string]bool{}, - wantModifiedMerged: map[string]int{"a": 1}, - wantDeletedMerged: map[string]bool{"b": true}, - }, - "MergeNonEmptyPatchBIntoEmptyPatchA": { - modifiedPatchA: map[string]int{}, - deletedPatchA: map[string]bool{}, - modifiedPatchB: map[string]int{"a": 1}, - deletedPatchB: map[string]bool{"b": true}, - wantModifiedMerged: map[string]int{"a": 1}, - wantDeletedMerged: map[string]bool{"b": true}, - }, - "MergeTwoEmptyPatches": { - modifiedPatchA: map[string]int{}, - deletedPatchA: map[string]bool{}, - modifiedPatchB: map[string]int{}, - deletedPatchB: map[string]bool{}, - wantModifiedMerged: map[string]int{}, - wantDeletedMerged: map[string]bool{}, - }, - "NoOverlapBetweenPatchAAndPatchBModifications": { - modifiedPatchA: map[string]int{"a1": 1, "a2": 2}, - deletedPatchA: map[string]bool{}, - modifiedPatchB: map[string]int{"b1": 10, "b2": 20}, - deletedPatchB: map[string]bool{}, - wantModifiedMerged: map[string]int{"a1": 1, "a2": 2, "b1": 10, "b2": 20}, - wantDeletedMerged: map[string]bool{}, - }, - "NoOverlapBetweenPatchAAndPatchBDeletions": { - modifiedPatchA: map[string]int{}, - deletedPatchA: map[string]bool{"a1": true, "a2": true}, - modifiedPatchB: map[string]int{}, - deletedPatchB: map[string]bool{"b1": true, "b2": true}, - wantModifiedMerged: map[string]int{}, - wantDeletedMerged: map[string]bool{"a1": true, "a2": true, "b1": true, "b2": true}, - }, - "PatchBOnlyAddsNewKeysPatchAUnchanged": { - modifiedPatchA: map[string]int{"orig": 5}, - modifiedPatchB: map[string]int{"new1": 100, "new2": 200}, - deletedPatchA: map[string]bool{}, - deletedPatchB: map[string]bool{}, - wantModifiedMerged: map[string]int{"orig": 5, "new1": 100, "new2": 200}, - wantDeletedMerged: map[string]bool{}, - }, - "PatchBOnlyDeletesNewKeysPatchAUnchanged": { - modifiedPatchA: map[string]int{"orig": 5}, - deletedPatchA: map[string]bool{}, - modifiedPatchB: map[string]int{}, - deletedPatchB: map[string]bool{"del1": true, "del2": true}, - wantModifiedMerged: map[string]int{"orig": 5}, - wantDeletedMerged: map[string]bool{"del1": true, "del2": true}, - }, - "AllInOne": { - modifiedPatchA: map[string]int{"k1": 1, "k2": 2, "k3": 3}, - deletedPatchA: map[string]bool{"d1": true, "d2": true}, - modifiedPatchB: map[string]int{"k2": 22, "k4": 4, "d1": 11}, - deletedPatchB: map[string]bool{"k3": true, "d2": true, "d3": true}, - wantModifiedMerged: map[string]int{"k1": 1, "k2": 22, "k4": 4, "d1": 11}, - wantDeletedMerged: map[string]bool{"k3": true, "d2": true, "d3": true}, - }, - } - - for testName, test := range tests { - t.Run(testName, func(t *testing.T) { - patchA := newPatchFromMap(test.modifiedPatchA) - for k := range test.deletedPatchA { - patchA.Delete(k) - } - - patchB := newPatchFromMap(test.modifiedPatchB) - for k := range test.deletedPatchB { - patchB.Delete(k) - } - - merged := mergePatchesInPlace(patchA, patchB) - - if merged != patchA { - t.Errorf("mergePatchesInPlace did not modify patchA inplace, references are different") - } - - if !maps.Equal(merged.Modified, test.wantModifiedMerged) { - t.Errorf("Modified map mismatch: got %v, want %v", merged.Modified, test.wantModifiedMerged) - } - - if !maps.Equal(merged.Deleted, test.wantDeletedMerged) { - t.Errorf("Deleted map mismatch: got %v, want %v", merged.Deleted, test.wantDeletedMerged) - } - }) - } -} - func TestPatchSetAsMap(t *testing.T) { tests := map[string]struct { patchLayers []map[string]*int @@ -572,7 +270,7 @@ func TestPatchSetRevert(t *testing.T) { func TestPatchSetForkRevert(t *testing.T) { // 1. Initialize empty patchset - ps := newPatchSet[string, int](newPatch[string, int]()) + ps := NewPatchSet(NewPatch[string, int]()) if initialMap := ps.AsMap(); len(initialMap) != 0 { t.Fatalf("Initial AsMap() got %v, want empty", initialMap) } @@ -602,7 +300,7 @@ func TestPatchSetForkRevert(t *testing.T) { func TestPatchSetForkCommit(t *testing.T) { // 1. Initialize empty patchset - ps := newPatchSet[string, int](newPatch[string, int]()) + ps := NewPatchSet(NewPatch[string, int]()) if initialMap := ps.AsMap(); len(initialMap) != 0 { t.Fatalf("Initial AsMap() got %v, want empty", initialMap) } @@ -726,13 +424,13 @@ func TestPatchSetFindValue(t *testing.T) { func TestPatchSetOperations(t *testing.T) { tests := map[string]struct { patchLayers []map[string]*int - mutatePatchSet func(ps *patchSet[string, int]) + mutatePatchSet func(ps *PatchSet[string, int]) searchKey string wantInCurrent bool }{ "SetCurrent_NewKey_InitiallyEmptyPatchSet": { patchLayers: []map[string]*int{}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.SetCurrent("a", 1) }, searchKey: "a", @@ -740,7 +438,7 @@ func TestPatchSetOperations(t *testing.T) { }, "SetCurrent_NewKey_OnExistingEmptyLayer": { patchLayers: []map[string]*int{{}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.SetCurrent("a", 1) }, searchKey: "a", @@ -748,7 +446,7 @@ func TestPatchSetOperations(t *testing.T) { }, "SetCurrent_OverwriteKey_InCurrentPatch": { patchLayers: []map[string]*int{{"a": ptr.To(10)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.SetCurrent("a", 1) }, searchKey: "a", @@ -756,7 +454,7 @@ func TestPatchSetOperations(t *testing.T) { }, "SetCurrent_OverwriteKey_InLowerPatch_AfterFork": { patchLayers: []map[string]*int{{"a": ptr.To(10)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.Fork() ps.SetCurrent("a", 1) }, @@ -765,7 +463,7 @@ func TestPatchSetOperations(t *testing.T) { }, "DeleteCurrent_ExistingKey_InCurrentPatch": { patchLayers: []map[string]*int{{"a": ptr.To(1)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.DeleteCurrent("a") }, searchKey: "a", @@ -773,7 +471,7 @@ func TestPatchSetOperations(t *testing.T) { }, "DeleteCurrent_ExistingKey_InLowerPatch_AfterFork": { patchLayers: []map[string]*int{{"a": ptr.To(1)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.Fork() ps.DeleteCurrent("a") }, @@ -782,7 +480,7 @@ func TestPatchSetOperations(t *testing.T) { }, "DeleteCurrent_NonExistentKey_OnExistingEmptyLayer": { patchLayers: []map[string]*int{{}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.DeleteCurrent("x") }, searchKey: "x", @@ -790,7 +488,7 @@ func TestPatchSetOperations(t *testing.T) { }, "DeleteCurrent_NonExistentKey_InitiallyEmptyPatchSet": { patchLayers: []map[string]*int{}, // Starts with no layers - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.DeleteCurrent("x") }, searchKey: "x", @@ -798,19 +496,19 @@ func TestPatchSetOperations(t *testing.T) { }, "InCurrentPatch_KeySetInCurrent": { patchLayers: []map[string]*int{{"a": ptr.To(1)}}, - mutatePatchSet: func(ps *patchSet[string, int]) {}, + mutatePatchSet: func(ps *PatchSet[string, int]) {}, searchKey: "a", wantInCurrent: true, }, "InCurrentPatch_KeySetInLower_NotInCurrent_AfterFork": { patchLayers: []map[string]*int{{"a": ptr.To(1)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { ps.Fork() }, + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.Fork() }, searchKey: "a", wantInCurrent: false, }, "InCurrentPatch_KeyDeletedInCurrent": { patchLayers: []map[string]*int{{"a": ptr.To(1)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.DeleteCurrent("a") }, searchKey: "a", @@ -818,7 +516,7 @@ func TestPatchSetOperations(t *testing.T) { }, "InCurrentPatch_NonExistentKey_InitiallyEmptyPatchSet": { patchLayers: []map[string]*int{}, - mutatePatchSet: func(ps *patchSet[string, int]) {}, + mutatePatchSet: func(ps *PatchSet[string, int]) {}, searchKey: "a", wantInCurrent: false, }, @@ -840,25 +538,25 @@ func TestPatchSetOperations(t *testing.T) { func TestPatchSetCache(t *testing.T) { tests := map[string]struct { patchLayers []map[string]*int - mutatePatchSet func(ps *patchSet[string, int]) + mutatePatchSet func(ps *PatchSet[string, int]) wantCache map[string]*int wantCacheInSync bool }{ "Initial_EmptyPatchSet": { patchLayers: []map[string]*int{}, - mutatePatchSet: func(ps *patchSet[string, int]) {}, + mutatePatchSet: func(ps *PatchSet[string, int]) {}, wantCache: map[string]*int{}, wantCacheInSync: false, }, "Initial_WithData_NoCacheAccess": { patchLayers: []map[string]*int{{"a": ptr.To(1)}}, - mutatePatchSet: func(ps *patchSet[string, int]) {}, + mutatePatchSet: func(ps *PatchSet[string, int]) {}, wantCache: map[string]*int{}, wantCacheInSync: false, }, "FindValue_PopulatesCacheForKey": { patchLayers: []map[string]*int{{"a": ptr.To(1), "b": ptr.To(2)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.FindValue("a") }, wantCache: map[string]*int{"a": ptr.To(1)}, @@ -866,7 +564,7 @@ func TestPatchSetCache(t *testing.T) { }, "FindValue_DeletedKey_PopulatesCacheWithNil": { patchLayers: []map[string]*int{{"a": nil, "b": ptr.To(2)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.FindValue("a") }, wantCache: map[string]*int{"a": nil}, @@ -874,7 +572,7 @@ func TestPatchSetCache(t *testing.T) { }, "AsMap_PopulatesAndSyncsCache": { patchLayers: []map[string]*int{{"a": ptr.To(1), "b": nil, "c": ptr.To(3)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.AsMap() }, wantCache: map[string]*int{"a": ptr.To(1), "c": ptr.To(3)}, // Cache does not necessarily track deletions like 'b' key @@ -882,7 +580,7 @@ func TestPatchSetCache(t *testing.T) { }, "SetCurrent_UpdatesCache_NewKey": { patchLayers: []map[string]*int{{}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.SetCurrent("x", 10) }, wantCache: map[string]*int{"x": ptr.To(10)}, @@ -890,7 +588,7 @@ func TestPatchSetCache(t *testing.T) { }, "SetCurrent_UpdatesCache_OverwriteKey": { patchLayers: []map[string]*int{{"x": ptr.To(5)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.FindValue("x") ps.SetCurrent("x", 10) }, @@ -899,7 +597,7 @@ func TestPatchSetCache(t *testing.T) { }, "DeleteCurrent_UpdatesCache": { patchLayers: []map[string]*int{{"x": ptr.To(5)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.FindValue("x") ps.DeleteCurrent("x") }, @@ -908,7 +606,7 @@ func TestPatchSetCache(t *testing.T) { }, "Revert_ClearsAffectedCacheEntries_And_SetsCacheNotInSync": { patchLayers: []map[string]*int{{"a": ptr.To(1)}, {"b": ptr.To(2), "a": ptr.To(11)}}, // Layer 0: a=1; Layer 1: b=2, a=11 - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.FindValue("a") ps.FindValue("b") ps.Revert() @@ -918,7 +616,7 @@ func TestPatchSetCache(t *testing.T) { }, "Revert_OnSyncedCache_SetsCacheNotInSync": { patchLayers: []map[string]*int{{"a": ptr.To(1)}, {"b": ptr.To(2)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.AsMap() ps.Revert() }, @@ -927,7 +625,7 @@ func TestPatchSetCache(t *testing.T) { }, "Commit_DoesNotInvalidateCache_IfValuesConsistent": { patchLayers: []map[string]*int{{"a": ptr.To(1)}, {"b": ptr.To(2)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.FindValue("a") ps.FindValue("b") ps.Commit() @@ -937,7 +635,7 @@ func TestPatchSetCache(t *testing.T) { }, "Commit_OnSyncedCache_KeepsCacheInSync": { patchLayers: []map[string]*int{{"a": ptr.To(1)}, {"b": ptr.To(2)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.AsMap() ps.Commit() }, @@ -946,7 +644,7 @@ func TestPatchSetCache(t *testing.T) { }, "Fork_DoesNotInvalidateCache": { patchLayers: []map[string]*int{{"a": ptr.To(1)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.FindValue("a") ps.Fork() }, @@ -955,7 +653,7 @@ func TestPatchSetCache(t *testing.T) { }, "Fork_OnSyncedCache_KeepsCacheInSync": { patchLayers: []map[string]*int{{"a": ptr.To(1)}}, - mutatePatchSet: func(ps *patchSet[string, int]) { + mutatePatchSet: func(ps *PatchSet[string, int]) { ps.AsMap() ps.Fork() }, @@ -988,14 +686,14 @@ func TestPatchSetCache(t *testing.T) { } } -func buildTestPatchSet[K comparable, V any](t *testing.T, patchLayers []map[K]*V) *patchSet[K, V] { +func buildTestPatchSet[K comparable, V any](t *testing.T, patchLayers []map[K]*V) *PatchSet[K, V] { t.Helper() patchesNumber := len(patchLayers) - patches := make([]*patch[K, V], patchesNumber) + patches := make([]*Patch[K, V], patchesNumber) for i := 0; i < patchesNumber; i++ { layerMap := patchLayers[i] - currentPatch := newPatch[K, V]() + currentPatch := NewPatch[K, V]() for k, vPtr := range layerMap { if vPtr != nil { currentPatch.Set(k, *vPtr) @@ -1006,5 +704,5 @@ func buildTestPatchSet[K comparable, V any](t *testing.T, patchLayers []map[K]*V patches[i] = currentPatch } - return newPatchSet(patches...) + return NewPatchSet(patches...) } diff --git a/cluster-autoscaler/simulator/dynamicresources/snapshot/snapshot.go b/cluster-autoscaler/simulator/dynamicresources/snapshot/snapshot.go index 34768c12610628ebee5d00a1177353c4e1b80c17..cc9007e213310086613e4ea33f4e09aadf413b04 100644 --- a/cluster-autoscaler/simulator/dynamicresources/snapshot/snapshot.go +++ b/cluster-autoscaler/simulator/dynamicresources/snapshot/snapshot.go @@ -23,6 +23,7 @@ import ( apiv1 "k8s.io/api/core/v1" resourceapi "k8s.io/api/resource/v1beta1" "k8s.io/apimachinery/pkg/types" + "k8s.io/autoscaler/cluster-autoscaler/simulator/common" drautils "k8s.io/autoscaler/cluster-autoscaler/simulator/dynamicresources/utils" "k8s.io/autoscaler/cluster-autoscaler/simulator/framework" resourceclaim "k8s.io/dynamic-resource-allocation/resourceclaim" @@ -45,9 +46,9 @@ func GetClaimId(claim *resourceapi.ResourceClaim) ResourceClaimId { // obtained via the Provider. Then, it can be modified using the exposed methods, to simulate scheduling actions // in the cluster. type Snapshot struct { - resourceClaims *patchSet[ResourceClaimId, *resourceapi.ResourceClaim] - resourceSlices *patchSet[string, []*resourceapi.ResourceSlice] - deviceClasses *patchSet[string, *resourceapi.DeviceClass] + resourceClaims *common.PatchSet[ResourceClaimId, *resourceapi.ResourceClaim] + resourceSlices *common.PatchSet[string, []*resourceapi.ResourceSlice] + deviceClasses *common.PatchSet[string, *resourceapi.DeviceClass] } // nonNodeLocalResourceSlicesIdentifier is a special key used in the resourceSlices patchSet @@ -61,25 +62,25 @@ func NewSnapshot(claims map[ResourceClaimId]*resourceapi.ResourceClaim, nodeLoca maps.Copy(slices, nodeLocalSlices) slices[nonNodeLocalResourceSlicesIdentifier] = nonNodeLocalSlices - claimsPatch := newPatchFromMap(claims) - slicesPatch := newPatchFromMap(slices) - devicesPatch := newPatchFromMap(deviceClasses) + claimsPatch := common.NewPatchFromMap(claims) + slicesPatch := common.NewPatchFromMap(slices) + devicesPatch := common.NewPatchFromMap(deviceClasses) return &Snapshot{ - resourceClaims: newPatchSet(claimsPatch), - resourceSlices: newPatchSet(slicesPatch), - deviceClasses: newPatchSet(devicesPatch), + resourceClaims: common.NewPatchSet(claimsPatch), + resourceSlices: common.NewPatchSet(slicesPatch), + deviceClasses: common.NewPatchSet(devicesPatch), } } // NewEmptySnapshot returns a zero initialized Snapshot. func NewEmptySnapshot() *Snapshot { - claimsPatch := newPatch[ResourceClaimId, *resourceapi.ResourceClaim]() - slicesPatch := newPatch[string, []*resourceapi.ResourceSlice]() - devicesPatch := newPatch[string, *resourceapi.DeviceClass]() + claimsPatch := common.NewPatch[ResourceClaimId, *resourceapi.ResourceClaim]() + slicesPatch := common.NewPatch[string, []*resourceapi.ResourceSlice]() + devicesPatch := common.NewPatch[string, *resourceapi.DeviceClass]() return &Snapshot{ - resourceClaims: newPatchSet(claimsPatch), - resourceSlices: newPatchSet(slicesPatch), - deviceClasses: newPatchSet(devicesPatch), + resourceClaims: common.NewPatchSet(claimsPatch), + resourceSlices: common.NewPatchSet(slicesPatch), + deviceClasses: common.NewPatchSet(devicesPatch), } } diff --git a/cluster-autoscaler/simulator/dynamicresources/snapshot/test_utils.go b/cluster-autoscaler/simulator/dynamicresources/snapshot/test_utils.go index 6293f3a8728a578f49cfdbf0c71db43687d2fb27..7a8f2cc32bfca80258856d4c5dbef15f96883939 100644 --- a/cluster-autoscaler/simulator/dynamicresources/snapshot/test_utils.go +++ b/cluster-autoscaler/simulator/dynamicresources/snapshot/test_utils.go @@ -17,57 +17,35 @@ limitations under the License. package snapshot import ( - "maps" - "github.com/google/go-cmp/cmp" resourceapi "k8s.io/api/resource/v1beta1" + "k8s.io/autoscaler/cluster-autoscaler/simulator/common" ) // CloneTestSnapshot creates a deep copy of the provided Snapshot. // This function is intended for testing purposes only. func CloneTestSnapshot(snapshot *Snapshot) *Snapshot { - cloned := &Snapshot{ - deviceClasses: newPatchSet[string, *resourceapi.DeviceClass](), - resourceSlices: newPatchSet[string, []*resourceapi.ResourceSlice](), - resourceClaims: newPatchSet[ResourceClaimId, *resourceapi.ResourceClaim](), - } - - for i := 0; i < len(snapshot.deviceClasses.patches); i++ { - devicesPatch := snapshot.deviceClasses.patches[i] - clonedPatch := newPatch[string, *resourceapi.DeviceClass]() - for key, deviceClass := range devicesPatch.Modified { - clonedPatch.Modified[key] = deviceClass.DeepCopy() + cloneString := func(s string) string { return s } + cloneResourceClaimId := func(rc ResourceClaimId) ResourceClaimId { return rc } + cloneDeviceClass := func(dc *resourceapi.DeviceClass) *resourceapi.DeviceClass { return dc.DeepCopy() } + cloneResourceClaim := func(rc *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { return rc.DeepCopy() } + cloneResourceSlices := func(rcs []*resourceapi.ResourceSlice) []*resourceapi.ResourceSlice { + clone := make([]*resourceapi.ResourceSlice, len(rcs)) + for i := range rcs { + clone[i] = rcs[i].DeepCopy() } - maps.Copy(clonedPatch.Deleted, devicesPatch.Deleted) - cloned.deviceClasses.patches = append(cloned.deviceClasses.patches, clonedPatch) + return clone } - for i := 0; i < len(snapshot.resourceSlices.patches); i++ { - slicesPatch := snapshot.resourceSlices.patches[i] - clonedPatch := newPatch[string, []*resourceapi.ResourceSlice]() - for key, slices := range slicesPatch.Modified { - deepCopySlices := make([]*resourceapi.ResourceSlice, len(slices)) - for i, slice := range slices { - deepCopySlices[i] = slice.DeepCopy() - } - - clonedPatch.Modified[key] = deepCopySlices - } - maps.Copy(clonedPatch.Deleted, slicesPatch.Deleted) - cloned.resourceSlices.patches = append(cloned.resourceSlices.patches, clonedPatch) - } + deviceClasses := common.ClonePatchSet(snapshot.deviceClasses, cloneString, cloneDeviceClass) + resourceSlices := common.ClonePatchSet(snapshot.resourceSlices, cloneString, cloneResourceSlices) + resourceClaims := common.ClonePatchSet(snapshot.resourceClaims, cloneResourceClaimId, cloneResourceClaim) - for i := 0; i < len(snapshot.resourceClaims.patches); i++ { - claimsPatch := snapshot.resourceClaims.patches[i] - clonedPatch := newPatch[ResourceClaimId, *resourceapi.ResourceClaim]() - for claimId, claim := range claimsPatch.Modified { - clonedPatch.Modified[claimId] = claim.DeepCopy() - } - maps.Copy(clonedPatch.Deleted, claimsPatch.Deleted) - cloned.resourceClaims.patches = append(cloned.resourceClaims.patches, clonedPatch) + return &Snapshot{ + deviceClasses: deviceClasses, + resourceSlices: resourceSlices, + resourceClaims: resourceClaims, } - - return cloned } // SnapshotFlattenedComparer returns a cmp.Option that provides a custom comparer function