opensearch-project / opensearch-devops

:smile: We welcome all the community members to help develop third party tools/automations/workflows for OpenSearch/OpenSearch-Dashboards.
https://opensearch.org/
Apache License 2.0
45 stars 36 forks source link

[Enhancement][Multi-Repository Workflow] Better Multi-Repo workflow needed #114

Open nknize opened 1 year ago

nknize commented 1 year ago

Is your feature request related to a problem? Please describe. Changes to opensearch core that break downstream repositories should be caught and repaired earlier.

This comes out of https://github.com/opensearch-project/OpenSearch/pull/6470#issuecomment-1468452646

Describe the solution you'd like Something like Android's git/repo tools where a checkout of a project / plugin repository can also checkout core opensearch and a meta build.gradle sets up the dependencies to use local source instead of SNAPSHOT builds. This would enable us to make the appropriate change (e.g., refactor) in the downstream project repo and open the PR earlier rather than waiting for cascading build failures across the ecosystem.

Describe alternatives you've considered Go back to a monolithic repository.

Additional context Mono vs Multi repository design is not a new concept. I encourage us to spend some time researching prior art, e.g.: https://kinsta.com/blog/monorepo-vs-multi-repo/ to see what's a best fit for the OpenSearch ecosystem.

gaiksaya commented 1 year ago

[Triage] Needs discussion and proposal to move forward.

gaiksaya commented 1 year ago

Hey @nknize ,

We have been brainstorming about this a bit. Adding thoughts here.

There are multiple changes that can be categorized as refactoring such as package level changes (example Xcontent) or class renamings, etc. It is difficult to have one solution for all in detecting what changes were made in core and what affects the downstream projects.

However, we can start with checking the compatibility of each child project. Similar to manifest checks that we have in build repository, we can have some checks in downstream projects to see if the upstream changes are breaking anything. If we need to run full blown gradle build on each project or can we have a lightweight tasks is yet to be decided after some more research but the gist of it is having an optional gradle task in OpenSearch repo (say ./gradlew check-compatibility) which takes an input of a repo name or multiple repos, checks it out and runs the compatibility check in child projects.

If indeed it is a breaking change for child projects, running ./gradlew check-compatibility --notify will just go ahead and create a GitHub issue in that repo as a heads up about any incoming breaking change.

If the PR author wishes to go ahead and fix the issue for each one of the failing components, its their call but we can start with the notification and then iterate upon it. This can be easily automated from developer manually running the gradle task to GHA doing it and notifying author + creating issues in affecting downstream projects.

For something like package refactoring as mentioned above it is simple string replacement in all repos (for most cases) but as I said that is just one example and can be solved like this https://github.com/opensearch-project/opensearch-build-libraries/blob/main/build.gradle#L125-L132 but for more complicated refactoring we might need a robust solution.

image

Let me know what do you think. Also adding @dblock @CEHENKLE @bbarani @prudhvigodithi to the conversation for more inputs.

gaiksaya commented 1 year ago

Looks like ./gradlew assemble is the right task to run from plugins point of view as it is light weight compared to ./gradlew build. Tried below:

  1. Publish opensearch to maven local with xcontent commit
  2. Run ./gradlew assemble on few plugins (alerting, index-management, k-NN, geospatial to be precise) before this change was accommodated in plugins repo and all failed within 15 seconds.
ylwu-amzn commented 1 year ago

Currently plugin teams develop features based on main branch, which depends on OpenSearch core main. That means if OpenSearch core introduce any breaking change, then plugin team have to pause their feature development work to fix the breaking changes first. And some plugins/component like ml-commons/common-utils consumed by other plugins, that mean other plugin teams need to wait for ml-commons/common-utils team fixing the breaking changes first. That time could be even longer.

Another thing is plugin team never knows there are breaking changes until they publish a PR to main branch.

Is it ok for plugin teams to develop feature on a stable branch like 2.x, so they won't be distracted by the breaking changes from OpenSearch core main.

dblock commented 1 year ago

Is it ok for plugin teams to develop feature on a stable branch like 2.x, so they won't be distracted by the breaking changes from OpenSearch core main.

I wouldn't. You're just moving the integration pain to later.

prudhvigodithi commented 1 year ago

Adding my thoughts.

Firstly I would not consider mono repo with challenges related to the increased complexity of managing a large codebase, longer build times, and potential conflicts among teams working on different projects. This also stops us from idea of releasing individual plugins with their own versions.

Thanks for the thought @gaiksaya.

The ./gradlew check-compatibility can be called during a PR creation (similar to existing gradle check) and add a comment to PR (something like this), that shows the list of plugins that could break with the PR from Core that introduce the breaking changes.

However for the gradle check-compatibility to run might take long time and not recommended to run for every PR, consider executing it with only a breaking-change label added to the PR.

We can start by notifying the plugins early and add intelligence down the line to raise a PR for the plugin that can fix the breaking change.

@ylwu-amzn concern can be solved by locking in the core snapshot artifact from the component repo and then come back to the latest snapshot artifact that had the breaking change, this way the component development would not be halted.

dependencies {
    implementation 'your.group:your.artifact:1.0.0-20230513-120000-XYZ' // Replace with the appropriate timestamp or version
}
ylwu-amzn commented 1 year ago

@ylwu-amzn concern can be solved by locking in the core snapshot artifact from the component repo and then come back to the latest snapshot artifact that had the breaking change, this way the component development would not be halted.

Thanks @prudhvigodithi , this looks a way to mitigate the pain. Need to coordinate all plugin teams to use the same snapshot, otherwise plugin will have conflicts.

For this one implementation 'your.group:your.artifact:1.0.0-20230513-120000-XYZ' // Replace with the appropriate timestamp or version, Can we avoid using the timestamp? If we can always use your.group:your.artifact:1.0.0-latest, that can save effort for all plugin teams to coordinate versions and bumping snapshot versions by changing the timestamps.

One concern is, if we use the latest compatible snapshot (or no new breaking change snapshot), we may need some way to detect and notify plugin teams that some new breaking changes happens in core. Once plugin teams address the new breaking changes , we need some way to bump the latest OpenSearch core snapshot to new commit.

gaiksaya commented 1 year ago

Can we avoid using the timestamp? If we can always use your.group:your.artifact:1.0.0-latest, that can save effort for all plugin teams to coordinate versions and bumping snapshot versions by changing the timestamps

Not specifying anything is latest which is updated per push o core repo.

One concern is, if we use the latest compatible snapshot (or no new breaking change snapshot), we may need some way to detect and notify plugin teams that some new breaking changes happens in core. Once plugin teams address the new breaking changes , we need some way to bump the latest OpenSearch core snapshot to new commit.

Snapshots are automatically pushed to sonatype using this workflow https://github.com/opensearch-project/OpenSearch/blob/main/.github/workflows/publish-maven-snapshots.yml once the PR is merged. The scope that this issue concentrates on is detecting what components are affected (if at all) before the change is merged into core and pushed to snapshots repo.

gaiksaya commented 1 year ago

See POC for above approach here: https://github.com/opensearch-project/OpenSearch/pull/7899/files Clone the fork and checkout the branch.

git clone https://github.com/gaiksaya/OpenSearch.git
cd OpenSearch && git checkout comp-check

Run below for specific plugins or later to run compatibility check for all plugins:

./gradlew checkCompatibility -PrepositoryUrls=https://github.com/opensearch-project/common-utils.git,https://github.com/opensearch-project/job-scheduler.git

./gradlew checkCompatibility 
gaiksaya commented 1 year ago

We have opened a GitHub issues in all plugin repositories to ensure they are ready for checking compatibility. See the linked issues above. This is to reduce false positives and negatives checks. Thanks!

bbarani commented 1 year ago

@gaiksaya Can we close this issue?

gaiksaya commented 1 year ago

Will let @nknize take the call. Notifying the failing/incompatible components is the next step. However, it can be tracked as a separate effort.