External changes messages enhancement #657
Conversation
worryg0d
force-pushed
the
external-changes-messages-enhancement
branch
from
ad56415
to
ee316e4
Compare
controllers/clusters/helpers.go
Outdated
|
|
||
| diffs := dcomparison.MapsDiff("spec", k8sSpecMap, iSpecMap) | ||
| if len(diffs) == 1 { | ||
| return fmt.Sprintf( | ||
| "%s Diff: {path: %s, k8sValue: %s, instaclustrValue: %s}", | ||
| externalChangesBaseMsg, | ||
| diffs[0].Path, | ||
| diffs[0].Value1, | ||
| diffs[0].Value2, | ||
| ), nil | ||
| } | ||
|
|
||
| var diffMessages []string | ||
| for _, diff := range diffs { | ||
| diffMessages = append(diffMessages, fmt.Sprintf( | ||
| "{field: %s, k8sValue: %v, instaclustrValue: %v}", | ||
| diff.Path, | ||
| diff.Value1, | ||
| diff.Value2, | ||
| )) | ||
| } |
There was a problem hiding this comment.
There are a lot of cases when our specification differs from instaclustr API specification. Does that mean that in all such cases even if the values are equal, but the structure is different, we will receive such messages about external changes?
There was a problem hiding this comment.
We received the spec from the Instaclustr API and unmarshaled it to the specific struct (e.g we got cassandra cluster details and unmarshal its spec to the Cassandra) before. Also, before calling MapsDiff we deleted redundant fields from the spec as userReferences. We already don't have any differences between Instaclustr and k8s representation of resources specs.
controllers/clusters/helpers.go
Outdated
| if len(diffs) == 1 { | ||
| return fmt.Sprintf( | ||
| "%s Diff: {path: %s, k8sValue: %s, instaclustrValue: %s}", | ||
| externalChangesBaseMsg, | ||
| diffs[0].Path, | ||
| diffs[0].Value1, | ||
| diffs[0].Value2, | ||
| ), nil | ||
| } |
pkg/utils/dcomparison/map_diff.go
Outdated
|
|
||
| // ObjectDiff stores the path to a value and the differing values from two maps. | ||
| type ObjectDiff struct { | ||
| Path string |
controllers/clusters/helpers.go
Outdated
| var diffMessages []string | ||
| for _, diff := range diffs { | ||
| diffMessages = append(diffMessages, fmt.Sprintf( | ||
| "{field: %s, k8sValue: %v, instaclustrValue: %v}", | ||
| diff.Path, | ||
| diff.Value1, | ||
| diff.Value2, | ||
| )) | ||
| } | ||
|
|
||
| return msg + specDifference, nil | ||
| return fmt.Sprintf("%s Diffs: %s", externalChangesBaseMsg, strings.Join(diffMessages, ", ")), nil |
There was a problem hiding this comment.
It will look a bit strange if use instaclustrValue and k8sValue inside methods of ObjectDiff. I decided to create a func that prepares the diff message.
worryg0d
force-pushed
the
external-changes-messages-enhancement
branch
from
ee316e4
to
8aafba7
Compare
worryg0d
force-pushed
the
external-changes-messages-enhancement
branch
from
8aafba7
to
f0e7583
Compare
controllers/clusters/helpers.go
Outdated
| return "", err | ||
| } | ||
|
|
||
| const externalChangesBaseMsg = "There are external changes on the Instaclustr console. Please reconcile the specification manually." |
There was a problem hiding this comment.
Let's move to the models
controllers/clusters/helpers.go
Outdated
|
|
||
| const externalChangesBaseMsg = "There are external changes on the Instaclustr console. Please reconcile the specification manually." | ||
|
|
||
| diffs := dcomparison.MapsDiff("spec", k8sSpecMap, iSpecMap) |
There was a problem hiding this comment.
Let's move the "spec" parameter to the constants also
| slice2 := reflect.ValueOf(value2) | ||
| maxLength := max(slice1.Len(), slice2.Len()) | ||
|
|
||
| for i := 0; i < maxLength; i++ { |
There was a problem hiding this comment.
What if two slices have different lengths?
There was a problem hiding this comment.
It uses the length of the biggest one to iterate over two slices. If they have different lengths it adds the corresponding element path to the result. One of the elements will equal the corresponding value, but the second will be nil.
I added some test cases for this.
There was a problem hiding this comment.
Also, I added the same test cases for nested maps with different lengths.
worryg0d
force-pushed
the
external-changes-messages-enhancement
branch
from
f0e7583
to
2047e24
Compare
Problem
The current implementation of indicating if there are any external changes to the resource spec writes to events the whole k8s and Instaclustr resource specs. It causes problems in identifying which exact field of the k8s resource differs from the value from the Instaclustr spec.
Solution
This PR adds
dcomprasionpkg that provides a deep comparing function. It returns a slice ofObjectDiff. EachObjectDiffstores the field, the value of the field from the first object, and the value from the second.