#1517 Integration test for the Cancel And Resume
Merged 4 years ago by csomh. Opened 4 years ago by mulaieva.
mulaieva/fm-orchestrator canc_and_res_test  into  master

@@ -48,9 +48,6 @@ 

          args = [

              "--branch",

              repo_conf["branch"],

-             "--single-branch",

-             "--depth",

-             "1",

              url,

              tempdir,

          ]

@@ -22,7 +22,7 @@ 

      module: testmodule

      # Branch which is going to be built for this test.

      branch: scratch-build-branch

- failed_build:

+   failed_build:

      module: testmodule

      branch: failed-build-branch

      # Batch considered by this test.
@@ -40,3 +40,6 @@ 

      buildorder: [{"module-build-macros"}, {"attr"}, {"acl"}]

      # True if buildrequire a Platform stream representing a GA RHEL release

      platform_is_ga: true

+   resume_cancelled_build:

+     module: testmodule

+     branch: cancel-build-branch

@@ -6,7 +6,7 @@ 

  

  def test_failed_build(test_env, repo, koji):

      """

-     Run a scratch build with "rebuild_strategy=all".

+     Run the build with "rebuild_strategy=all".

  

      Check that:

        * Check that the module build eventually fails
@@ -26,8 +26,8 @@ 

      batch = test_env["testdata"]["failed_build"]["batch"]

      failing_components = test_env["testdata"]["failed_build"]["failing_components"]

      canceled_components = test_env["testdata"]["failed_build"]["canceled_components"]

-     assert sorted(failing_components) == sorted(build.components(state="FAILED", batch=batch))

+     assert sorted(failing_components) == sorted(build.component_names(state="FAILED", batch=batch))

      assert sorted(canceled_components) == sorted(

-         build.components(state="COMPLETE", batch=batch)

-         + build.components(state="CANCELED", batch=batch)

+         build.component_names(state="COMPLETE", batch=batch)

+         + build.component_names(state="CANCELED", batch=batch)

      )

@@ -25,7 +25,7 @@ 

          "rebuild_strategy=all",

          reuse=test_env["testdata"]["normal_build"].get("build_id"),

      )

-     assert sorted(build.components()) == sorted(repo.components + ["module-build-macros"])

+     assert sorted(build.component_names()) == sorted(repo.components + ["module-build-macros"])

  

      expected_buildorder = test_env["testdata"]["normal_build"]["buildorder"]

      expected_buildorder = [set(batch) for batch in expected_buildorder]

@@ -0,0 +1,33 @@ 

+ # -*- coding: utf-8 -*-

+ # SPDX-License-Identifier: MIT

+ 

+ import utils

+ import time

+ 

+ 

+ def test_resume_cancelled_build(test_env, repo, koji):

+     """

+     Run the  build with "rebuild_strategy=all".

+     Wait until the module-build-macros build is submitted to Koji.

+     Cancel module build.

+     Resume the module with "rhpkg-stage module-build -w".

+ 

+     Check that:

+       * Check that the testmodule had actually been cancelled

+       * Check that the testmodule build succeeded

+ 

+     """

+     build = utils.Build(test_env["packaging_utility"], test_env["mbs_api"])

+     repo.bump()

+     build.run(

+         "--optional",

+         "rebuild_strategy=all",

+     )

+     build.wait_for_koji_task_id(package="module-build-macros", batch=1)

+     build.cancel()

+     # Behave like a human: restarting the build too quickly would lead to an error.

+     time.sleep(10)

+     build.run("--watch")

+ 

+     assert build.state_name == "ready"

+     assert build.was_cancelled()

@@ -24,7 +24,7 @@ 

      )

  

      assert build.state_name == "done"

-     assert sorted(build.components(state="COMPLETE")) == sorted(

+     assert sorted(build.component_names(state="COMPLETE")) == sorted(

          repo.components + ["module-build-macros"]

      )

  

file modified
+96 -5
@@ -2,12 +2,13 @@ 

  # SPDX-License-Identifier: MIT

  

  import re

+ import time

  

  from kobo import rpmlib

  import koji

  import yaml

  import requests

- from sh import Command

+ from sh import Command, git

  

  

  class Koji:
@@ -99,6 +100,16 @@ 

          elif self._version == 2:

              return self._modulemd["data"]["dependencies"][0]["buildrequires"].get("platform")

  

+     def bump(self):

+         """Create a "bump" commit"""

+         args = [

+             "--allow-empty",

+             "-m",

+             "Bump"

+         ]

+         git("commit", *args)

+         git("push")

+ 

  

  class Build:

      """Wrapper class to work with module builds
@@ -108,6 +119,7 @@ 

      :attribute string _mbs_api: URL of the MBS API (including trailing '/')

      :attribute string _url: URL of this MBS module build

      :attribute string _data: Module build data cache for this build fetched from MBS

+     :attribute string _module_build_data: Verbose module build data cache for this build

      """

  

      def __init__(self, packaging_utility, mbs_api):
@@ -116,6 +128,7 @@ 

          self._data = None

          self._component_data = None

          self._build_id = None

+         self._module_build_data = None

  

      def run(self, *args, reuse=None):

          """Run a module build
@@ -135,6 +148,16 @@ 

              self._build_id = int(re.search(self._mbs_api + r"module-builds/(\d+)", stdout).group(1))

          return self._build_id

  

+     def cancel(self):

+         """Cancel the module build

+ 

+         :return: Standard output of the "module-build-cancel <build id=""> command

+         :rtype: str

+         """

+         stdout = self._packaging_utility("module-build-cancel", self._build_id).stdout.decode(

+             "utf-8")

+         return stdout

+ 

      @property

      def data(self):

          """Module build data cache for this build fetched from MBS"""
@@ -158,25 +181,56 @@ 

          return self._component_data

  

      @property

+     def module_build_data(self):

+         """Verbose module build

+ 

+         :return: Dictionary of the verbose module build parameters

+         :rtype: dict

+         """

+         if self._build_id:

+             params = {

+                 "verbose": True,

+             }

+             r = requests.get(f"{self._mbs_api}module-builds/{self._build_id}", params=params)

+             r.raise_for_status()

+             self._module_build_data = r.json()

+         return self._module_build_data

+ 

+     @property

      def state_name(self):

          """Name of the state of this module build"""

          return self.data["state_name"]

  

-     def components(self, state="COMPLETE", batch=None):

-         """Components of this module build which are in some state and in some batch

+     def components(self, state=None, batch=None, package=None):

+         """Components of this module build, optionally filtered based on properties

  

          :param string state: Koji build state the components should be in

          :param int batch: the number of the batch the components should be in

+         :param string package: name of the component (package)

          :return: List of filtered components

-         :rtype: list of strings

+         :rtype: list of dict

          """

          filtered = self.component_data["items"]

          if batch is not None:

              filtered = filter(lambda x: x["batch"] == batch, filtered)

          if state is not None:

              filtered = filter(lambda x: x["state_name"] == state, filtered)

+         if package is not None:

+             filtered = filter(lambda x: x["package"] == package, filtered)

  

-         return [item["package"] for item in filtered]

+         return list(filtered)

+ 

+     def component_names(self, state=None, batch=None, package=None):

+         """Component names of this module build, optionally filtered based on properties

+ 

+         :param string state: Koji build state the components should be in

+         :param int batch: the number of the batch the components should be in

+         :param string: name of component (package):

+         :return: List of components packages

+         :rtype: list of strings

+         """

+         components = self.components(state, batch, package)

+         return [item["package"] for item in components]

  

      def batches(self):

          """
@@ -195,6 +249,28 @@ 

  

          return batches

  

+     def wait_for_koji_task_id(self, package, batch, timeout=60, sleep=10):

+         """Wait until the component is submitted to Koji (has a task_id)

+ 

+         :param string: name of component (package)

+         :param int batch: the number of the batch the components should be in

+         :param int timeout: time in seconds

+         :param int sleep: time in seconds

+         """

+         start = time.time()

+         while time.time() - start <= timeout:

+             # Clear cached data

+             self._component_data = None

+             components = self.components(package=package, batch=batch)

+             # Wait until the right component appears and has a task_id

+             if components and components[0]["task_id"]:

+                 return components[0]["task_id"]

+             time.sleep(sleep)

+ 

+         raise RuntimeError(

+             f'Koji task for "{package}" did not start in {timeout} seconds'

+         )

+ 

      def nvr(self, name_suffix=""):

          """NVR dictionary of this module build

  
@@ -207,3 +283,18 @@ 

              "version": self.data["stream"].replace("-", "_"),

              "release": f'{self.data["version"]}.{self.data["context"]}',

          }

+ 

+     def was_cancelled(self):

+         """Checking in the status trace if module was canceled

+ 

+         :return: Whether exists required status

+         :rtype: bool

+         """

+         for item in self.module_build_data["state_trace"]:

+             if (

+                     item["reason"] is not None

+                     and "Canceled" in item["reason"]

+                     and item["state_name"] == "failed"

+             ):

+                 return True

+         return False

file modified
+1 -1
@@ -79,6 +79,6 @@ 

      sh

  # Set this to /etc/pki/tls/certs/ca-bundle.crt, for example,

  # if the instance tested has a self-signed certificate.

- passenv = REQUESTS_CA_BUNDLE MBS_TEST_CONFIG MBS_TEST_WORKERS

+ passenv = REQUESTS_CA_BUNDLE MBS_TEST_CONFIG MBS_TEST_WORKERS HOME

  commands =

      pytest -vv --confcutdir=tests/integration -n {env:MBS_TEST_WORKERS:0} {posargs:tests/integration}

Implement the integration test for the Cancel and Resume scenario.

Build #612 failed (commit: 1394ff9167f6ceb7b347add09e9a0d874a1c1e8b).
Rebase or make new commits to rebuild.

rebased onto 095fa49daf7d78d1877702189594178e46e029df

4 years ago

Build #616 failed (commit: 095fa49daf7d78d1877702189594178e46e029df).
Rebase or make new commits to rebuild.

rebased onto 149287e5cd6af56e93340ddce5c4badc39b3ced7

4 years ago

Build #618 failed (commit: 149287e5cd6af56e93340ddce5c4badc39b3ced7).
Rebase or make new commits to rebuild.

rebased onto 647bdc0b7310c0698c8feddd0bf5e45f7270cca0

4 years ago

Build #621 failed (commit: 647bdc0b7310c0698c8feddd0bf5e45f7270cca0).
Rebase or make new commits to rebuild.

rebased onto f3123ebc27e6979ead1238c52db620aede5faae7

4 years ago

rebased onto 6be9f79fe312cff0ce84d747394eed4814c76c89

4 years ago

Build #622 failed (commit: f3123ebc27e6979ead1238c52db620aede5faae7).
Rebase or make new commits to rebuild.

rebased onto 8318d3cbf2bd77de1248ae2fba023143c878140a

4 years ago

Build #625 failed (commit: 8318d3cbf2bd77de1248ae2fba023143c878140a).
Rebase or make new commits to rebuild.

Let's use requests parameters for the verbose=true part!

I find this to be too generic. How about changing to module_build_data? Which would be similar to component_data.

Nit: "Cancel the module build"

Specify what is returned, and what type it is.

reuse is None by default. Non need to specify it here.

Instead of writing a new property, components could be extended to get an optional package property.

You could improve the readability of the test by making this code to become a property of the Build class.

Then you could call here:

assert build.was_cancelled

Is this sleep still required?

It is in the scenario description
"* Cancel module build ...`
* Wait 10 seconds
- This is more or less arbitrary, but it’s intended to simulate a user having cancelled the build by mistake and they want to resume it."

Instead of writing a new property, components could be extended to get an optional package property.

I'd rather leave it like that. Because in components we get the list of components. But in in this property I get the particular component in the form of the dictionary with all parameters I need. Or I can change this property for example like specific_component (self, name = "module-build-macros"). And we can use it not only for module-build-macros but for any required component.

You could improve the readability of the test by making this code to become a property of the Build class.
Then you could call here:
assert build.was_cancelled

And now I'm thinking about checking not only reason and state name, but also the time... What do you think about it?

rebased onto 788e309bbd7b48bda2302e4ca78e8396ac9ebaf2

4 years ago

rebased onto d6b93cb7ecfbd2282f907c583eeb3d18b1091b98

4 years ago

cancel_resume is not a very descriptive name. How about something like resume_cancelled_build ?

Why is this needed? Can we add a comment explaining?

Should this be if self._module_build_data is None and self._build_id ? Usually, that's the pattern used for only caching the data. Unless, caching is not what you're looking for. In that case, I'd remove the self._module_build_data attribute completely, and just have this method either return None, or return r.json().

Similar comment to the previous one. Just return list_macros[0] directly. There's no need to have an object attribute to store this.

Personally, I find list comprehensions much easier to read than filter + lambda. For instance:

list_macros = [
  item for item in self.component_data["items"] 
  if item["package"] == "module_build_macros"
]

In this case though, you may just want to use a for loop and stop the iteration as soon as the first match is found (since that's what you care about anyways):

for item in self.component_data["items"]:
  if item["package"] == "module-build-macros":
    return item

This should be named something like wait_for_macros_state_trace to better provide intent.

We probably want to have some sort of timeout here.

Why are we assigning a value to state_name here? Should this be a conditional instead?

start = time.time()
while time.time() - start <= timeout:
  for item in self.module_build_macros["state_trace"]:
    if item["state_name"] == state:
      return
  time.sleep(10)

raise RuntimeError('Build did not reach {} state in {} seconds'.format(state, timeout))

For loop would make this easier to read.

rebased onto f6bf10cc186d98eee1a0bf07243f45ab31ab1128

4 years ago

Build #641 failed (commit: f6bf10cc186d98eee1a0bf07243f45ab31ab1128).
Rebase or make new commits to rebuild.

rebased onto 423df34b786487ba5ddfab3e0e12e7dd3632409a

4 years ago

rebased onto d3a2395585ed5dc23861f1863c3caadc2abe9e6f

4 years ago

@csomh , please take a look one more time

Should be indented with 2 spaces.

Should be indented with 2 spaces.

Please update this to be the actual description of the scenario.

Please move this after the import re line above. According to the Contribution Guide:

imports should be ordered by standard library, third-party, then local.

This attribute needs documentation in the doc-string of the Build class.

This can be removed. It's not used anywhere.

rebased onto bf0982be39e7400db21300f1ef065e893081889c

4 years ago

Spelling: `Standard output of the "module-build-cancel <build id> command."

Brew -> Koji, b/c Koji is the upstream name of the tool.

__module_build_data -> _module_build_data

Spelling: Standard output of the "module-build-cancel <build id>" command

Document return value and type.

:param string package: name of the component (package)

Rephrase: "Components of this module build, optionally filtered based on properties."

Rephrase: "Component names of this module build, optionally filtered based on properties."

Please, document the arguments.

Rephrase: "Wait until the component is submitted to Koji (has a task_id)."

Explanation: It's more concrete to have documentation phrased in present tense (is/are) then in future (will) or past (was/were).

rebased onto 4b62d16ebf52d49d4dfa872eeca19e3314acf6b7

4 years ago

rebased onto 8776a23e27c47fe34609b285ac399cf341d88bc5

4 years ago

rebased onto 4c8a92c

4 years ago

Pull-Request has been merged by csomh

4 years ago
Metadata
Flags
jenkins
success (100%)
Build #1353 successful (commit: 4c8a92cb)
4 years ago
c3i-jenkins
success (100%)
Build #651 successful (commit: 8776a23e)
4 years ago
jenkins
success (100%)
Build #1352 successful (commit: 8776a23e)
4 years ago
jenkins
success (100%)
Build #1348 successful (commit: 4b62d16e)
4 years ago
c3i-jenkins
success (100%)
Build #647 successful (commit: 4b62d16e)
4 years ago
jenkins
success (100%)
Build #1345 successful (commit: bf0982be)
4 years ago
c3i-jenkins
success (100%)
Build #644 successful (commit: bf0982be)
4 years ago
jenkins
success (100%)
Build #1344 successful (commit: d3a23955)
4 years ago
c3i-jenkins
success (100%)
Build #643 successful (commit: d3a23955)
4 years ago
jenkins
success (100%)
Build #1343 successful (commit: 423df34b)
4 years ago
c3i-jenkins
success (100%)
Build #642 successful (commit: 423df34b)
4 years ago
c3i-jenkins
failure
Build #641 failed (commit: f6bf10cc)
4 years ago
c3i-jenkins
success (100%)
Build #638 successful (commit: d6b93cb7)
4 years ago
jenkins
success (100%)
Build #1339 successful (commit: d6b93cb7)
4 years ago
jenkins
success (100%)
Build #1338 successful (commit: 788e309b)
4 years ago
c3i-jenkins
success (100%)
Build #637 successful (commit: 788e309b)
4 years ago
jenkins
success (100%)
Build #1327 successful (commit: 8318d3cb)
4 years ago
c3i-jenkins
success (100%)
Build #632 successful (commit: 8318d3cb)
4 years ago
jenkins
failure
Build #1325 failed (commit: 6be9f79f)
4 years ago
c3i-jenkins
pending
Build #623 in progress (commit: 6be9f79f)
4 years ago
c3i-jenkins
failure
Build #622 failed (commit: f3123ebc)
4 years ago
jenkins
success (100%)
Build #1324 successful (commit: f3123ebc)
4 years ago
jenkins
success (100%)
Build #1323 successful (commit: 647bdc0b)
4 years ago
c3i-jenkins
failure
Build #621 failed (commit: 647bdc0b)
4 years ago
c3i-jenkins
failure
Build #618 failed (commit: 149287e5)
4 years ago
jenkins
failure
Build #1320 failed (commit: 149287e5)
4 years ago
jenkins
success (100%)
Build #1317 successful (commit: 095fa49d)
4 years ago
c3i-jenkins
failure
Build #616 failed (commit: 095fa49d)
4 years ago
c3i-jenkins
failure
Build #612 failed (commit: 1394ff91)
4 years ago
jenkins
success (100%)
Build #1313 successful (commit: 1394ff91)
4 years ago