Skip to content

Commit 3a1ab2c

Browse files
committed
fix delta comparison for auto failover, multi AZ, primary cluster ID
1 parent ebb3ad0 commit 3a1ab2c

File tree

7 files changed

+114
-36
lines changed

7 files changed

+114
-36
lines changed
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
ack_generate_info:
2-
build_date: "2021-10-14T18:44:40Z"
2+
build_date: "2021-10-20T20:27:40Z"
33
build_hash: 385779a205bea50e8762b76bc75cab957cf723b9
44
go_version: go1.15.2
55
version: v0.15.1
66
api_directory_checksum: 8e4808b17b3a814f0a624eee8d68a0b45ba5e2c4
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.38.52
99
generator_config_info:
10-
file_checksum: 28e3cd3119df8028a65eb0729a462b4354d27fa9
10+
file_checksum: 1bcc3bfaceacd99d1844e1a6f0b9aa0df0a96edf
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation

apis/v1alpha1/generator.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ resources:
4848
from:
4949
operation: ListAllowedNodeTypeModifications
5050
path: ScaleDownModifications
51+
AutomaticFailoverEnabled:
52+
compare:
53+
is_ignored: true
5154
Events:
5255
is_read_only: true
5356
from:
@@ -62,6 +65,12 @@ resources:
6265
path: ReplicationGroup.LogDeliveryConfigurations
6366
compare: # removes the spec field from automatic delta comparison
6467
is_ignored: true
68+
MultiAZEnabled:
69+
compare:
70+
is_ignored: true
71+
PrimaryClusterId:
72+
compare:
73+
is_ignored: true
6574
hooks:
6675
sdk_read_many_post_set_output:
6776
template_path: hooks/replication_group/sdk_read_many_post_set_output.go.tpl

generator.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ resources:
4848
from:
4949
operation: ListAllowedNodeTypeModifications
5050
path: ScaleDownModifications
51+
AutomaticFailoverEnabled:
52+
compare:
53+
is_ignored: true
5154
Events:
5255
is_read_only: true
5356
from:
@@ -62,6 +65,12 @@ resources:
6265
path: ReplicationGroup.LogDeliveryConfigurations
6366
compare: # removes the spec field from automatic delta comparison
6467
is_ignored: true
68+
MultiAZEnabled:
69+
compare:
70+
is_ignored: true
71+
PrimaryClusterId: # note: "PrimaryClusterID" will not function properly
72+
compare:
73+
is_ignored: true
6574
hooks:
6675
sdk_read_many_post_set_output:
6776
template_path: hooks/replication_group/sdk_read_many_post_set_output.go.tpl

pkg/resource/replication_group/custom_update_api.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,13 @@ func (rm *resourceManager) CustomModifyReplicationGroup(
8080
// 3. updateReplicaCount() is invoked Before updateShardConfiguration()
8181
// because both accept availability zones, however the number of
8282
// values depend on replica count.
83-
if desired.ko.Spec.AutomaticFailoverEnabled != nil && *desired.ko.Spec.AutomaticFailoverEnabled == false {
84-
latestAutomaticFailoverEnabled := latest.ko.Status.AutomaticFailover != nil && *latest.ko.Status.AutomaticFailover == "enabled"
85-
if latestAutomaticFailoverEnabled != *desired.ko.Spec.AutomaticFailoverEnabled {
86-
return rm.modifyReplicationGroup(ctx, desired, latest, delta)
87-
}
83+
//TODO: evaluate if we can remove this chunk - seems like the fields (replica count, shard config) etc.
84+
// will be modified before AF or multi AZ regardless of whether these lines are here or not
85+
if delta.DifferentAt("Spec.AutomaticFailoverEnabled") {
86+
return rm.modifyReplicationGroup(ctx, desired, latest, delta)
8887
}
89-
if desired.ko.Spec.MultiAZEnabled != nil && *desired.ko.Spec.MultiAZEnabled == false {
90-
latestMultiAZEnabled := latest.ko.Status.MultiAZ != nil && *latest.ko.Status.MultiAZ == "enabled"
91-
if latestMultiAZEnabled != *desired.ko.Spec.MultiAZEnabled {
92-
return rm.modifyReplicationGroup(ctx, desired, latest, delta)
93-
}
88+
if delta.DifferentAt("Spec.MultiAZEnabled") {
89+
return rm.modifyReplicationGroup(ctx, desired, latest, delta)
9490
}
9591

9692
// increase/decrease replica count

pkg/resource/replication_group/delta.go

Lines changed: 0 additions & 21 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/resource/replication_group/delta_util.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,19 @@ func filterDelta(
5454
delta.Add("Spec.LogDeliveryConfigurations", desired.ko.Spec.LogDeliveryConfigurations,
5555
unmarshalLastRequestedLDCs(desired))
5656
}
57+
58+
if multiAZRequiresUpdate(desired, latest) {
59+
delta.Add("Spec.MultiAZEnabled", desired.ko.Spec.MultiAZEnabled, latest.ko.Status.MultiAZ)
60+
}
61+
62+
if autoFailoverRequiresUpdate(desired, latest) {
63+
delta.Add("Spec.AutomaticFailoverEnabled", desired.ko.Spec.AutomaticFailoverEnabled,
64+
latest.ko.Status.AutomaticFailover)
65+
}
66+
67+
if updateRequired, current := primaryClusterIDRequiresUpdate(desired, latest); updateRequired {
68+
delta.Add("Spec.PrimaryClusterID", desired.ko.Spec.PrimaryClusterID, *current)
69+
}
5770
}
5871

5972
// returns true if desired and latest engine versions match and false otherwise
@@ -100,3 +113,76 @@ func unmarshalLastRequestedLDCs(desired *resource) []*svcapitypes.LogDeliveryCon
100113

101114
return lastRequestedConfigs
102115
}
116+
117+
// multiAZRequiresUpdate returns true if the latest multi AZ status does not yet match the
118+
// desired state, and false otherwise
119+
func multiAZRequiresUpdate(desired *resource, latest *resource) bool {
120+
// no preference for multi AZ specified; no update required
121+
if desired.ko.Spec.MultiAZEnabled == nil {
122+
return false
123+
}
124+
125+
// API should return a non-nil value, but if it doesn't then attempt to update
126+
if latest.ko.Status.MultiAZ == nil {
127+
return true
128+
}
129+
130+
// true maps to "enabled"; false maps to "disabled"
131+
// this accounts for values such as "enabling" and "disabling"
132+
if *desired.ko.Spec.MultiAZEnabled {
133+
return *latest.ko.Status.MultiAZ != "enabled"
134+
} else {
135+
return *latest.ko.Status.MultiAZ != "disabled"
136+
}
137+
}
138+
139+
// autoFailoverRequiresUpdate returns true if the latest auto failover status does not yet match the
140+
// desired state, and false otherwise
141+
func autoFailoverRequiresUpdate(desired *resource, latest *resource) bool {
142+
// the logic is exactly analogous to multiAZRequiresUpdate above
143+
if desired.ko.Spec.AutomaticFailoverEnabled == nil {
144+
return false
145+
}
146+
147+
if latest.ko.Status.AutomaticFailover == nil {
148+
return true
149+
}
150+
151+
if *desired.ko.Spec.AutomaticFailoverEnabled {
152+
return *latest.ko.Status.AutomaticFailover != "enabled"
153+
} else {
154+
return *latest.ko.Status.AutomaticFailover != "disabled"
155+
}
156+
}
157+
158+
// primaryClusterIDRequiresUpdate retrieves the current primary cluster ID and determines whether
159+
// an update is required. If no desired state is specified or there is an issue retrieving the
160+
// latest state, return false, nil. Otherwise, return false or true depending on equality of
161+
// the latest and desired states, and a non-nil pointer to the latest value
162+
func primaryClusterIDRequiresUpdate(desired *resource, latest *resource) (bool, *string) {
163+
if desired.ko.Spec.PrimaryClusterID == nil {
164+
return false, nil
165+
}
166+
167+
// primary cluster ID applies to cluster mode disabled only; if API returns multiple
168+
// or no node groups, or the provided node group is nil, there is nothing that can be done
169+
if len(latest.ko.Status.NodeGroups) != 1 || latest.ko.Status.NodeGroups[0] == nil {
170+
return false, nil
171+
}
172+
173+
// attempt to find primary cluster in node group. If for some reason it is not present, we
174+
// don't have a reliable latest state, so do nothing
175+
ng := *latest.ko.Status.NodeGroups[0]
176+
for _, member := range ng.NodeGroupMembers {
177+
if member == nil {
178+
continue
179+
}
180+
181+
if member.CurrentRole != nil && *member.CurrentRole == "primary" && member.CacheClusterID != nil {
182+
val := *member.CacheClusterID
183+
return val != *desired.ko.Spec.PrimaryClusterID, &val
184+
}
185+
}
186+
187+
return false, nil
188+
}

test/e2e/tests/test_replicationgroup.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,6 @@ def test_rg_update_misc(self, rg_update_misc_input, rg_update_misc):
555555
assert_misc_fields(reference, rg_update_misc_input['RG_ID'], pmw, description, srl, sw)
556556

557557
# test modifying properties related to tolerance: replica promotion, multi AZ, automatic failover
558-
@pytest.mark.blocked # TODO: remove when passing
559558
def test_rg_fault_tolerance(self, rg_fault_tolerance):
560559
(reference, _) = rg_fault_tolerance
561560
assert k8s.wait_on_condition(reference, "ACK.ResourceSynced", "True", wait_periods=30)
@@ -607,8 +606,8 @@ def test_rg_fault_tolerance(self, rg_fault_tolerance):
607606
raise AssertionError(f"Unknown node {node['cacheClusterID']}")
608607

609608
# assert AF and multi AZ
610-
assert resource['status']['automaticFailover'] == "disabled"
611-
assert resource['status']['multiAZ'] == "disabled"
609+
assert resource['status']['automaticFailover'] == "enabled"
610+
assert resource['status']['multiAZ'] == "enabled"
612611

613612
# test association and disassociation of other resources (VPC security groups, SNS topic, user groups)
614613
@pytest.mark.blocked # TODO: remove when passing

0 commit comments

Comments
 (0)