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

Add CI on whether a change will break plugins #3740

Closed
dblock opened this issue Jun 29, 2022 · 7 comments
Closed

Add CI on whether a change will break plugins #3740

dblock opened this issue Jun 29, 2022 · 7 comments
Labels
CI CI related enhancement Enhancement or improvement to existing feature or request Plugins

Comments

@dblock
Copy link
Member

dblock commented Jun 29, 2022

Is your feature request related to a problem? Please describe.
In #2692 we have backported a Gradle upgrade to 7.4.2 that broke plugins. It was late in the game for 2.1 release and shouldn't have. Had I known that it will break plugins we wouldn't have merged it.

Describe the solution you'd like
Add CI that attempts to run tests for one/some/all the plugins to execute as part of a PR. This way we will know that "this PR will break plugins like so".

@dblock dblock added enhancement Enhancement or improvement to existing feature or request untriaged labels Jun 29, 2022
@dblock
Copy link
Member Author

dblock commented Jun 29, 2022

@ylwu-amzn you'll probably have a lot of thoughts on this one!

@tlfeng
Copy link
Collaborator

tlfeng commented Jun 29, 2022

I agree that the plugin repositories should be notified for any corresponding changes they have to make, caused by the changes in this core repository.
If we want to avoid all changes to the plugins in minor OpenSearch version, then there is no need to notify plugin repositories, after we made promise that plugins don't need to make any changes except "the OpenSearch version number to build with".
@amitgalitz shared his opinions in the issue of "Restrospective 2.1.0 release". In the recent version 2.1.0 release, I know some of the plugins didn't clear with what they need to change and what is not, and many plugins bumping the OpenSearch version to 2.1.0 to start building with version 2.1.0 only within a week before the feature freeze date, so that it makes the plugin maintainers anxious with making the plugins compatible.

While I have concern with the solution that "adding CI that attempts to run tests for plugins as part of a PR".
The PR check will be likely to fail when the version bumps in OpenSearch repository, because the plugin has to bump the OpenSearch version in their build script to be able to build with new version of OpenSearch, and it takes time for the version bump in plugin repository.
Waiting for the specific plugin to bump version to build with a newer version of OpenSearch, will impact the efficiency for the PR merges in this repository. If the PR check for that test in this repo is not mandatory, that will be fine.
However, the current obstacle will likely be disappear, after decoupling the OpenSearch version and the plugin compatible version, which is the project lead by @saratvemulapalli, #2283. But I still have the concern that adding the PR check in this repository which rely on the plugin repositories looks like too much coupling.

Maybe as @amitgalitz suggested, starting running routine build on OpenSearch with all plugins (with the script in https://github.com/opensearch-project/opensearch-build), once OpenSearch bumps version is more feasible. So that nothing in this repository will be stuck when there is a plugin not bumping their OpenSearch version, and we can monitor the build separately.

In addition, I'm having a high expectation on decoupling the OpenSearch version and plugin version. With that achievement, adding such CI or PR check to validate the plugins compatibility will be much easier. 😄

@ylwu-amzn
Copy link
Contributor

ylwu-amzn commented Jun 29, 2022

hi, @dblock thanks for creating this issue to improve our release process. I think plugin team should be aware of such breaking changes as early as possible. Adding CI to run some/all plugins can help catch any breaking part easily. Just have some concern that adding CI in OpenSearch core means it will take much longer time and sometimes the breaking change is expected like change master to cluster_management. So plugin CI failure should not block changes in OpenSearch core.

I think plugin team should make sure they keep same version in <major_version>.x branch with OpenSearch core. Then infra team can run some daily build workflow to check if plugins can build successfully with OpenSearch.

@downsrob
Copy link

While running the entirety of CI takes a long time, just doing build and unit tests likely would take less time than the OpenSearch core tests which would run in parallel. Plugin integration tests are also very flaky, so I think that introducing CI in core to build and maybe run unit tests would help catch these issues immediately. Also, the breaking change would be caught by the person introducing the change, and they will be the most informed on what the plugin owners will need to fix. It may also help to catch cases where a core change breaks plugins without knowing it.

I think that tlfeng brings up a valid point but once we have all plugins on the same branching and we are using automatic version bumps then waiting on plugins to bump versions shouldn't be a big problem.

@peternied
Copy link
Member

It seems like the change would be to adopt building 'canary' plugin within OpenSearch where changes are restricted to major versions. This canary plugin is that it would be good at spotting some issues, like gradle changes; however, it will miss other changes like dependencies or internal interface changes.

The only true way to know if things have been broke is what I've seen called the 'scream test', with plugins adopting [Campaign] Ensure 1.x and 2.x branches (main should be 3.0) then attempting to create a distribution build and message that something is wrong when they are built into the distribution. I think this is basically what we have now in OpenSearch-Build and its associated systems.

@andrross
Copy link
Member

@dblock Does the check compatibility workflow solve this problem?

@dblock
Copy link
Member Author

dblock commented Feb 28, 2024

@andrross Yes, I think so, for plugins we ship together! Closing.

@dblock dblock closed this as completed Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI CI related enhancement Enhancement or improvement to existing feature or request Plugins
Projects
None yet
Development

No branches or pull requests

7 participants