Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#Opensearch: missing fields during update cluster configuration #78

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

#Opensearch: missing fields during update cluster configuration #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2617c7f
#2219 more checks
acharviakou Dec 2, 2024
e60e8fe
#2219 add missing fields during cluster modifications
acharviakou Dec 3, 2024
706a4db
#2219 remove duplicated nil check
acharviakou Jan 9, 2025
92fcaf7
#2219 remove duplicated nil check
acharviakou Jan 9, 2025
2b9fbe2
#2219 update e2e tests
acharviakou Jan 13, 2025
0098533
#2219 update e2e tests
acharviakou Jan 14, 2025
242e5b6
#2219 fix test
acharviakou Jan 15, 2025
60bea5e
#2219 fix test
acharviakou Jan 15, 2025
fe51021
#2219 fix test
acharviakou Jan 15, 2025
9d433d0
#2219 fix test
acharviakou Jan 15, 2025
fb6ae54
#2219 fix test
acharviakou Jan 15, 2025
68b7d78
#2219 fix test
acharviakou Jan 16, 2025
909ed31
#2219 fix tests
acharviakou Jan 16, 2025
f01bb05
#2219 fix tests
acharviakou Jan 16, 2025
bedcc6c
#2219 fix tests
acharviakou Jan 16, 2025
6254018
#2219 fix tests
acharviakou Jan 16, 2025
c6bff12
#2219 fix tests
acharviakou Jan 17, 2025
5d3c4c6
#2219 handle autotune options
acharviakou Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions pkg/resource/domain/descriptor.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

132 changes: 121 additions & 11 deletions pkg/resource/domain/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package domain
import (
"context"
"errors"
"strings"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a simple update test checking these updates work? Thanks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for comment
Should I add e2e tests like here in PR? https://github.com/aws-controllers-k8s/opensearchservice-controller/pull/67/files#diff-aee6724aa306271cdd04ef6ed757ce3f297dd5b23d0b198d5012299779261960
Should I add patch commands in the same method or create separate one?

"github.com/aws-controllers-k8s/opensearchservice-controller/apis/v1alpha1"
ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
Expand All @@ -33,6 +35,7 @@ var (
errors.New("domain is currently processing changes, cannot be modified or deleted."),
ackrequeue.DefaultRequeueAfterDuration,
)
noAutoTuneInstances = []string{"t2", "t3"}
)

// domainProcessing returns true if the supplied domain is in a state of
Expand All @@ -44,6 +47,19 @@ func domainProcessing(r *resource) bool {
return *r.ko.Status.Processing
}

// isAutoTuneSupported returns true if instance type supports AutoTune
// https://docs.aws.amazon.com/opensearch-service/latest/developerguide/supported-instance-types.html
func isAutoTuneSupported(r *resource) bool {
if r.ko.Spec.ClusterConfig != nil && r.ko.Spec.ClusterConfig.InstanceType != nil {
for _, v := range noAutoTuneInstances {
if strings.HasPrefix(*r.ko.Spec.ClusterConfig.InstanceType, v) {
return false
}
}
}
return true
}

func (rm *resourceManager) customUpdateDomain(ctx context.Context, desired, latest *resource,
delta *ackcompare.Delta) (updated *resource, err error) {
rlog := ackrtlog.FromContext(ctx)
Expand Down Expand Up @@ -179,6 +195,7 @@ func (rm *resourceManager) customUpdateDomain(ctx context.Context, desired, late
}
ko.Spec.AutoTuneOptions = &v1alpha1.AutoTuneOptionsInput{
DesiredState: resp.DomainConfig.AutoTuneOptions.Options.DesiredState,
UseOffPeakWindow: resp.DomainConfig.AutoTuneOptions.Options.UseOffPeakWindow,
MaintenanceSchedules: maintSchedules,
}
} else {
Expand All @@ -198,17 +215,18 @@ func (rm *resourceManager) customUpdateDomain(ctx context.Context, desired, late
}
}
ko.Spec.ClusterConfig = &v1alpha1.ClusterConfig{
ColdStorageOptions: csOptions,
DedicatedMasterCount: resp.DomainConfig.ClusterConfig.Options.DedicatedMasterCount,
DedicatedMasterEnabled: resp.DomainConfig.ClusterConfig.Options.DedicatedMasterEnabled,
DedicatedMasterType: resp.DomainConfig.ClusterConfig.Options.DedicatedMasterType,
InstanceCount: resp.DomainConfig.ClusterConfig.Options.InstanceCount,
InstanceType: resp.DomainConfig.ClusterConfig.Options.InstanceType,
WarmCount: resp.DomainConfig.ClusterConfig.Options.WarmCount,
WarmEnabled: resp.DomainConfig.ClusterConfig.Options.WarmEnabled,
WarmType: resp.DomainConfig.ClusterConfig.Options.WarmType,
ZoneAwarenessConfig: zaConfig,
ZoneAwarenessEnabled: resp.DomainConfig.ClusterConfig.Options.ZoneAwarenessEnabled,
ColdStorageOptions: csOptions,
DedicatedMasterCount: resp.DomainConfig.ClusterConfig.Options.DedicatedMasterCount,
DedicatedMasterEnabled: resp.DomainConfig.ClusterConfig.Options.DedicatedMasterEnabled,
DedicatedMasterType: resp.DomainConfig.ClusterConfig.Options.DedicatedMasterType,
InstanceCount: resp.DomainConfig.ClusterConfig.Options.InstanceCount,
InstanceType: resp.DomainConfig.ClusterConfig.Options.InstanceType,
WarmCount: resp.DomainConfig.ClusterConfig.Options.WarmCount,
WarmEnabled: resp.DomainConfig.ClusterConfig.Options.WarmEnabled,
WarmType: resp.DomainConfig.ClusterConfig.Options.WarmType,
ZoneAwarenessConfig: zaConfig,
ZoneAwarenessEnabled: resp.DomainConfig.ClusterConfig.Options.ZoneAwarenessEnabled,
MultiAZWithStandbyEnabled: resp.DomainConfig.ClusterConfig.Options.MultiAZWithStandbyEnabled,
}
} else {
ko.Spec.ClusterConfig = nil
Expand Down Expand Up @@ -258,13 +276,53 @@ func (rm *resourceManager) customUpdateDomain(ctx context.Context, desired, late
} else {
ko.Spec.EngineVersion = nil
}
if resp.DomainConfig.IPAddressType != nil {
ko.Spec.IPAddressType = resp.DomainConfig.IPAddressType.Options
} else {
ko.Spec.IPAddressType = nil
}
if resp.DomainConfig.NodeToNodeEncryptionOptions != nil {
ko.Spec.NodeToNodeEncryptionOptions = &v1alpha1.NodeToNodeEncryptionOptions{
Enabled: resp.DomainConfig.NodeToNodeEncryptionOptions.Options.Enabled,
}
} else {
ko.Spec.NodeToNodeEncryptionOptions = nil
}
if resp.DomainConfig.SoftwareUpdateOptions != nil {
ko.Spec.SoftwareUpdateOptions = &v1alpha1.SoftwareUpdateOptions{
AutoSoftwareUpdateEnabled: resp.DomainConfig.SoftwareUpdateOptions.Options.AutoSoftwareUpdateEnabled,
}
} else {
ko.Spec.SoftwareUpdateOptions = nil
}
if resp.DomainConfig.AIMLOptions != nil && resp.DomainConfig.AIMLOptions.Options != nil {
if resp.DomainConfig.AIMLOptions.Options.NaturalLanguageQueryGenerationOptions != nil {
acharviakou marked this conversation as resolved.
Show resolved Hide resolved
ko.Spec.AIMLOptions = &v1alpha1.AIMLOptionsInput{
NATuralLanguageQueryGenerationOptions: &v1alpha1.NATuralLanguageQueryGenerationOptionsInput{
DesiredState: resp.DomainConfig.AIMLOptions.Options.NaturalLanguageQueryGenerationOptions.DesiredState,
},
}
}
} else {
ko.Spec.AIMLOptions = nil
}
if resp.DomainConfig.OffPeakWindowOptions != nil && resp.DomainConfig.OffPeakWindowOptions.Options != nil {
var offPeakWindow *v1alpha1.OffPeakWindow
if resp.DomainConfig.OffPeakWindowOptions.Options.OffPeakWindow != nil {
offPeakWindow = &v1alpha1.OffPeakWindow{
WindowStartTime: &v1alpha1.WindowStartTime{
Hours: resp.DomainConfig.OffPeakWindowOptions.Options.OffPeakWindow.WindowStartTime.Hours,
Minutes: resp.DomainConfig.OffPeakWindowOptions.Options.OffPeakWindow.WindowStartTime.Minutes,
},
}
}
ko.Spec.OffPeakWindowOptions = &v1alpha1.OffPeakWindowOptions{
Enabled: resp.DomainConfig.OffPeakWindowOptions.Options.Enabled,
OffPeakWindow: offPeakWindow,
}
} else {
ko.Spec.OffPeakWindowOptions = nil
}

rm.setStatusDefaults(ko)

Expand Down Expand Up @@ -370,6 +428,9 @@ func (rm *resourceManager) newCustomUpdateRequestPayload(
if desired.ko.Spec.AutoTuneOptions.DesiredState != nil {
f3.SetDesiredState(*desired.ko.Spec.AutoTuneOptions.DesiredState)
}
if desired.ko.Spec.AutoTuneOptions.UseOffPeakWindow != nil {
f3.SetUseOffPeakWindow(*desired.ko.Spec.AutoTuneOptions.UseOffPeakWindow)
}
if desired.ko.Spec.AutoTuneOptions.MaintenanceSchedules != nil {
f3f1 := []*svcsdk.AutoTuneMaintenanceSchedule{}
for _, f3f1iter := range desired.ko.Spec.AutoTuneOptions.MaintenanceSchedules {
Expand Down Expand Up @@ -440,6 +501,9 @@ func (rm *resourceManager) newCustomUpdateRequestPayload(
if desired.ko.Spec.ClusterConfig.ZoneAwarenessEnabled != nil {
f4.SetZoneAwarenessEnabled(*desired.ko.Spec.ClusterConfig.ZoneAwarenessEnabled)
}
if desired.ko.Spec.ClusterConfig.MultiAZWithStandbyEnabled != nil {
f4.SetMultiAZWithStandbyEnabled(*desired.ko.Spec.ClusterConfig.MultiAZWithStandbyEnabled)
}
res.SetClusterConfig(f4)
}

Expand Down Expand Up @@ -557,5 +621,51 @@ func (rm *resourceManager) newCustomUpdateRequestPayload(
res.SetVPCOptions(f14)
}

if desired.ko.Spec.IPAddressType != nil && delta.DifferentAt("Spec.IPAddressType") {
res.SetIPAddressType(*desired.ko.Spec.IPAddressType)
}

if desired.ko.Spec.SoftwareUpdateOptions != nil && delta.DifferentAt("Spec.SoftwareUpdateOptions") {
f15 := &svcsdk.SoftwareUpdateOptions{}
if desired.ko.Spec.SoftwareUpdateOptions.AutoSoftwareUpdateEnabled != nil {
f15.SetAutoSoftwareUpdateEnabled(*desired.ko.Spec.SoftwareUpdateOptions.AutoSoftwareUpdateEnabled)
}
res.SetSoftwareUpdateOptions(f15)
}

if desired.ko.Spec.AIMLOptions != nil && delta.DifferentAt("Spec.AIMLOptions") {
f16 := &svcsdk.AIMLOptionsInput_{}
if desired.ko.Spec.AIMLOptions.NATuralLanguageQueryGenerationOptions != nil {
f16f0 := &svcsdk.NaturalLanguageQueryGenerationOptionsInput_{}
if desired.ko.Spec.AIMLOptions.NATuralLanguageQueryGenerationOptions.DesiredState != nil {
f16f0.SetDesiredState(*desired.ko.Spec.AIMLOptions.NATuralLanguageQueryGenerationOptions.DesiredState)
}
f16.SetNaturalLanguageQueryGenerationOptions(f16f0)
}
res.SetAIMLOptions(f16)
}

if desired.ko.Spec.OffPeakWindowOptions != nil && delta.DifferentAt("Spec.OffPeakWindowOptions") {
f17 := &svcsdk.OffPeakWindowOptions{}
if desired.ko.Spec.OffPeakWindowOptions.Enabled != nil {
f17.SetEnabled(*desired.ko.Spec.OffPeakWindowOptions.Enabled)
}
if desired.ko.Spec.OffPeakWindowOptions.OffPeakWindow != nil {
f17f1 := &svcsdk.OffPeakWindow{}
if desired.ko.Spec.OffPeakWindowOptions.OffPeakWindow.WindowStartTime != nil {
f17f1f1 := &svcsdk.WindowStartTime{}
if desired.ko.Spec.OffPeakWindowOptions.OffPeakWindow.WindowStartTime.Hours != nil {
f17f1f1.SetHours(*desired.ko.Spec.OffPeakWindowOptions.OffPeakWindow.WindowStartTime.Hours)
}
if desired.ko.Spec.OffPeakWindowOptions.OffPeakWindow.WindowStartTime.Minutes != nil {
f17f1f1.SetMinutes(*desired.ko.Spec.OffPeakWindowOptions.OffPeakWindow.WindowStartTime.Minutes)
}
f17f1.SetWindowStartTime(f17f1f1)
}
f17.SetOffPeakWindow(f17f1)
}
res.SetOffPeakWindowOptions(f17)
}

return res, nil
}
20 changes: 20 additions & 0 deletions pkg/resource/domain/sdk.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions templates/hooks/domain/sdk_create_post_set_output.go.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
if resp.DomainStatus.AutoTuneOptions != nil && resp.DomainStatus.AutoTuneOptions.State != nil {
if *resp.DomainStatus.AutoTuneOptions.State == "ERROR" && !isAutoTuneSupported(&resource{ko}){
// t2,t3 instances does not support AutoTuneOptions.DesiredState: DISABLED
// set value manually to remove delta
ko.Spec.AutoTuneOptions.DesiredState = aws.String("DISABLED")
} else {
ko.Spec.AutoTuneOptions.DesiredState = resp.DomainStatus.AutoTuneOptions.State
}
}

if domainProcessing(&resource{ko}) {
// Setting resource synced condition to false will trigger a requeue of
// the resource. No need to return a requeue error here.
Expand Down
10 changes: 10 additions & 0 deletions templates/hooks/domain/sdk_read_one_post_set_output.go.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
if resp.DomainStatus.AutoTuneOptions != nil && resp.DomainStatus.AutoTuneOptions.State != nil {
if *resp.DomainStatus.AutoTuneOptions.State == "ERROR" && !isAutoTuneSupported(&resource{ko}){
// t2,t3 instances does not support AutoTuneOptions.DesiredState: DISABLED
// set value manually to remove delta
ko.Spec.AutoTuneOptions.DesiredState = aws.String("DISABLED")
} else {
ko.Spec.AutoTuneOptions.DesiredState = resp.DomainStatus.AutoTuneOptions.State
}
}

if domainProcessing(&resource{ko}) {
// Setting resource synced condition to false will trigger a requeue of
// the resource. No need to return a requeue error here.
Expand Down
54 changes: 54 additions & 0 deletions test/e2e/tests/test_domain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,60 @@ def test_create_delete_es_2d3m_multi_az_no_vpc_7_9(self, es_2d3m_multi_az_no_vpc
assert cr is not None
assert 'status' in cr
domain.assert_endpoint(cr)
print("before:", domain.get(resource.name))

# modify some cluster parameters to test updates
updates = {
"spec": {
"softwareUpdateOptions": {
"autoSoftwareUpdateEnabled": True
},
"offPeakWindowOptions": {
"enabled": True,
"offPeakWindow": {
"windowStartTime": {
"hours": 23,
"minutes": 30
}
}
}
},
}
# updates = {
# "spec": {
# "AutoTuneOptions": {
# "UseOffPeakWindow": False
# },
# "ClusterConfig": {
# "MultiAZWithStandbyEnabled": False
# },
# "OffPeakWindowOptions": {
# "Enabled": True,
# "OffPeakWindow": {
# "WindowStartTime": {
# "Hours": 23,
# "Minutes": 30
# }
# }
# },
# "SoftwareUpdateOptions": {
# "AutoSoftwareUpdateEnabled": True
# }
# }
# }
k8s.patch_custom_resource(ref, updates)
time.sleep(CHECK_STATUS_WAIT_SECONDS)
print("after check wait:", domain.get(resource.name))
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=30)
latest = domain.get(resource.name)
print("latest:", latest)

# assert latest['DomainStatus']['AutoTuneOptions']['UseOffPeakWindow'] is False
# assert latest['DomainStatus']['ClusterConfig']['MultiAZWithStandbyEnabled'] is False
assert latest['DomainStatus']['OffPeakWindowOptions']["Enabled"] is True
assert latest['DomainStatus']['OffPeakWindowOptions']["OffPeakWindow"]["WindowStartTime"]["Hours"] == 23
assert latest['DomainStatus']['OffPeakWindowOptions']["OffPeakWindow"]["WindowStartTime"]["Minutes"] == 30
assert latest['DomainStatus']['SoftwareUpdateOptions']["AutoSoftwareUpdateEnabled"] is True

def test_create_delete_es_2d3m_multi_az_vpc_2_subnet7_9(self, es_2d3m_multi_az_vpc_2_subnet7_9_domain):
ref, resource = es_2d3m_multi_az_vpc_2_subnet7_9_domain
Expand Down