Skip to content

Commit

Permalink
Refractoring the incremental building process
Browse files Browse the repository at this point in the history
Signed-off-by: Zelin Hao <[email protected]>
  • Loading branch information
zelinh committed Jan 3, 2024
1 parent 20b74d4 commit 441c604
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 206 deletions.
59 changes: 0 additions & 59 deletions src/build_workflow/build_incremental.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,8 @@
import os
from typing import List

from build_workflow.build_args import BuildArgs
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.temporary_directory import TemporaryDirectory


class BuildIncremental:
Expand Down Expand Up @@ -68,56 +62,3 @@ def rebuild_plugins(self, changed_plugins: List, input_manifest: InputManifest)

logging.info(f"Rebuilding list is {rebuild_list}")
return rebuild_list

def build_incremental(self, args: BuildArgs, input_manifest: InputManifest, components: List) -> None:
build_manifest_path = os.path.join(self.distribution, "builds", input_manifest.build.filename, "manifest.yml")
if not os.path.exists(build_manifest_path):
logging.error("Previous build manifest is not exists. Throw error.")

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

build_manifest_data = BuildManifest.from_path(build_manifest_path).__to_dict__()

output_dir = BuildOutputDir(input_manifest.build.filename, args.distribution).dir
failed_plugins = []

with TemporaryDirectory(keep=args.keep, chdir=True) as work_dir:
logging.info(f"Building in {work_dir.name}")
target = BuildTarget(
name=input_manifest.build.name,
version=input_manifest.build.version,
qualifier=input_manifest.build.qualifier,
patches=input_manifest.build.patches,
snapshot=args.snapshot if args.snapshot is not None else input_manifest.build.snapshot,
output_dir=output_dir,
distribution=args.distribution,
platform=args.platform or input_manifest.build.platform,
architecture=args.architecture or input_manifest.build.architecture,
)

build_recorder_incremental = BuildRecorder(target, build_manifest_data)

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

for component in input_manifest.components.select(focus=components, platform=target.platform):
logging.info(f"Rebuilding {component.name}")

builder = Builders.builder_from(component, target)
try:
builder.checkout(work_dir.name)
builder.build(build_recorder_incremental)
builder.export_artifacts(build_recorder_incremental)
logging.info(f"Successfully built {component.name}")
except:
logging.error(f"Error incremental building {component.name}.")
if args.continue_on_error and component.name not in ['OpenSearch', 'job-scheduler', 'common-utils', 'OpenSearch-Dashboards']:
failed_plugins.append(component.name)
continue
else:
raise

build_recorder_incremental.write_manifest()

if len(failed_plugins) > 0:
logging.error(f"Failed plugins are {failed_plugins}")
logging.info("Done.")
17 changes: 13 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_data = {}
components = args.components
failed_plugins = []

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

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_data = BuildManifest.from_path(build_manifest_path).__to_dict__()

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

build_recorder = BuildRecorder(target)
build_recorder = BuildRecorder(target, build_manifest_data) 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):
logging.info(f"Building {component.name}")

builder = Builders.builder_from(component, target)
Expand Down
153 changes: 150 additions & 3 deletions 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 @@ -231,10 +244,144 @@ def test_failed_plugins_default(self, mock_logging_error: Mock, mock_temp: Mock,
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("run_build.Builders.builder_from", return_value=MagicMock())
@patch("run_build.BuildRecorder", return_value=MagicMock())
@patch("run_build.TemporaryDirectory")
@patch("run_build.BuildIncremental")
def test_main_incremental(self, mock_build_incremental: Mock, *mocks: Any) -> None:
def test_main_incremental(self, mock_build_incremental: MagicMock, mock_temp: MagicMock, *mocks: Any) -> None:
mock_temp.return_value.__enter__.return_value.name = tempfile.gettempdir()
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()
mock_build_incremental.return_value.build_incremental.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("paths.build_output_dir")
@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("paths.build_output_dir")
@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("paths.build_output_dir")
@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()
Loading

0 comments on commit 441c604

Please sign in to comment.