Fix cache parameter group test failure #68
Conversation
echen-98
force-pushed
the
test-failures
branch
from
6d11c98 to
6b48b1a
Compare
|
Fixed unit test failures and removed a unit test redundant with existing declarative unit tests. Reason for that being we can't necessarily expect ReadOne to result in no difference between |
| ko := resource.ko | ||
| if latestCacheCluster.CacheParameterGroup != nil && latestCacheCluster.CacheParameterGroup.CacheParameterGroupName != nil { | ||
| cpgName := *latestCacheCluster.CacheParameterGroup.CacheParameterGroupName | ||
| ko.Spec.CacheParameterGroupName = &cpgName |
There was a problem hiding this comment.
if ko.Spec.CacheParameterGroupName was non-nil and different from latestCacheCluster.CacheParameterGroup.CacheParameterGroupName, why would we overwrite ko.Spec.CacheParameterGroupName here? This smells funny...
There was a problem hiding this comment.
The intention is just to retrieve the cache parameter group currently associated to the replication group. That information comes from the DescribeCacheClusters API since it is not present in the DescribeReplicationGroups API response.
If ko.Spec.CacheParameterGroupName is non-nil and gets overwritten, that's precisely what we want because the original value was copied over from desired, the latest value is different from that, and the controller will go on to detect a diff and invoke a Modify call with the desired CacheParameterGroupName.
There was a problem hiding this comment.
aws-controllers-k8s/community#846 (comment)
This falls under "case 3" of spec fields as I mentioned in this issue a while back
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: echen-98, kumargauravsharma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
Description of changes:
Associating then updating a cache parameter group to a replication group previously did not work. Now we populate
latest.Spec.CacheParameterGroupNamefrom the response of theDescribeCacheClustersAPI.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.