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 incremental components through build workflow with loading previous build manifest #4289

Merged
merged 13 commits into from
Jan 4, 2024
4 changes: 2 additions & 2 deletions jenkins/opensearch/distribution-build.jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pipeline {
def snapshotBuild =
build job: 'publish-opensearch-min-snapshots',
propagate: false,
wait: false,
wait: false,
parameters: [
string(name: 'INPUT_MANIFEST', value: "${INPUT_MANIFEST}"),
]
Expand Down Expand Up @@ -849,4 +849,4 @@ def markStageUnstableIfPluginsFailedToBuild() {
if (stageLogs.any{e -> e.contains('Failed plugins are')}) {
unstable('Some plugins failed to build. See the ./build.sh step for logs and more details')
}
}
}
21 changes: 17 additions & 4 deletions src/build_workflow/README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
- [Building from Source](#building-from-source)
- [OpenSearch](#opensearch)
- [OpenSearch Dashboards](#opensearch-dashboards)
- [Build Paths](#build-paths)
- [Build.sh Options](#buildsh-options)
- [Custom Build Scripts](#custom-build-scripts)
- [Avoiding Rebuilds](#avoiding-rebuilds)
- [Build Paths](#build-paths)
- [Build.sh Options](#buildsh-options)
- [Custom Build Scripts](#custom-build-scripts)
- [Avoiding Rebuilds](#avoiding-rebuilds)
- [Incremental Build](#incremental-build)

## Building from Source

Expand Down Expand Up @@ -101,3 +102,15 @@ fi
```

The [Jenkins workflows](../../jenkins) in this repository can use this mechanism to avoid rebuilding all of OpenSearch and OpenSearch Dashboards unnecessarily.

### Incremental Build

This functionality augments the existing build process by introducing the `--incremental` binary parameter.

Sample command: `./build.sh manifests/2.12.0/opensearch-2.12.0.yml --incremental`.

The build workflow will examine the build manifest from the previous build using the example path `tar/builds/opensearch/manifest.yml` when this command is executed.
The build workflow will be executed in accordance with the comparison between the commits for each component in the preceding build manifest and the current input manifest.
It will contain every modified component, and every component that relies on these revised components based on the `depends_on` entry in the input manifest.

Once build is finished, a new build manifest will be generated using the previous build manifest as a reference, ensuring that all non-modified components remain unchanged.
gaiksaya marked this conversation as resolved.
Show resolved Hide resolved
29 changes: 18 additions & 11 deletions src/build_workflow/build_recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@


class BuildRecorder:
def __init__(self, target: BuildTarget) -> None:
self.build_manifest = self.BuildManifestBuilder(target)
def __init__(self, target: BuildTarget, build_manifest: BuildManifest = None) -> None:
self.build_manifest = self.BuildManifestBuilder(target, build_manifest)
self.target = target
self.name = target.name

Expand Down Expand Up @@ -53,18 +53,24 @@ def write_manifest(self) -> None:
logging.info(f"Created build manifest {manifest_path}")

class BuildManifestBuilder:
def __init__(self, target: BuildTarget) -> None:
def __init__(self, target: BuildTarget, build_manifest: BuildManifest = None) -> None:
self.data: Dict[str, Any] = {}
self.data["build"] = {}
self.data["build"]["id"] = target.build_id
self.data["build"]["name"] = target.name
self.data["build"]["version"] = target.opensearch_version
self.data["build"]["platform"] = target.platform
self.data["build"]["architecture"] = target.architecture
self.data["build"]["distribution"] = target.distribution if target.distribution else "tar"
self.data["schema-version"] = "1.2"
self.components_hash: Dict[str, Dict[str, Any]] = {}

if build_manifest:
self.data = build_manifest.__to_dict__()
for component in build_manifest.components.select():
self.components_hash[component.name] = component.__to_dict__()
else:
self.data["build"] = {}
self.data["build"]["id"] = target.build_id
self.data["build"]["name"] = target.name
self.data["build"]["version"] = target.opensearch_version
self.data["build"]["platform"] = target.platform
self.data["build"]["architecture"] = target.architecture
self.data["build"]["distribution"] = target.distribution if target.distribution else "tar"
self.data["schema-version"] = "1.2"

def append_component(self, name: str, version: str, repository_url: str, ref: str, commit_id: str) -> None:
component = {
"name": name,
Expand All @@ -75,6 +81,7 @@ def append_component(self, name: str, version: str, repository_url: str, ref: st
"version": version,
}
self.components_hash[name] = component
logging.info(f"Appended {name} component in input manifest.")
gaiksaya marked this conversation as resolved.
Show resolved Hide resolved

def append_artifact(self, component: str, type: str, path: str) -> None:
artifacts = self.components_hash[component]["artifacts"]
Expand Down
19 changes: 15 additions & 4 deletions src/run_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from build_workflow.build_recorder import BuildRecorder
from build_workflow.build_target import BuildTarget
from build_workflow.builders import Builders
from manifests.build_manifest import BuildManifest
from manifests.input_manifest import InputManifest
from paths.build_output_dir import BuildOutputDir
from system import console
Expand All @@ -25,6 +26,8 @@ def main() -> int:
args = BuildArgs()
console.configure(level=args.logging_level)
manifest = InputManifest.from_file(args.manifest)
build_manifest = None
components = args.components
failed_plugins = []

if args.ref_manifest:
Expand All @@ -45,8 +48,16 @@ def main() -> int:
if args.incremental:
buildIncremental = BuildIncremental(manifest, args.distribution)
list_of_updated_plugins = buildIncremental.commits_diff(manifest)
logging.info(f"Plugins for incremental build: {buildIncremental.rebuild_plugins(list_of_updated_plugins, manifest)}")
return 0
components = buildIncremental.rebuild_plugins(list_of_updated_plugins, manifest)
logging.info(f"Plugins for incremental build: {components}")

build_manifest_path = os.path.join(args.distribution, "builds", manifest.build.filename, "manifest.yml")
if not os.path.exists(build_manifest_path):
logging.error(f"Previous build manifest missing at path: {build_manifest_path}")

logging.info(f"Build {components} incrementally.")

build_manifest = BuildManifest.from_path(build_manifest_path)

with TemporaryDirectory(keep=args.keep, chdir=True) as work_dir:
logging.info(f"Building in {work_dir.name}")
Expand All @@ -63,11 +74,11 @@ def main() -> int:
architecture=args.architecture or manifest.build.architecture,
)

build_recorder = BuildRecorder(target)
build_recorder = BuildRecorder(target, build_manifest) if args.incremental else BuildRecorder(target)

logging.info(f"Building {manifest.build.name} ({target.architecture}) into {target.output_dir}")

for component in manifest.components.select(focus=args.components, platform=target.platform):
for component in manifest.components.select(focus=components, platform=target.platform):
gaiksaya marked this conversation as resolved.
Show resolved Hide resolved
zelinh marked this conversation as resolved.
Show resolved Hide resolved
logging.info(f"Building {component.name}")

builder = Builders.builder_from(component, target)
Expand Down
162 changes: 161 additions & 1 deletion tests/test_run_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import tempfile
import unittest
from typing import Any
from unittest.mock import MagicMock, Mock, patch
from unittest.mock import MagicMock, Mock, call, patch

import pytest

from build_workflow.build_incremental import BuildIncremental
from manifests.build_manifest import BuildManifest
from manifests.input_manifest import InputManifest
from run_build import main

Expand Down Expand Up @@ -40,6 +42,17 @@ def test_usage(self) -> None:
OPENSEARCH_MANIFEST_2_12 = os.path.realpath(os.path.join(MANIFESTS, "templates", "opensearch", "2.x", "os-template-2.12.0.yml"))
NON_OPENSEARCH_MANIFEST = os.path.realpath(os.path.join(MANIFESTS, "templates", "opensearch", "1.x", "non-os-template-1.1.0.yml"))

INPUT_MANIFEST_PATH = os.path.realpath(os.path.join(os.path.dirname(__file__), "tests_build_workflow", "data", "opensearch-input-2.12.0.yml"))
INPUT_MANIFEST = InputManifest.from_path(INPUT_MANIFEST_PATH)
BUILD_MANIFEST = BuildManifest.from_path(
os.path.join(os.path.dirname(__file__), "tests_build_workflow", "data", "opensearch-build-tar-2.12.0.yml"))
BUILD_MANIFEST_PATH = os.path.join(os.path.dirname(__file__), "tests_build_workflow", "data", "opensearch-build-tar-2.12.0.yml")
INPUT_MANIFEST_DASHBOARDS = InputManifest.from_path(
os.path.join(os.path.dirname(__file__), "tests_build_workflow", "data", "opensearch-dashboards-input-2.12.0.yml"))
BUILD_MANIFEST_DASHBOARDS = BuildManifest.from_path(
os.path.join(os.path.dirname(__file__), "tests_build_workflow", "data", "opensearch-dashboards-build-tar-2.12.0.yml"))
buildIncremental = BuildIncremental(INPUT_MANIFEST, "tar")

@patch("argparse._sys.argv", ["run_build.py", OPENSEARCH_MANIFEST, "-p", "linux"])
@patch("run_build.Builders.builder_from", return_value=MagicMock())
@patch("run_build.BuildRecorder", return_value=MagicMock())
Expand Down Expand Up @@ -229,3 +242,150 @@ def test_failed_plugins_default(self, mock_logging_error: Mock, mock_temp: Mock,
with pytest.raises(Exception, match="Error during build"):
main()
mock_logging_error.assert_called_with(f"Error building common-utils, retry with: run_build.py {self.NON_OPENSEARCH_MANIFEST} --component common-utils")

@patch("argparse._sys.argv", ["run_build.py", OPENSEARCH_MANIFEST, "--incremental"])
@patch("os.path.exists")
@patch("run_build.Builders.builder_from", return_value=MagicMock())
@patch("run_build.BuildRecorder", return_value=MagicMock())
@patch("manifests.build_manifest.BuildManifest.from_path")
@patch("run_build.TemporaryDirectory")
@patch("run_build.BuildIncremental")
def test_main_incremental(self, mock_build_incremental: MagicMock, mock_temp: MagicMock,
mock_build_manifest: MagicMock, *mocks: Any) -> None:
mock_temp.return_value.__enter__.return_value.name = tempfile.gettempdir()
mock_build_manifest.return_value = self.BUILD_MANIFEST
main()
self.assertEqual(mock_build_incremental.call_count, 1)
mock_build_incremental.return_value.commits_diff.assert_called()
mock_build_incremental.return_value.rebuild_plugins.assert_called()

@patch("argparse._sys.argv", ["run_build.py", INPUT_MANIFEST_PATH, "--incremental"])
@patch("run_build.BuildIncremental", return_value=MagicMock())
@patch("os.path.exists")
@patch("run_build.Builders.builder_from", return_value=MagicMock())
@patch("run_build.BuildRecorder", return_value=MagicMock())
@patch("run_build.TemporaryDirectory")
def test_build_incremental_no_prebuild_manifest(self, mock_temp: MagicMock, mock_recorder: MagicMock,
mock_builder: MagicMock, mock_path_exists: MagicMock,
*mocks: Any) -> None:
mock_temp.return_value.__enter__.return_value.name = tempfile.gettempdir()
mock_path_exists.return_value = False
try:
main()
self.assertRaises(FileNotFoundError)
except FileNotFoundError:
pass

@patch("argparse._sys.argv", ["run_build.py", INPUT_MANIFEST_PATH, "--incremental", "-p", "linux"])
@patch("run_build.BuildIncremental.commits_diff", return_value=MagicMock())
@patch("run_build.BuildIncremental.rebuild_plugins", return_value=MagicMock())
@patch("run_build.logging.info")
@patch("run_build.BuildOutputDir")
@patch("os.path.exists")
@patch("manifests.build_manifest.BuildManifest.from_path")
@patch("run_build.Builders.builder_from", return_value=MagicMock())
@patch("run_build.BuildRecorder", return_value=MagicMock())
@patch("run_build.TemporaryDirectory")
def test_build_incremental_with_prebuild_manifest(self, mock_temp: MagicMock, mock_recorder: MagicMock,
mock_builder: MagicMock, mock_build_manifest: MagicMock,
mock_path_exist: MagicMock, mock_build_output_dir: MagicMock,
mock_logging_info: MagicMock, mock_build_incremental: MagicMock,
*mocks: Any) -> None:
mock_temp.return_value.__enter__.return_value.name = tempfile.gettempdir()
mock_path_exist.return_value = True
mock_build_manifest.return_value = self.BUILD_MANIFEST
mock_build_incremental.return_value = ["common-utils", "opensearch-observability"]
main()
mock_build_manifest.assert_called_once()
mock_build_manifest.assert_called_with(os.path.join("tar", "builds", "opensearch", "manifest.yml"))
self.assertNotEqual(mock_builder.return_value.build.call_count, 0)
self.assertEqual(mock_builder.return_value.build.call_count, 2)
self.assertEqual(mock_builder.return_value.build.call_count, mock_builder.return_value.export_artifacts.call_count)

mock_logging_info.assert_has_calls([
call('Building common-utils'),
call('Building opensearch-observability'),
], any_order=True)

mock_recorder.assert_called_once()
mock_recorder.return_value.write_manifest.assert_called()

@patch("argparse._sys.argv", ["run_build.py", INPUT_MANIFEST_PATH, "--incremental", "-p", "linux", "--continue-on-error"])
@patch("run_build.BuildIncremental.commits_diff", return_value=MagicMock())
@patch("run_build.BuildIncremental.rebuild_plugins", return_value=MagicMock())
@patch("run_build.logging.error")
@patch("run_build.logging.info")
@patch("run_build.BuildOutputDir")
@patch("os.path.exists")
@patch("manifests.build_manifest.BuildManifest.from_path")
@patch("run_build.Builders.builder_from", return_value=MagicMock())
@patch("run_build.BuildRecorder", return_value=MagicMock())
@patch("run_build.TemporaryDirectory")
def test_build_incremental_continue_on_fail_core(self, mock_temp: MagicMock, mock_recorder: MagicMock,
mock_builder_from: MagicMock, mock_build_manifest: MagicMock,
mock_path_exist: MagicMock, mock_build_output_dir: MagicMock,
mock_logging_info: MagicMock, mock_logging_error: MagicMock,
mock_build_incremental: MagicMock, *mocks: Any) -> None:
mock_temp.return_value.__enter__.return_value.name = tempfile.gettempdir()
mock_path_exist.return_value = True
mock_build_manifest.return_value = self.BUILD_MANIFEST
mock_builder = MagicMock()
mock_builder.build.side_effect = Exception("Error building")
mock_builder_from.return_value = mock_builder
mock_build_incremental.return_value = ["common-utils", "opensearch-observability"]

with pytest.raises(Exception, match="Error building"):
main()

mock_logging_error.assert_called_with(f"Error building common-utils, retry with: run_build.py {self.INPUT_MANIFEST_PATH} --component common-utils")
mock_build_manifest.assert_called_once()
mock_build_manifest.assert_called_with(os.path.join("tar", "builds", "opensearch", "manifest.yml"))
self.assertNotEqual(mock_builder.build.call_count, 0)
self.assertEqual(mock_builder.build.call_count, 1)

mock_logging_info.assert_has_calls([
call('Building common-utils')
], any_order=True)

mock_recorder.assert_called_once()
mock_recorder.return_value.write_manifest.assert_not_called()

@patch("argparse._sys.argv", ["run_build.py", INPUT_MANIFEST_PATH, "--incremental", "-p", "linux", "--continue-on-error"])
@patch("run_build.BuildIncremental.commits_diff", return_value=MagicMock())
@patch("run_build.BuildIncremental.rebuild_plugins", return_value=MagicMock())
@patch("run_build.logging.error")
@patch("run_build.logging.info")
@patch("run_build.BuildOutputDir")
@patch("os.path.exists")
@patch("manifests.build_manifest.BuildManifest.from_path")
@patch("run_build.Builders.builder_from", return_value=MagicMock())
@patch("run_build.BuildRecorder", return_value=MagicMock())
@patch("run_build.TemporaryDirectory")
def test_build_incremental_continue_on_fail_plugin(self, mock_temp: MagicMock, mock_recorder: MagicMock,
mock_builder_from: MagicMock, mock_build_manifest: MagicMock,
mock_path_exist: MagicMock, mock_build_output_dir: MagicMock,
mock_logging_info: MagicMock, mock_logging_error: MagicMock,
mock_build_incremental: MagicMock, *mocks: Any) -> None:
mock_temp.return_value.__enter__.return_value.name = tempfile.gettempdir()
mock_path_exist.return_value = True
mock_build_manifest.return_value = self.BUILD_MANIFEST
mock_builder = MagicMock()
mock_builder.build.side_effect = Exception("Error build")
mock_builder_from.return_value = mock_builder
mock_build_incremental.return_value = ["ml-commons", "opensearch-observability"]

main()

mock_logging_error.assert_called_with("Failed plugins are ['ml-commons', 'opensearch-observability']")
mock_build_manifest.assert_called_once()
mock_build_manifest.assert_called_with(os.path.join("tar", "builds", "opensearch", "manifest.yml"))
self.assertNotEqual(mock_builder.build.call_count, 0)
self.assertEqual(mock_builder.build.call_count, 2)

mock_logging_info.assert_has_calls([
call('Building ml-commons'),
call('Building opensearch-observability')
], any_order=True)

mock_recorder.assert_called_once()
mock_recorder.return_value.write_manifest.assert_called()
1 change: 1 addition & 0 deletions tests/tests_build_workflow/test_build_incremental.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class TestBuildIncremental(unittest.TestCase):
os.path.join(os.path.dirname(__file__), "data", "opensearch-input-2.12.0.yml"))
BUILD_MANIFEST = BuildManifest.from_path(
os.path.join(os.path.dirname(__file__), "data", "opensearch-build-tar-2.12.0.yml"))
BUILD_MANIFEST_PATH = os.path.join(os.path.dirname(__file__), "data", "opensearch-build-tar-2.12.0.yml")
INPUT_MANIFEST_DASHBOARDS = InputManifest.from_path(
os.path.join(os.path.dirname(__file__), "data", "opensearch-dashboards-input-2.12.0.yml"))
BUILD_MANIFEST_DASHBOARDS = BuildManifest.from_path(
Expand Down
Loading
Loading