Allow manifest templates to include all the contents while changing version and ref based on core branch properties #4271

peterzhuamazon · 2023-12-06T22:13:07Z

Description

Allow manifest templates to include all the contents while changing version and ref based on core branch properties.

All changes:

Remove very old code which assumes 1.x is the only line with componentsfromsource.
Add all version new templates with proper list of components.
Remove component alternation in favor of directly copy contents from templates, then replace versions and refs before writing to disk.

Thanks.

Issues Resolved

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.

Signed-off-by: Peter Zhu <[email protected]>

codecov · 2023-12-06T22:49:10Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dd41a58) 93.16% compared to head (6f5488a) 91.23%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4271      +/-   ##
==========================================
- Coverage   93.16%   91.23%   -1.94%     
==========================================
  Files         189      189              
  Lines        5973     6102     +129     
==========================================
+ Hits         5565     5567       +2     
- Misses        408      535     +127

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tests/jenkins/data/zip/builds/opensearch/manifest.yml

src/manifests_workflow/input_manifests.py

Signed-off-by: Peter Zhu <[email protected]>

gaiksaya · 2023-12-08T01:14:40Z

src/manifests_workflow/input_manifests.py

        templates_base_path = os.path.join(self.manifests_path(), "templates")
        template_version_folder = version.split(".")[0] + ".x"
        template_full_path = os.path.join(templates_base_path, self.prefix, template_version_folder, "manifest.yml")
        if not os.path.exists(template_full_path):
+            logging.info(f"Not found: {template_full_path}")


Suggested change

logging.info(f"Not found: {template_full_path}")

logging.info(f"Manifest template not found: {template_full_path}")

gaiksaya · 2023-12-08T01:17:10Z

src/manifests_workflow/input_manifests.py

-                            logging.info(f"{component.name}#main is version {release_version} (from {component_version})")
-
-            # summarize
-            logging.info("Found versions on main:")


Won't we be checking the versions on main now?

gaiksaya · 2023-12-08T01:18:21Z

src/manifests_workflow/input_manifests.py

-                    main_versions[version] = [c]
-
-            if component_klass is not None:
-                # components can increment their own version first without incrementing min


Looks like this logic was not right and not useful that's why it was removed? since components won't be able to increment without min

This was working but only limited to OpenSearch and OpenSearch-Dashboards, please correct me if I am wrong? @peterzhuamazon

rishabh6788 · 2023-12-08T21:20:13Z

Looks good but I have a few queries.
Now that we are updating templates with all the components for the workflow to read, update the refs to 2.x branch and write it back for the upcoming release version.
To make it even more simple, why can't we just read last released version input manifest, update the refs to 2.x and write to the new release version manifest? @peterzhuamazon

Also, I did not see where we are confirming whether each component has updated the next release version on 2.x/1.x branch before we update the refs and check in the new input manifest?
If this workflow runs just after 2.12 release, and if I understand the code flow correctly, this workflow will basically add 2.13 input manifest and will update the refs to 2.x (assuming OS/OSD have already updated the versions on their 2.x branch), but it may not be necessary that all the other components have updated the version on their 2.x?

peterzhuamazon · 2023-12-08T23:06:37Z

Hi,

The original setups is assuming there is only 1.x version thus always use the latest one (aka latest patch) as the new template to add components, which is not suitable for now because you cannot use 3.0.0 on 1.x or 2.x.

The original componentfromsource and componentfromdist is useless now as we are not using python to directly call http pull anymore since it is already getting replace with Zelin's incremental build. So that logic is uselss at this point and just add confusing.

The new logic is very simple. If called, get the major version to find the template, copy the entire template and replace version with version, ref with branch. If there is a new entry in the new release, then release manager just add it into the template.

If we dont like the above logic then I would suggest completely remove the template system written by Zelin, and just always pick the latest patch version from the same line.

Thanks.

rishabh6788 · 2023-12-08T23:37:39Z

Hi,

The original setups is assuming there is only 1.x version thus always use the latest one (aka latest patch) as the new template to add components, which is not suitable for now because you cannot use 3.0.0 on 1.x or 2.x.

The original componentfromsource and componentfromdist is useless now as we are not using python to directly call http pull anymore since it is already getting replace with Zelin's incremental build. So that logic is uselss at this point and just add confusing.

The new logic is very simple. If called, get the major version to find the template, copy the entire template and replace version with version, ref with branch. If there is a new entry in the new release, then release manager just add it into the template.

If we dont like the above logic then I would suggest completely remove the template system written by Zelin, and just always pick the latest patch version from the same line.

Thanks.

What happens when all the other components have not yet updated release version on their respective major version branch, e.g., 2x, do we let the builds for next release version fail for those components?

It is fine for non-critical ones but in case CU or JS don't update their versions on their major branches then even though OS is up-to-date the overall build will keep failing till the critical components also update the release version on their major branches.

If that is the case then we should make sure all the critical components, CU and JS along with Core, also update their versions immediately after release, the rest can follow later.

gaiksaya · 2024-01-04T21:01:37Z

It is fine for non-critical ones but in case CU or JS don't update their versions on their major branches then even though OS is up-to-date the overall build will keep failing till the critical components also update the release version on their major branches.

If that is the case then we should make sure all the critical components, CU and JS along with Core, also update their versions immediately after release, the rest can follow later.

+1 I believe even JS CU won't be able to upgrade as almost all plugin's CI dependent on min snapshot artifact and that is only generated once manifest is added to this repo. The flow in current situation looks like this:

OpenSearch repo bumps the version
manifest-update.jenkinsFile detects the change and creates a manifest in build
check-for-build.jenkinsFile detects the change and triggers distribution-build-opensearch job which is responsible for triggering publish-min-snapshot responsible for creating min snapshots that are used in plugin CIs.

Also important note that manifest checks will fail since the code will not be compiled as version increment PRs are not merged.

peterzhuamazon · 2024-01-29T20:52:22Z

We have a different approach on this:

Add placeholder plugins when auto-generating next version's manifests #3707 (comment)

peterzhuamazon added 2 commits December 6, 2023 20:13

Initial implementation of the input manifest template copy

c227b69

Signed-off-by: Peter Zhu <[email protected]>

Add test cases

b155ee9

Signed-off-by: Peter Zhu <[email protected]>

peterzhuamazon added enhancement New Enhancement cicd labels Dec 6, 2023

peterzhuamazon self-assigned this Dec 6, 2023

peterzhuamazon requested review from dblock, bbarani, gaiksaya, rishabh6788, zelinh, jordarlu, prudhvigodithi, Divyaasm and tianleh as code owners December 6, 2023 22:13

github-actions bot added the distinguished-contributor label Dec 6, 2023

peterzhuamazon added 3 commits December 6, 2023 22:13

Remove test manifests

7039348

Signed-off-by: Peter Zhu <[email protected]>

fix import

47cc8dd

Signed-off-by: Peter Zhu <[email protected]>

Remove unused imports

ba3b2cc

Signed-off-by: Peter Zhu <[email protected]>

peterzhuamazon mentioned this pull request Dec 6, 2023

Add placeholder plugins when auto-generating next version's manifests #3707

Closed

zelinh reviewed Dec 7, 2023

View reviewed changes

tests/jenkins/data/zip/builds/opensearch/manifest.yml Outdated Show resolved Hide resolved

rishabh6788 reviewed Dec 7, 2023

View reviewed changes

src/manifests_workflow/input_manifests.py Show resolved Hide resolved

Remove temp yml

6f5488a

Signed-off-by: Peter Zhu <[email protected]>

gaiksaya reviewed Dec 8, 2023

View reviewed changes

peterzhuamazon closed this Jan 29, 2024

peterzhuamazon mentioned this pull request Jan 30, 2024

[Enhancement] Generate Input Manifest to support multiple major versions #4389

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow manifest templates to include all the contents while changing version and ref based on core branch properties #4271

Allow manifest templates to include all the contents while changing version and ref based on core branch properties #4271

peterzhuamazon commented Dec 6, 2023

codecov bot commented Dec 6, 2023 •

edited

Loading

gaiksaya Dec 8, 2023

gaiksaya Dec 8, 2023

gaiksaya Dec 8, 2023

rishabh6788 Dec 8, 2023

rishabh6788 commented Dec 8, 2023 •

edited

Loading

peterzhuamazon commented Dec 8, 2023 •

edited

Loading

rishabh6788 commented Dec 8, 2023

gaiksaya commented Jan 4, 2024 •

edited

Loading

peterzhuamazon commented Jan 29, 2024

	logging.info(f"Not found: {template_full_path}")
	logging.info(f"Manifest template not found: {template_full_path}")

Allow manifest templates to include all the contents while changing version and ref based on core branch properties #4271

Allow manifest templates to include all the contents while changing version and ref based on core branch properties #4271

Conversation

peterzhuamazon commented Dec 6, 2023

Description

Issues Resolved

codecov bot commented Dec 6, 2023 • edited Loading

Codecov Report

gaiksaya Dec 8, 2023

Choose a reason for hiding this comment

gaiksaya Dec 8, 2023

Choose a reason for hiding this comment

gaiksaya Dec 8, 2023

Choose a reason for hiding this comment

rishabh6788 Dec 8, 2023

Choose a reason for hiding this comment

rishabh6788 commented Dec 8, 2023 • edited Loading

peterzhuamazon commented Dec 8, 2023 • edited Loading

rishabh6788 commented Dec 8, 2023

gaiksaya commented Jan 4, 2024 • edited Loading

peterzhuamazon commented Jan 29, 2024

codecov bot commented Dec 6, 2023 •

edited

Loading

rishabh6788 commented Dec 8, 2023 •

edited

Loading

peterzhuamazon commented Dec 8, 2023 •

edited

Loading

gaiksaya commented Jan 4, 2024 •

edited

Loading