Skip to content

Commit af798bf

Browse files
authored
Fix MergeConfigs() to work with multiple node pools (#1059)
### Description Fix MergeConfigs() to work with multiple node pools ### Issues Resolved Fixes #1057 ### Check List - [x] Commits are signed per the DCO using --signoff - [x] Unittest added for the new/changed functionality and all unit tests are successful - [x] Customer-visible features documented - [x] No linter warnings (`make lint`) If CRDs are changed: - [x] CRD YAMLs updated (`make manifests`) and also copied into the helm chart - [x] Changes to CRDs documented Please refer to the [PR guidelines](https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/docs/developing.md#submitting-a-pr) before submitting this pull request. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). --------- Signed-off-by: Jim Dickinson <[email protected]>
1 parent 9a6ea21 commit af798bf

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

opensearch-operator/pkg/helpers/helpers.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,14 @@ func GetByComponent(left opsterv1.ComponentStatus, right opsterv1.ComponentStatu
154154
}
155155

156156
func MergeConfigs(left map[string]string, right map[string]string) map[string]string {
157-
if left == nil {
158-
return right
157+
result := make(map[string]string)
158+
for k, v := range left {
159+
result[k] = v
159160
}
160161
for k, v := range right {
161-
left[k] = v
162+
result[k] = v
162163
}
163-
return left
164+
return result
164165
}
165166

166167
// Return the keys of the input map in sorted order

opensearch-operator/pkg/helpers/helpers_test.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package helpers
22

33
import (
4-
opsterv1 "github.com/Opster/opensearch-k8s-operator/opensearch-operator/api/v1"
4+
opsterv1 "github.com/Opster/opensearch-k8s-operator/opensearch-operator/api/v1"
55
. "github.com/onsi/ginkgo/v2"
66
. "github.com/onsi/gomega"
77
corev1 "k8s.io/api/core/v1"
@@ -133,4 +133,32 @@ var _ = Describe("Helper Functions", func() {
133133
})
134134
})
135135
})
136+
137+
Describe("MergeConfigs mutation behavior", func() {
138+
It("should merge the maps such that right is higher priority than left, and not mutate either argument when merging", func() {
139+
generalConfig := map[string]string{"http.compression": "true"}
140+
poolConfig := map[string]string{"node.data": "false"}
141+
142+
// Save a copy of the original
143+
original := map[string]string{"http.compression": "true"}
144+
145+
// Merge and check result
146+
merged := MergeConfigs(generalConfig, poolConfig)
147+
expected := map[string]string{"http.compression": "true", "node.data": "false"}
148+
Expect(merged).To(Equal(expected))
149+
150+
// Check that longLived was not mutated
151+
Expect(generalConfig).To(Equal(original))
152+
153+
// Merge again with a new config
154+
poolConfig2 := map[string]string{"node.master": "false", "http.compression": "false"}
155+
expected2 := map[string]string{"http.compression": "false", "node.master": "false"}
156+
merged2 := MergeConfigs(generalConfig, poolConfig2)
157+
Expect(merged2).To(Equal(expected2))
158+
159+
// Still not mutated
160+
Expect(generalConfig).To(Equal(original))
161+
})
162+
})
163+
136164
})

0 commit comments

Comments
 (0)