-
Notifications
You must be signed in to change notification settings - Fork 100
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
fix bug Observability plugin calling createIndex from onNodeStarted m… #1885
base: 2.x
Are you sure you want to change the base?
Changes from 4 commits
3851e34
5c1b8b2
2537f48
595e5ab
d9964be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,27 +11,32 @@ jobs: | |
build-linux: | ||
needs: Get-CI-Image-Tag | ||
strategy: | ||
# Run all jobs | ||
fail-fast: false | ||
matrix: | ||
java: [11, 17, 21] | ||
runs-on: ubuntu-latest | ||
container: | ||
# using the same image which is used by opensearch-build team to build the OpenSearch Distribution | ||
# this image tag is subject to change as more dependencies and updates will arrive over time | ||
image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }} | ||
# need to switch to root so that github actions can install runner binary on container without permission issues. | ||
options: --user root | ||
options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }} | ||
|
||
env: | ||
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true | ||
|
||
steps: | ||
- uses: actions/checkout@v1 | ||
- name: Run start commands | ||
run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }} | ||
|
||
- uses: actions/checkout@v4 | ||
|
||
- name: Set up JDK ${{ matrix.java }} | ||
uses: actions/setup-java@v1 | ||
uses: actions/setup-java@v4 | ||
with: | ||
java-version: ${{ matrix.java }} | ||
|
||
distribution: 'temurin' | ||
|
||
- name: Run Backwards Compatibility Tests | ||
run: | | ||
echo "Running backwards compatibility tests ..." | ||
|
@@ -56,7 +61,7 @@ jobs: | |
cp -r ./build/distributions/*.zip opensearch-observability-builds/ | ||
|
||
- name: Upload Artifacts | ||
uses: actions/upload-artifact@v1 | ||
uses: actions/upload-artifact@v3-node20 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use v4, as v3 has been deprecated by github. |
||
with: | ||
name: opensearch-observability-ubuntu-latest | ||
path: opensearch-observability-builds | ||
|
@@ -71,11 +76,12 @@ jobs: | |
runs-on: ${{ matrix.os }} | ||
|
||
steps: | ||
- uses: actions/checkout@v1 | ||
- uses: actions/checkout@v4 | ||
|
||
- name: Set up JDK ${{ matrix.java }} | ||
uses: actions/setup-java@v1 | ||
uses: actions/setup-java@v4 | ||
with: | ||
distribution: 'temurin' | ||
java-version: ${{ matrix.java }} | ||
|
||
- name: Build with Gradle | ||
|
@@ -88,7 +94,7 @@ jobs: | |
cp -r ./build/distributions/*.zip opensearch-observability-builds/ | ||
|
||
- name: Upload Artifacts | ||
uses: actions/upload-artifact@v1 | ||
uses: actions/upload-artifact@v3-node20 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
with: | ||
name: opensearch-observability-${{ matrix.os }} | ||
path: opensearch-observability-builds |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,32 +96,41 @@ internal object ObservabilityIndex : LifecycleListener() { | |
*/ | ||
@Suppress("TooGenericExceptionCaught") | ||
private fun createIndex() { | ||
if (!isIndexExists(INDEX_NAME)) { | ||
val classLoader = ObservabilityIndex::class.java.classLoader | ||
val indexMappingSource = classLoader.getResource(OBSERVABILITY_MAPPING_FILE_NAME)?.readText()!! | ||
val indexSettingsSource = classLoader.getResource(OBSERVABILITY_SETTINGS_FILE_NAME)?.readText()!! | ||
val request = CreateIndexRequest(INDEX_NAME) | ||
.mapping(indexMappingSource, XContentType.YAML) | ||
.settings(indexSettingsSource, XContentType.YAML) | ||
try { | ||
val actionFuture = client.admin().indices().create(request) | ||
val response = actionFuture.actionGet(PluginSettings.operationTimeoutMs) | ||
if (response.isAcknowledged) { | ||
log.info("$LOG_PREFIX:Index $INDEX_NAME creation Acknowledged") | ||
reindexNotebooks() | ||
} else { | ||
error("$LOG_PREFIX:Index $INDEX_NAME creation not Acknowledged") | ||
} | ||
} catch (exception: ResourceAlreadyExistsException) { | ||
log.warn("message: ${exception.message}") | ||
} catch (exception: Exception) { | ||
if (exception.cause !is ResourceAlreadyExistsException) { | ||
throw exception | ||
try { | ||
// wait for the cluster here until it will finish managed node election | ||
while (clusterService.state().blocks().hasGlobalBlockWithStatus(RestStatus.SERVICE_UNAVAILABLE)) { | ||
log.info("Wait for cluster to be available ...") | ||
Comment on lines
+101
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the fix!
Do we know why was this only needed for 2.x and what's the path ahead to converge these branches? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ps48 I'm not sure why this was removed in the main branch |
||
TimeUnit.SECONDS.sleep(1) | ||
} | ||
if (!isIndexExists(INDEX_NAME)) { | ||
val classLoader = ObservabilityIndex::class.java.classLoader | ||
val indexMappingSource = classLoader.getResource(OBSERVABILITY_MAPPING_FILE_NAME)?.readText()!! | ||
val indexSettingsSource = classLoader.getResource(OBSERVABILITY_SETTINGS_FILE_NAME)?.readText()!! | ||
val request = CreateIndexRequest(INDEX_NAME) | ||
.mapping(indexMappingSource, XContentType.YAML) | ||
.settings(indexSettingsSource, XContentType.YAML) | ||
try { | ||
val actionFuture = client.admin().indices().create(request) | ||
val response = actionFuture.actionGet(PluginSettings.operationTimeoutMs) | ||
if (response.isAcknowledged) { | ||
log.info("$LOG_PREFIX:Index $INDEX_NAME creation Acknowledged") | ||
reindexNotebooks() | ||
} else { | ||
error("$LOG_PREFIX:Index $INDEX_NAME creation not Acknowledged") | ||
} | ||
} catch (exception: ResourceAlreadyExistsException) { | ||
log.warn("message: ${exception.message}") | ||
} catch (exception: Exception) { | ||
if (exception.cause !is ResourceAlreadyExistsException) { | ||
throw exception | ||
} | ||
} | ||
this.mappingsUpdated = true | ||
} else if (!this.mappingsUpdated) { | ||
updateMappings() | ||
} | ||
this.mappingsUpdated = true | ||
} else if (!this.mappingsUpdated) { | ||
updateMappings() | ||
} catch (exception: Exception) { | ||
log.error("Unexpected exception while initializing node $exception", exception) | ||
} | ||
} | ||
|
||
|
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.
Thanks can be removed as it is deprecated by github.