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

[BUG] fix healthcheck logic to expect object and return ids #2277

Merged

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Sep 6, 2022

Description

Original implementation incorrectly assumed the return list of nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return which is a function for an array.

Also, refactors to return a list of node_ids because the original implementation indicated that it should but it can return node ids but it never did. It only returned null or _local, the problem with this approach is that it doesn't expect valid node version with different DIs or filter out nodes when we pass null as the node ID for the node info call because it was fan out the request to all nodes.

Now this function will return _local if all the nodes share the same cluster_id using a greedy approach since we can assume it is all the same version.

Will return node ids, if the cluster_id are different so it will pass a CSV to the node info call and return the info for those nodes. And null if no cluster_id is present, ie, fan out the response.
Signed-off-by: Kawika Avilla kavilla414@gmail.com

Original Issue

#2203

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

@kavilla kavilla requested a review from a team as a code owner September 6, 2022 22:50
@kavilla
Copy link
Member Author

kavilla commented Sep 6, 2022

Thanks @AMoo-Miki for this help kavilla#45

@codecov-commenter
Copy link

Codecov Report

Merging #2277 (eaad89b) into main (140c56f) will increase coverage by 0.00%.
The diff coverage is 56.25%.

@@           Coverage Diff           @@
##             main    #2277   +/-   ##
=======================================
  Coverage   66.77%   66.77%           
=======================================
  Files        3133     3133           
  Lines       60079    60077    -2     
  Branches     9152     9154    +2     
=======================================
+ Hits        40117    40119    +2     
+ Misses      17771    17765    -6     
- Partials     2191     2193    +2     
Impacted Files Coverage Δ
...nsearch/version_check/ensure_opensearch_version.ts 83.60% <56.25%> (-0.53%) ⬇️
packages/osd-optimizer/src/node/cache.ts 52.77% <0.00%> (+2.77%) ⬆️
...s/osd-optimizer/src/node/node_auto_tranpilation.ts 87.75% <0.00%> (+4.08%) ⬆️

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

joshuarrrr
joshuarrrr previously approved these changes Sep 6, 2022
@@ -35,7 +35,6 @@

import { timer, of, from, Observable } from 'rxjs';
import { map, distinctUntilChanged, catchError, exhaustMap, mergeMap } from 'rxjs/operators';
import { get } from 'lodash';
Copy link
Member

Choose a reason for hiding this comment

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

😎

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
opensearch-project#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
opensearch-project#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
@kavilla kavilla merged commit 3496526 into opensearch-project:main Sep 7, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 7, 2022
Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit 3496526)
ananzh pushed a commit that referenced this pull request Sep 7, 2022
…2283)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit 3496526)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.0 failed:

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

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-2.0 2.0
# Navigate to the new working tree
cd .worktrees/backport-2.0
# Create a new branch
git switch --create backport/backport-2277-to-2.0
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 349652685f84aa0c058315d846b110fcf8d23ee2
# Push it to GitHub
git push --set-upstream origin backport/backport-2277-to-2.0
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.0

Then, create a pull request where the base branch is 2.0 and the compare/head branch is backport/backport-2277-to-2.0.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.2 failed:

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

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-2.2 2.2
# Navigate to the new working tree
cd .worktrees/backport-2.2
# Create a new branch
git switch --create backport/backport-2277-to-2.2
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 349652685f84aa0c058315d846b110fcf8d23ee2
# Push it to GitHub
git push --set-upstream origin backport/backport-2277-to-2.2
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.2

Then, create a pull request where the base branch is 2.2 and the compare/head branch is backport/backport-2277-to-2.2.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.x failed:

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

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
cd .worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-2277-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 349652685f84aa0c058315d846b110fcf8d23ee2
# Push it to GitHub
git push --set-upstream origin backport/backport-2277-to-1.x
# Go back to the original working tree
cd ../..
# 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-2277-to-1.x.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.1 failed:

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

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-2.1 2.1
# Navigate to the new working tree
cd .worktrees/backport-2.1
# Create a new branch
git switch --create backport/backport-2277-to-2.1
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 349652685f84aa0c058315d846b110fcf8d23ee2
# Push it to GitHub
git push --set-upstream origin backport/backport-2277-to-2.1
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.1

Then, create a pull request where the base branch is 2.1 and the compare/head branch is backport/backport-2277-to-2.1.

opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 8, 2022
Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit 3496526)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 8, 2022
Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit 3496526)
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.0 failed:

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

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-2.0 2.0
# Navigate to the new working tree
cd .worktrees/backport-2.0
# Create a new branch
git switch --create backport/backport-2277-to-2.0
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 349652685f84aa0c058315d846b110fcf8d23ee2
# Push it to GitHub
git push --set-upstream origin backport/backport-2277-to-2.0
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.0

Then, create a pull request where the base branch is 2.0 and the compare/head branch is backport/backport-2277-to-2.0.

kavilla added a commit to kavilla/OpenSearch-Dashboards-1 that referenced this pull request Sep 8, 2022
…ch-project#2277)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
opensearch-project#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
opensearch-project#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
kavilla added a commit to kavilla/OpenSearch-Dashboards-1 that referenced this pull request Sep 8, 2022
…ch-project#2277)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
opensearch-project#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
opensearch-project#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
kavilla added a commit that referenced this pull request Sep 8, 2022
…2296)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit 3496526)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
kavilla added a commit that referenced this pull request Sep 8, 2022
…2297)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit 3496526)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
kavilla added a commit that referenced this pull request Sep 8, 2022
…2300)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
kavilla added a commit that referenced this pull request Sep 8, 2022
…2298)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 8, 2022
…2300)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 8, 2022
…2300)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 8, 2022
…2300)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 8, 2022
…2300)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)
kavilla added a commit that referenced this pull request Sep 8, 2022
…2300) (#2304)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
kavilla added a commit that referenced this pull request Sep 8, 2022
…2300) (#2302)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
kavilla added a commit that referenced this pull request Sep 9, 2022
…2300) (#2301)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
kavilla added a commit to kavilla/OpenSearch-Dashboards-1 that referenced this pull request Sep 9, 2022
Add more tests to verify logic added here:
opensearch-project#2277

That makes a request for nodes info with a list of node ids.

Issue:
n/a

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Flyingliuhub added a commit that referenced this pull request Sep 9, 2022
* [BUG] fix healthcheck logic to expect object and return ids (#2277) (#2300) (#2301)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>

* change version

Signed-off-by: Tao liu <liutaoaz@amazon.com>

Signed-off-by: Tao liu <liutaoaz@amazon.com>
Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com>
Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
Flyingliuhub pushed a commit that referenced this pull request Sep 12, 2022
…2300) (#2303)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e10ba34)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this pull request Sep 14, 2022
…ch-project#2277)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
opensearch-project#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
opensearch-project#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Sergey V. Osipov <sipopo@yandex.ru>
pjfitzgibbons pushed a commit to pjfitzgibbons/OpenSearch-Dashboards that referenced this pull request Oct 24, 2022
…ch-project#2277) (opensearch-project#2298)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
opensearch-project#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
opensearch-project#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this pull request Dec 16, 2022
…ch-project#2277)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
opensearch-project#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
opensearch-project#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Sergey V. Osipov <sipopo@yandex.ru>
@ananzh ananzh added the v1.3.6 label Jan 26, 2023
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.

8 participants