opensearch cluster cdk package #2863

rishabh6788 · 2022-11-08T06:21:58Z

Signed-off-by: Rishabh Singh [email protected]

Description

This PR adds capability to spin up single-node and multi-node opensearch cluster using cdk.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov-commenter · 2022-11-08T06:26:20Z

Codecov Report

Merging #2863 (4debce8) into main (23d9485) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2863   +/-   ##
=======================================
  Coverage   93.71%   93.71%           
=======================================
  Files         158      158           
  Lines        4391     4391           
=======================================
  Hits         4115     4115           
  Misses        276      276

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

opensearch-cluster-cdk/lib/opensearch-config/node-config.ts

ylwu-amzn · 2022-11-08T18:32:00Z

opensearch-cluster-cdk/lib/opensearch-config/node-config.ts

+  'node.ingest': false,
+});
+
+nodeConfig.set('seed-data', {


Not related with this line. Can we support creating ML nodes? node.roles: [ml]

Ack! @ylwu-amzn

peternied

Thanks for the contribution, its great to have this be more reusable for the community. Is OpenSearch-Build the right place for this code, it feels like maybe it should be in its own repository?

What the plan going forward for how this will be used/supported?

dblock · 2022-11-08T19:01:21Z

Thanks for the contribution, its great to have this be more reusable for the community. Is OpenSearch-Build the right place for this code, it feels like maybe it should be in its own repository?

What the plan going forward for how this will be used/supported?

Maybe this should go to https://github.com/opensearch-project/opensearch-devops? Or better, its own standalone repo?

bbarani · 2022-11-08T19:21:12Z

Thanks for the contribution, its great to have this be more reusable for the community. Is OpenSearch-Build the right place for this code, it feels like maybe it should be in its own repository?
What the plan going forward for how this will be used/supported?

Maybe this should go to https://github.com/opensearch-project/opensearch-devops? Or better, its own standalone repo?

Yes, I have been thinking about that as well. opensearch-devops repo is currently acting as a pointer to actual repos like Helm charts, Ansible etc.. We can create a new OpenSearch-CDK repo if needed. Any other recommendation on the repo name? @dblock @peternied @gaiksaya @rishabh6788 @peterzhuamazon @prudhvigodithi

Signed-off-by: Rishabh Singh <[email protected]>

gaiksaya

Overall looks great! Needs some work with regards to documentation.
Thanks!

gaiksaya · 2022-11-08T22:32:44Z

opensearch-cluster-cdk/README.md

+
+| Name                                                   | Type     | Description                                                                              |
+|--------------------------------------------------------|:---------|:-----------------------------------------------------------------------------------------|
+| distVersion (mandatory)                                | string   | The OpenSearch distribution version (released/un-released) the user wants to deploy      |


nit: required instead of mandatory

gaiksaya · 2022-11-08T22:33:11Z

opensearch-cluster-cdk/README.md

+| vpcId (Optional)                                       | string   | Re-use existing vpc, provide vpc id                                                      |
+| securityGroupId (Optional)                             | boolean  | Re-use existing security group, provide security group id                                |
+| cidr (Optional)                                        | string   | User provided CIDR block for new Vpc, default is `10.0.0.0/16`                           |
+| managerNodeCount (Optional)                            | number   | Number of cluster manager nodes, default is 3                                            |


nit: integer/float/etc instead of number?

gaiksaya · 2022-11-08T22:34:55Z

opensearch-cluster-cdk/README.md

+#### Please note the load-balancer url is internet facing and can be accessed by anyone.
+To restrict access please refer [Client IP Preservation](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/load-balancer-target-groups.html#client-ip-preservation) to restrict access on internet-facing network load balancer.
+You need to add the ip/prefix-list rule in the security group created in network stack.


Can we do something like opensearch-project/opensearch-ci#171 to restrict server access?

gaiksaya · 2022-11-08T22:35:56Z

opensearch-cluster-cdk/README.md

+|--------------------------------------------------------|:---------|:-----------------------------------------------------------------------------------------|
+| distVersion (mandatory)                                | string   | The OpenSearch distribution version (released/un-released) the user wants to deploy      |
+| securityDisabled (mandatory)                           | boolean  | Enable or disable security plugin                                                        |
+| minDistribution (mandatory)                            | boolean  | Is it an un-released OpenSearch distribution with no plugins                             |


Confused about unreleased so using a released version will be a problem?

gaiksaya · 2022-11-08T22:36:33Z

opensearch-cluster-cdk/bin/app.ts

+const app = new App();
+
+new OsClusterEntrypoint(app, {
+  env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION },


Recommending to remove this and let the default credentials in user's workspace take effect.

gaiksaya · 2022-11-08T22:40:22Z

opensearch-cluster-cdk/README.md

+| distVersion (mandatory)                                | string   | The OpenSearch distribution version (released/un-released) the user wants to deploy      |
+| securityDisabled (mandatory)                           | boolean  | Enable or disable security plugin                                                        |
+| minDistribution (mandatory)                            | boolean  | Is it an un-released OpenSearch distribution with no plugins                             |
+| distributionUrl (mandatory)                            | string   | OpenSearch tar distribution url                                                          |


Please specify that we are using tarball only. Maybe give an example?

gaiksaya · 2022-11-08T22:40:59Z

opensearch-cluster-cdk/lib/opensearch-config/multi-node-base-config.yml

@@ -0,0 +1,5 @@
+cluster.name: "my-stack"


nit: opensearch as the cluster name?

gaiksaya · 2022-11-08T22:42:21Z

opensearch-cluster-cdk/lib/networking/vpc-stack.ts

+    if (props.securityGroupId === undefined) {
+      this.osSecurityGroup = new SecurityGroup(this, 'osSecurityGroup', {
+        vpc: this.vpc,
+        securityGroupName: 'opensearchSG',
+        allowAllOutbound: true,
+      });
+    } else {
+      this.osSecurityGroup = SecurityGroup.fromSecurityGroupId(this, 'osSecurityGroup', props.securityGroupId);
+    }
+
+    /* The security group allows all ip access by default to all the ports.
+    Please update below if you want to restrict access to certain ips and ports */
+    this.osSecurityGroup.addIngressRule(Peer.anyIpv4(), Port.allTcp());
+    this.osSecurityGroup.addIngressRule(this.osSecurityGroup, Port.allTraffic());
+  }


Should this pull this out into a different class to manage security altogether? Since its a critical aspect?

gaiksaya · 2022-11-08T22:42:50Z

opensearch-cluster-cdk/lib/opensearch-config/single-node-base-config.yml

@@ -0,0 +1,4 @@
+cluster.name: my-application


nit: opensearch

gaiksaya · 2022-11-08T22:48:00Z

opensearch-cluster-cdk/README.md

+This project enables uses to deploy either a single-node or a multi-node OpenSearch cluster.
+There are two stacks that get deployed:
+1. OpenSearch-Network-Stack: Use this stack to either use an existing Vpc or create a new Vpc. This stack also creates a new security group to manage access.
+2. OpenSearch-Infra-Stack: Sets up EC2 ASG (installs opensearch and opensearch-dashboards using userdata), cloudwatch logging, load balancer. Check your cluster log in the log group created from your stack in the cloudwatch.


Can you also specify what kind of AMI (AL2/ubuntu/etc) we are using in this? Seeing a lot of /home/ec2-user commands so if the user uses something else, its gonna fail. Or maybe replace those commands with someone $(whoami). Can come as an enhancement

dblock · 2022-11-09T13:57:55Z

Thanks for the contribution, its great to have this be more reusable for the community. Is OpenSearch-Build the right place for this code, it feels like maybe it should be in its own repository?
What the plan going forward for how this will be used/supported?

Maybe this should go to https://github.com/opensearch-project/opensearch-devops? Or better, its own standalone repo?

Yes, I have been thinking about that as well. opensearch-devops repo is currently acting as a pointer to actual repos like Helm charts, Ansible etc.. We can create a new OpenSearch-CDK repo if needed. Any other recommendation on the repo name? @dblock @peternied @gaiksaya @rishabh6788 @peterzhuamazon @prudhvigodithi

opensearch-cdk or opensearch-cluster-cdk are great names IMO

prudhvigodithi · 2022-11-09T16:51:08Z

opensearch-cdk

Ya opensearch-cdk is good choice, In future we can onboard a new product of OpenSearch CDK installation to this same repo :)

rishabh6788 requested a review from a team as a code owner November 8, 2022 06:21

ylwu-amzn reviewed Nov 8, 2022

View reviewed changes

opensearch-cluster-cdk/lib/opensearch-config/node-config.ts Show resolved Hide resolved

ylwu-amzn reviewed Nov 8, 2022

View reviewed changes

peternied reviewed Nov 8, 2022

View reviewed changes

rishabh6788 and others added 2 commits November 8, 2022 13:59

opensearch cluster cdk package

faa405d

Signed-off-by: Rishabh Singh <[email protected]>

Merge branch 'opensearch-project:main' into main

4debce8

gaiksaya reviewed Nov 8, 2022

View reviewed changes

rishabh6788 closed this by deleting the head repository Nov 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

opensearch cluster cdk package #2863

opensearch cluster cdk package #2863

rishabh6788 commented Nov 8, 2022

codecov-commenter commented Nov 8, 2022 •

edited

Loading

ylwu-amzn Nov 8, 2022 •

edited

Loading

rishabh6788 Nov 8, 2022

peternied left a comment

dblock commented Nov 8, 2022 •

edited

Loading

bbarani commented Nov 8, 2022 •

edited

Loading

gaiksaya left a comment

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

gaiksaya Nov 8, 2022

dblock commented Nov 9, 2022

prudhvigodithi commented Nov 9, 2022

opensearch cluster cdk package #2863

opensearch cluster cdk package #2863

Conversation

rishabh6788 commented Nov 8, 2022

Description

Issues Resolved

codecov-commenter commented Nov 8, 2022 • edited Loading

Codecov Report

ylwu-amzn Nov 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternied left a comment

Choose a reason for hiding this comment

dblock commented Nov 8, 2022 • edited Loading

bbarani commented Nov 8, 2022 • edited Loading

gaiksaya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dblock commented Nov 9, 2022

prudhvigodithi commented Nov 9, 2022

codecov-commenter commented Nov 8, 2022 •

edited

Loading

ylwu-amzn Nov 8, 2022 •

edited

Loading

dblock commented Nov 8, 2022 •

edited

Loading

bbarani commented Nov 8, 2022 •

edited

Loading