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.

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

add respository-s3 plugin install and support to pass custom iam role #71

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

rishabh6788
Copy link
Collaborator

This PR:

  • Adds respository-s3 native plugin install. This will help users to use snapshot feature out of the box, else they need to manually install the plugin and then restart the cluster. This will not affect the existing behavior.
  • Provide option to pass user-defined IAM roles to be used as ec2 instance profile, the default behavior will not change in case the user doesn't provide the custom role arn and a new role will be created and attached to the instances.

Description

Describe what this change achieves.

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.

@@ -475,30 +482,26 @@ export class InfraStack extends Stack {
cwd: '/home/ec2-user',
ignoreErrors: false,
}));
cfnInitConfig.push(InitCommand.shellCommand('set -ex;cd opensearch;sudo -u ec2-user bin/opensearch-plugin install repository-s3 --batch', {
Copy link
Member

Choose a reason for hiding this comment

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

Hi @rishabh6788 ,

Instead of manually adding the plugin name here, can we take a list of native plugins to be installed and then install in one go?
Right now its repository-s3 and discovery-ec2. In future there might be more.

Something like:
native-plugins-to-install: repository-s3, discovery-ec2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is good point. Is it okay if I take that up in future PRs?
@gaiksaya

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #71 (c34638d) into main (07ca896) will increase coverage by 0.10%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##             main      #71      +/-   ##
==========================================
+ Coverage   78.22%   78.33%   +0.10%     
==========================================
  Files           6        6              
  Lines         418      420       +2     
  Branches      125      125              
==========================================
+ Hits          327      329       +2     
  Misses         91       91              
Files Coverage Δ
lib/os-cluster-entrypoint.ts 85.82% <100.00%> (+0.11%) ⬆️
lib/infra/infra-stack.ts 90.69% <85.71%> (+0.07%) ⬆️

| isInternal | Optional | boolean | Boolean flag to make network load balancer internal. Defaults to internet-facing e.g., `--context isInternal=true` |
| enableRemoteStore | Optional | boolean | Boolean flag to enable Remote Store feature e.g., `--context enableRemoteStore=true`. See [Enable Remote Store Feature](#enable-remote-store-feature) for more details. Defaults to false |
| storageVolumeType | Optional | string | EBS volume type for all the nodes (data, ml, cluster manager). Defaults to gp2. See `lib/opensearch-config/node-config.ts` for available options. E.g., `-c storageVolumeType=gp3`. For SSD based instance (i.e. i3 family), it is used for root volume configuration. |
| customRoleArn | Optional | string | User provided IAM role arn to be used as ec2 instance profile. `-c customRoleArn=arn:aws:iam::<AWS_ACCOUNT_ID>:role/<ROLE_NAME>` |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| customRoleArn | Optional | string | User provided IAM role arn to be used as ec2 instance profile. `-c customRoleArn=arn:aws:iam::<AWS_ACCOUNT_ID>:role/<ROLE_NAME>` |
| customRoleArn | Optional | string | User provided IAM role arn to be used as EC2 instance profile. `-c customRoleArn=arn:aws:iam::<AWS_ACCOUNT_ID>:role/<ROLE_NAME>` |

@rishabh6788 rishabh6788 merged commit 401e39e into opensearch-project:main Oct 25, 2023
5 checks passed
rishabh6788 added a commit that referenced this pull request Nov 1, 2023
@opensearch-trigger-bot
Copy link

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-1.x 1.x
# Navigate to the new working tree
pushd ../.worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-71-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 401e39e0f3661928bf24b466a935499990b118ef
# Push it to GitHub
git push --set-upstream origin backport/backport-71-to-1.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-71-to-1.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants