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.

Sign up for GitHub

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

Jump to bottom

Add console_link library to migration console, implement cat-indices #657

Merged
merged 7 commits into from
May 16, 2024
Merged

Add console_link library to migration console, implement cat-indices #657

merged 7 commits into from
May 16, 2024

Conversation

mikaylathompson
Copy link
Collaborator

@mikaylathompson mikaylathompson commented May 15, 2024 •
edited
Loading

Description

Unintentionally pulled in @gregschohn 's jenkins changes, will rebase branch to get those out.

But the actual content:
the beginnings of a python library that serves as the link between a frontend (CLI in this case, though web API works too) and various backends (source & target clusters, replayer & backfill services, etc.).

image

What's included in this PR specifically

Docker env support for cat indices:

root@ad3133b6248d:~# console_link cat-indices
SOURCE CLUSTER
green open searchguard lpxD3MRARnmbEXmmaqELJQ 1 0 8 0 45.4kb 45.4kb

TARGET CLUSTER
green  open .plugins-ml-config           zibQKJ8HTCuKuK53BSe_2g 1 0    1 0   3.9kb   3.9kb
green  open .opensearch-observability    GZSXkl41RvSVUnXMIAa2Wg 1 0    0 0    208b    208b
green  open sonested                     SFq9CcuhQp-FMK84TvbtvA 1 0 5954 0 950.3kb 950.3kb
yellow open security-auditlog-2024.05.15 6VmmdJjOS9q8QU-GZDCwdg 1 1   27 0   156kb   156kb
yellow open security-auditlog-2024.04.26 b5Ut-5rQQrq5PwYJ5Np5Mg 1 1   30 0 170.5kb 170.5kb
green  open .opendistro_security         phu-ZEoRTGmFWARaxx7O3w 1 0   10 0    79kb    79kb

Issues Resolved

[List any issues this PR will resolve]

Is this a backport? If so, please add backport PR # and/or commits #

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]

Check List

  • New functionality includes testing
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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.
Setup a docker-compose environment to run jenkins and have a couple build options to build with local (current) changes.
I'm also setting up/testing some changes for loading the Jenkinsfile from the repo.

Signed-off-by: Greg Schohn greg.schohn@gmail.com

gregschohn and others added 3 commits May 14, 2024 10:42
@gregschohn
Jenkins improvements for local testing.
864eb6e 
Setup a docker-compose environment to run jenkins and have a couple build options to build with local (current) changes.
I'm also setting up/testing some changes for loading the Jenkinsfile from the repo.

Signed-off-by: Greg Schohn <greg.schohn@gmail.com>
@mikaylathompson
init console_link library
d990bc8 
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
@mikaylathompson
fix bugs, clean up function
5f0b9f1 
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
@lewijacn
Copy link
Collaborator

Couple high level thoughts from scanning over:

  1. Can we move this console_link folder under our lib folder on the migration console, as I imagine we may have a few libraries
  2. What does console_link mean, I think I'm just missing something? Naming is hard though so I'll leave to your discretion, does using a cli command name like console (this could be confusing) or mconsole sound appealing at all?

@mikaylathompson
Add some schema validation, fix replayer
6f0c2ae 
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
@mikaylathompson
Start adding tests
4534f6c 
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
@mikaylathompson
Copy link
Collaborator Author

@lewijacn

  1. Can we move this console_link folder under our lib folder on the migration console, as I imagine we may have a few libraries

Yep, that works! It can really be anywhere as long as we can pip install it.

  1. What does console_link mean, I think I'm just missing something? Naming is hard though so I'll leave to your discretion, does using a cli command name like console (this could be confusing) or mconsole sound appealing at all?

Yeah, I ended up at console link as I'm thinking of this as the piece that links our frontends and backends, but I have zero attachment to it. And at the very least, I like the idea of the cli command being console. If you're logging onto the migration console, it makes sense to then "speak" to the console.

@mikaylathompson
Change cli name to just console
12e8554 
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
@mikaylathompson
Copy link
Collaborator Author

Okay, changed the name of the cli, so the example from above is now:

root@ad3133b6248d:~# console cat-indices   
SOURCE CLUSTER
green open searchguard lpxD3MRARnmbEXmmaqELJQ 1 0 8 0 45.4kb 45.4kb

TARGET CLUSTER
green  open .plugins-ml-config           zibQKJ8HTCuKuK53BSe_2g 1 0    1 0   3.9kb   3.9kb
green  open .opensearch-observability    GZSXkl41RvSVUnXMIAa2Wg 1 0    0 0    208b    208b
green  open sonested                     SFq9CcuhQp-FMK84TvbtvA 1 0 5954 0 950.3kb 950.3kb
yellow open security-auditlog-2024.05.15 6VmmdJjOS9q8QU-GZDCwdg 1 1   27 0   156kb   156kb
yellow open security-auditlog-2024.04.26 b5Ut-5rQQrq5PwYJ5Np5Mg 1 1   30 0 170.5kb 170.5kb
green  open .opendistro_security         phu-ZEoRTGmFWARaxx7O3w 1 0   10 0    79kb    79kb

lewijacn
lewijacn reviewed May 16, 2024
View reviewed changes
TrafficCapture/dockerSolution/src/main/docker/docker-compose.yml Outdated
@@ -95,6 +95,8 @@ services:
- migrations
volumes:
- sharedReplayerOutput:/shared-replayer-output
- ./migrationConsole/console_link/services.yaml:/etc/migration_services.yaml
- ./migrationConsole/console_link:/root/console_link
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this needed since image already has the library?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it's not needed, but it means that I can make code changes locally and instantly test it on the docker container. What do you think about making a note that this is a temporary thing, but keeping it in for now? This seems like the sort of thing that's okay on the e2e branch, but I want a reminder to take it out before we merge to main

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah gotcha, yeah I'm fine with a comment for now

@mikaylathompson
Address review comments & flake8
dfea935 
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
@mikaylathompson mikaylathompson merged commit 1f6e4d1 into opensearch-project:e2e-tests-q2 May 16, 2024
5 checks passed
@mikaylathompson mikaylathompson deleted the console_link_library branch May 16, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndreKurait AndreKurait Awaiting requested review from AndreKurait AndreKurait is a code owner

@chelma chelma Awaiting requested review from chelma chelma is a code owner

@gregschohn gregschohn Awaiting requested review from gregschohn gregschohn is a code owner

@okhasawn okhasawn Awaiting requested review from okhasawn

@peternied peternied Awaiting requested review from peternied peternied is a code owner

@sumobrian sumobrian Awaiting requested review from sumobrian sumobrian is a code owner

@lewijacn lewijacn Awaiting requested review from lewijacn lewijacn is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mikaylathompson @lewijacn @gregschohn