-
Notifications
You must be signed in to change notification settings - Fork 778
Remove plugins from prepare-fs script in init container #1060
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
Conversation
sebgl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM, I think we can remove a bit more.
There is also a reference to remove in NewPrepareFSInitContainer function comment.
| ###################### | ||
| # Persist the content of bin/, config/ and plugins/ | ||
| # Persist the content of bin/ and config/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to persist bin & config anymore, since was mostly done for additional plugins stuff that can get installed in there.
| ###################### | ||
| # Files persistence # |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PLUGIN_BIN=$ES_DIR/bin/elasticsearch-plugin can also be removed?
| "$PLUGIN_BIN install --batch repository-s3", | ||
| "$PLUGIN_BIN install --batch repository-gcs", | ||
| "mv /usr/share/elasticsearch/config/* /mnt/elastic-internal/elasticsearch-config-local/", | ||
| "mv /usr/share/elasticsearch/bin/* /mnt/elastic-internal/elasticsearch-bin-local/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need these 2 anymore (see above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think I need to revert the last commit and restore the plugin mount again. Otherwise it will be really tricky for users to make this work correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm you're right.
This reverts commit 25b577e.
Fixes #620