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

[BUILD][OSD] Support version qualifier #1836

Merged
merged 5 commits into from
Mar 29, 2022

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Mar 27, 2022

Description

Support version qualifier builds for OpenSearch Dashboards and
OpenSearch Dashboards Plugins.

This will create a zip for plugins with the OpenSearch Dashboards
version + qualifier. But the version of the plugin will remain the
same whilst opensearchDashboardsVersion in the opensearch_dashboards.json
will be the OpenSearch Dashboards version, eg, 1.3.0-alpha1. This is why
the version check needed to be modified a little to ensure that it can
get the version + qualifier and the default version of the plugin.

Updated for plugins as well.

Was able to successfully build and connect OSD 1.3.0-alpha1 to
the release version of OS 1.3.0.

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

Issues Resolved

opensearch-project/OpenSearch-Dashboards#1340

Issue partially resolved:

#1632

Check List

  • 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.

@kavilla kavilla requested a review from a team as a code owner March 27, 2022 10:43
@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2022

Codecov Report

Merging #1836 (37dd8d0) into main (440c3c0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               main    #1836   +/-   ##
=========================================
  Coverage     94.53%   94.54%           
  Complexity       19       19           
=========================================
  Files           176      176           
  Lines          3622     3627    +5     
  Branches         27       27           
=========================================
+ Hits           3424     3429    +5     
  Misses          194      194           
  Partials          4        4           
Impacted Files Coverage Δ
src/build_workflow/build_target.py 100.00% <100.00%> (ø)
..._workflow/opensearch/build_artifact_check_maven.py 95.65% <100.00%> (ø)
...ensearch_dashboards/build_artifact_check_plugin.py 96.55% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 440c3c0...37dd8d0. Read the comment docs.

@kavilla
Copy link
Member Author

kavilla commented Mar 27, 2022

  • Created an "alpha" version of 1.3 OSD (with all the plugins). It was able to connect successfully to release OS 1.3
  • Created an "alpha" of 2.0 OSD was able to successfully connect to alpha 2.0 OS.
  • Created a 1.3 OSD with no qualifier (with all the plugins). It was able to connect successfully to release OS 1.3

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Something feels fishy. Are you saying that an OpenSearch Dashboards version will be different from OpenSearch? Same question for plugins. Do you have an example of what that looks like?

src/build_workflow/build_target.py Outdated Show resolved Hide resolved
@kavilla
Copy link
Member Author

kavilla commented Mar 28, 2022

Something feels fishy. Are you saying that an OpenSearch Dashboards version will be different from OpenSearch? Same question for plugins. Do you have an example of what that looks like?

I'm sorry I'm not sure what you are referring to. Can you expand on this?

@dblock
Copy link
Member

dblock commented Mar 28, 2022

Something feels fishy. Are you saying that an OpenSearch Dashboards version will be different from OpenSearch? Same question for plugins. Do you have an example of what that looks like?

I'm sorry I'm not sure what you are referring to. Can you expand on this?

Why is compatible_opensearch_dashboards_component_versions needed, meaning why is that set different from OpenSearch component versions?

@kavilla
Copy link
Member Author

kavilla commented Mar 28, 2022

Something feels fishy. Are you saying that an OpenSearch Dashboards version will be different from OpenSearch? Same question for plugins. Do you have an example of what that looks like?

I'm sorry I'm not sure what you are referring to. Can you expand on this?

Why is compatible_opensearch_dashboards_component_versions needed, meaning why is that set different from OpenSearch component versions?

Got it I was going to bring this up so thanks for mentioning. With this change this makes it so plugins can install to specific version of OpenSearch Dashboards without doing the update manually. For example, https://github.com/opensearch-project/alerting-dashboards-plugin/blob/main/opensearch_dashboards.json#L3 can remain "version": "1.3.0.0", but this command will update "opensearchDashboardsVersion": "1.3.0", to be the version that is passed in the parameter so for example if building 2.0.0-alpha1 this plugin would look like:

{
  "id": "alertingDashboards",
  "version": "2.0.0.0",
  "opensearchDashboardsVersion": "2.0.0-alpha1",
  "configPath": ["opensearch_alerting"],
  "requiredPlugins": [],
  "server": true,
  "ui": true
}

But with the current condition check it will expect that version to be 2.0.0.0-alpha1 and the only way to change this is if plugins manually update the version for every qualifier. But since that is just used internally with the plugin (as far as I know not sure if plugins do some special check but they don't need to) it shouldn't require that to be change for every qualifier release. So as like as we tell that this plugin is for a specific version it build successfully and run successfully.

So the new logic just allows for the extra .0 without the qualifier.

@kavilla kavilla force-pushed the avillk/osd_support_qualifier branch from 98a1e87 to 37dd8d0 Compare March 29, 2022 01:20
@kavilla kavilla requested a review from dblock March 29, 2022 01:23
@kavilla
Copy link
Member Author

kavilla commented Mar 29, 2022

Not sure why the manifest for OpenSearch 2.0.0 keeps failing but should be good to go.

@dblock
Copy link
Member

dblock commented Mar 29, 2022

@kavilla Why should it be possible to install a 2.0.0 plugin on top of 2.0.0-alpha dashboards? That seems broken by design.

@kavilla
Copy link
Member Author

kavilla commented Mar 29, 2022

@kavilla Why should it be possible to install a 2.0.0 plugin on top of 2.0.0-alpha dashboards? That seems broken by design.

From OpenSearch Dashboards perspective the logic checks opensearchDashboardsVersion if it's defined and ensures the parity. If it's not defined then it will resort to using the plugin version to see if it's compatible. So this means external plugins do not need to match versions if they define opensearchDashboardsVersion and by passing --opensearch-dashboards-version= it specifically builds out the version. So using this will really depend on plugins knowing that they are building out to correct version OpenSearch Dashboards.

From OpenSearch Dashboards perspective if we want to change that logic might need to create a proposal because forks could be always making that assumption.

If we want to handle the prevention of plugins from building 2.0.0 to 2.0.0-alpha within this repo we could delete compatible_opensearch_dashboards_component_versions. Then it would require plugins to version bump for each qualifier.

Copy link
Member

@peterzhuamazon peterzhuamazon left a comment

Choose a reason for hiding this comment

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

Thanks @kavilla I have no questions except DB's latest comment on function names.

Thanks.

@peterzhuamazon
Copy link
Member

Not sure why the manifest for OpenSearch 2.0.0 keeps failing but should be good to go.

@gaiksaya is going to add qualifier support for ci workflow soon.

@gaiksaya
Copy link
Member

Not sure why the manifest for OpenSearch 2.0.0 keeps failing but should be good to go.

@gaiksaya is going to add qualifier support for ci workflow soon.

Not sure if I have the bandwidth but if anyone has please feel free to pick up this issue #1839

@kavilla kavilla requested a review from dblock March 29, 2022 19:04
Support version qualifier builds for OpenSearch Dashboards and
OpenSearch Dashboards Plugins.

This will create a zip for plugins with the OpenSearch Dashboards
version + qualifier. But the version of the plugin will remain the
same whilst opensearchDashboardsVersion in the opensearch_dashboards.json
will be the OpenSearch Dashboards version, eg, 1.3.0-alpha1. This is why
the version check needed to be modified a little to ensure that it can
get the version + qualifier and the default version of the plugin.

Updated for plugins as well.

Was able to successfully build and connect OSD 1.3.0-alpha1 to
the release version of OS 1.3.0.

Issue resolved:
opensearch-project/OpenSearch-Dashboards#1340

Issue partially resolved:
opensearch-project#1632

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
@kavilla kavilla force-pushed the avillk/osd_support_qualifier branch from 64c8f46 to a8391a2 Compare March 29, 2022 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants