feat(opensearchservice): support for MultiAZWithStandBy (under feature flag) #26082
Conversation
|
|
github-actions
bot
added
admired-contributor
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
|
Exemption Request. |
aws-cdk-automation
added
pr-linter/exemption-requested
There was a problem hiding this comment.
We do need an integration test for this to ensure that we can create a domain
with this enabled (I don't think it will create currently).
I think we will also need a README entry that covers the requirements and
migration. Ideally we probably want a feature flag that will allow us to set
this to true by default (unless there is a reason that this shouldn't be the
default).
| @@ -1556,6 +1566,7 @@ export class Domain extends DomainBase implements IDomain, ec2.IConnectable { | |||
| : undefined, | |||
| instanceCount, | |||
| instanceType, | |||
| multiAzWithStandbyEnabled, | |||
There was a problem hiding this comment.
| multiAzWithStandbyEnabled, | |
| multiAzWithStandbyEnabled: props.capacity?.multiAzWithStandbyEnabled, |
I don't think there is any reason to explicitly set this to false.
There was a problem hiding this comment.
I've added a feature flag that defaults multiAzWithStandbyEnabled to true.
This is causing some integration tests to fail.
Let me know if the implementation is correct so I can proceed to fix the failing tests.
corymhall
removed
pr/needs-community-review
lpizzinidev
changed the title
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
@lpizzinidev hey what are the current issues that you are blocked upon please let me know if you need any help with getting unblocked |
aws-cdk-automation
added
the
pr/needs-community-review
mrgrain
changed the title
mrgrain
changed the title
aws-cdk-automation
removed
the
pr/needs-community-review
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
@lpizzinidev Failing example otherwise this is good to go, should be an easy fix! |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
aws-cdk-automation
added
the
pr/needs-community-review
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…e flag) (aws#26082) This fix adds support for the [`MultiAZWithStandbyEnabled`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-opensearchservice-domain-clusterconfig.html#:~:text=%3A%20No%20interruption-,MultiAZWithStandbyEnabled,Update%20requires%3A%20No%20interruption,-WarmCount) flag to the [`CapacityConfig`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_opensearchservice.CapacityConfig.html) interface. If enabled, the `ENABLE_OPENSEARCH_MULTIAZ_WITH_STANDBY` feature flag set the default value of `MultiAZWithStandbyEnabled` to `true` Closes aws#26026. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…with standby feature (#29607) ### Issue # (if applicable) Related with #26026 ### Reason for this change #26082 enabled Multi-AZ with Standby by default, but deployment fails if we use t3 instance type, because it does not support the feature. To fail fast, this PR adds validation on synth time. > Multi-AZ with Standby only works with the m5, c5, r5, r6g, c6g, m6g, r6gd and i3 instance types. > https://docs.aws.amazon.com/opensearch-service/latest/developerguide/managedomains-multiaz.html > You can use T3 instance types only if your domain is provisioned without standby. > https://docs.aws.amazon.com/opensearch-service/latest/developerguide/supported-instance-types.html#latest-gen ### Description of changes If the instance type of data node or master node is t3, throws an error. I also considered to automatically set `multiAzWithStandbyEnabled: false` if we detect any t3 instance type, but it would introduce unwanted behavior e.g. in the below case: ```ts // Initial state // multiAzWithStandbyEnabled: true as there's no t3 instance type new Domain(stack, 'Domain', { version: engineVersion, capacity: { dataNodeInstanceType: 'r5.large.search', }, }) // Update domain to add master nodes with t3 instance type new Domain(stack, 'Domain', { version: engineVersion, capacity: { dataNodeInstanceType: 'r5.large.search', masterNodeInstanceType: 't3.medium.search', masterNodes: 3, }, }) // multiAzWithStandbyEnabled suddenly become false! ``` so we just throw an error. ### Description of how you validated changes Added some unit tests. I also confirmed that it results in deployment error if we try to deploy with t3 instance type & `multiAzWithStandbyEnabled : true` for both data node and master node. ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This fix adds support for the
MultiAZWithStandbyEnabledflag to theCapacityConfiginterface.If enabled, the
ENABLE_OPENSEARCH_MULTIAZ_WITH_STANDBYfeature flag set the default value ofMultiAZWithStandbyEnabledtotrueCloses #26026.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license