feat(pkg_size): Delete old code and round the diff

DataDog · Dec 16, 2024 · ba99a37 · ba99a37
1 parent fde3c5b
commit ba99a37
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 173 deletions.
diff --git a/.gitlab/.ci-linters.yml b/.gitlab/.ci-linters.yml
@@ -56,8 +56,6 @@ job-owners:
     - build_otel_agent_binary_arm64
     - build_otel_agent_binary_x64
     - cancel-prev-pipelines
-    - check_pkg_size-amd64-a7
-    - check_pkg_size-arm64-a7
     - close_failing_tests_stale_issues
     - compute_gitlab_ci_config
     - deploy_cluster_agent_cloudfoundry

diff --git a/.gitlab/JOBOWNERS b/.gitlab/JOBOWNERS
@@ -67,7 +67,7 @@ iot_agent_suse*                      @DataDog/agent-delivery
 dogstatsd_suse*                      @DataDog/agent-delivery
 agent_oci*                           @DataDog/agent-delivery
 installer_oci*                       @DataDog/agent-delivery
-new_check_pkg_size                   @DataDog/agent-delivery
+check_pkg_size                       @DataDog/agent-delivery
 
 # Testing package deploy
 deploy_deb_testing*                  @DataDog/agent-delivery

diff --git a/.gitlab/pkg_metrics/pkg_metrics.yml b/.gitlab/pkg_metrics/pkg_metrics.yml
@@ -76,118 +76,14 @@ send_pkg_size:
 
     - inv package.send-size --flavor "agent" --package-os "suse" --package-path $OMNIBUS_PACKAGE_DIR_SUSE/datadog-agent-7.*.aarch64.rpm --major-version "7" --git-ref "${CI_COMMIT_REF_SLUG}" --bucket-branch "${BUCKET_BRANCH}" --arch arm64
 
-.check_pkg_size:
-  stage: pkg_metrics
-  image: registry.ddbuild.io/ci/datadog-agent-buildimages/deb_x64$DATADOG_AGENT_BUILDIMAGES_SUFFIX:$DATADOG_AGENT_BUILDIMAGES
-  tags: ["arch:amd64"]
-  script:
-    - ls -l $OMNIBUS_PACKAGE_DIR
-    - if [[ "${ARCH}" == "amd64" ]]; then ls -l $OMNIBUS_PACKAGE_DIR_SUSE; fi
-
-    - export failures=0
-    - export last_stable=$(inv release.get-release-json-value "last_stable::${MAJOR_VERSION}" --no-worktree)
-    # Get stable packages from S3 buckets, send new package sizes & compare stable and new package sizes
-    # The loop assumes that all flavors start with "da", which is currently the case
-    # We want to run all package size comparisons before failing, so we set +e while doing the comparisons
-    # to get the error codes without exiting the shell.
-    - |
-      if [[ "${ARCH}" == "amd64" ]]; then ARCH_RPM_EXT="x86_64"; else ARCH_RPM_EXT="aarch64"; fi
-      for flavor in ${FLAVORS}; do
-
-          if [[ "${ARCH}" == "amd64" && "$flavor" != "datadog-heroku-agent" ]]; then
-            mkdir -p "/tmp/stable/${flavor}/suse"
-            curl -sSL "https://s3.amazonaws.com/yum.datadoghq.com/suse/stable/${MAJOR_VERSION}/${ARCH_RPM_EXT}/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm" -o "/tmp/stable/${flavor}/suse/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm"
-            set +e
-            inv package.compare-size --package-type "${flavor} suse rpm" --last-stable "${last_stable}" --threshold "${max_sizes[${flavor}]}" --new-package "$OMNIBUS_PACKAGE_DIR_SUSE/${flavor}-${MAJOR_VERSION}.*.${ARCH_RPM_EXT}.rpm" --stable-package "/tmp/stable/${flavor}/suse/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm"
-            failures=$((${failures}+$?))
-            set -e
-          fi
-
-          mkdir -p "/tmp/stable/${flavor}"
-
-          curl -sSL "https://s3.amazonaws.com/apt.datadoghq.com/pool/d/da/${flavor}_${last_stable}-1_${ARCH}.deb" -o "/tmp/stable/${flavor}/${flavor}_${last_stable}-1_${ARCH}.deb"
-
-          set +e
-          inv package.compare-size --package-type "${flavor} deb" --last-stable "${last_stable}" --threshold "${max_sizes[${flavor}]}" --new-package "$OMNIBUS_PACKAGE_DIR/${flavor}_${MAJOR_VERSION}*_${ARCH}.deb" --stable-package "/tmp/stable/${flavor}/${flavor}_${last_stable}-1_${ARCH}.deb"
-          failures=$((${failures}+$?))
-          set -e
-
-          if [[ "$flavor" != "datadog-heroku-agent" && ( "${ARCH}" == "amd64" || "$flavor" != "datadog-dogstatsd") ]]; then
-            # We don't build RPM packages for the heroku flavor
-            curl -sSL "https://s3.amazonaws.com/yum.datadoghq.com/stable/${MAJOR_VERSION}/${ARCH_RPM_EXT}/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm" -o "/tmp/stable/${flavor}/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm"
-            set +e
-            inv package.compare-size --package-type "${flavor} rpm" --last-stable "${last_stable}" --threshold "${max_sizes[${flavor}]}" --new-package "$OMNIBUS_PACKAGE_DIR/${flavor}-${MAJOR_VERSION}.*.${ARCH_RPM_EXT}.rpm" --stable-package "/tmp/stable/${flavor}/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm"
-            failures=$((${failures}+$?))
-            set -e
-          fi
-      done
-
-    # Make the job fail if at least one package is above threshold
-    - if [ "${failures}" -ne "0" ]; then false; fi
-
-check_pkg_size-amd64-a7:
-  extends: .check_pkg_size
-  rules:
-    - !reference [.except_mergequeue]
-    - when: on_success
-  needs:
-    - agent_deb-x64-a7
-    - iot_agent_deb-x64
-    - dogstatsd_deb-x64
-    - agent_heroku_deb-x64-a7
-    - agent_rpm-x64-a7
-    - iot_agent_rpm-x64
-    - dogstatsd_rpm-x64
-    - agent_suse-x64-a7
-    - dogstatsd_suse-x64
-    - iot_agent_suse-x64
-  variables:
-    MAJOR_VERSION: 7
-    FLAVORS: "datadog-agent datadog-iot-agent datadog-dogstatsd datadog-heroku-agent"
-    ARCH: "amd64"
-  before_script:
-    # FIXME: ["datadog-agent"]="140000000" and ["datadog-heroku-agent"]="140000000" should
-    # be replaced by "50000000"
-    # "70000000" is needed as of now because of multiple large additions in 7.45
-    - |
-      declare -Ar max_sizes=(
-          ["datadog-agent"]="140000000"
-          ["datadog-iot-agent"]="10000000"
-          ["datadog-dogstatsd"]="10000000"
-          ["datadog-heroku-agent"]="70000000"
-      )
-
-check_pkg_size-arm64-a7:
-  extends: .check_pkg_size
-  rules: !reference [.on_all_builds]
-  needs:
-    - agent_deb-arm64-a7
-    - iot_agent_deb-arm64
-    - dogstatsd_deb-arm64
-    - agent_rpm-arm64-a7
-    - iot_agent_rpm-arm64
-  variables:
-    MAJOR_VERSION: 7
-    FLAVORS: "datadog-agent datadog-iot-agent datadog-dogstatsd"
-    ARCH: "arm64"
-  before_script:
-    # FIXME: ["datadog-agent"]="140000000" should be replaced by "70000000"
-    # "140000000" is needed as of now because of multiple large additions in 7.45
-    - |
-      declare -Ar max_sizes=(
-          ["datadog-agent"]="140000000"
-          ["datadog-iot-agent"]="10000000"
-          ["datadog-dogstatsd"]="10000000"
-      )
 
-new_check_pkg_size:
+check_pkg_size:
   stage: pkg_metrics
   image: registry.ddbuild.io/ci/datadog-agent-buildimages/deb_x64$DATADOG_AGENT_BUILDIMAGES_SUFFIX:$DATADOG_AGENT_BUILDIMAGES
   tags: ["arch:amd64"]
   rules:
     - !reference [.except_mergequeue]
     - when: on_success
-  allow_failure: true
   needs:
     - agent_deb-x64-a7
     - iot_agent_deb-x64

diff --git a/tasks/libs/package/size.py b/tasks/libs/package/size.py
@@ -177,25 +177,11 @@ def compare(ctx, package_sizes, ancestor, pkg_size):
     return pkg_size
 
 
-def mb(value):
-    return f"{value / 1000000:.2f}MB"
-
-
 def _get_uncompressed_size(ctx, package, os_name):
     if os_name == 'deb':
-        return _get_deb_uncompressed_size(ctx, package)
+        return (
+            int(ctx.run(f'dpkg-deb --info {package} | grep Installed-Size | cut -d : -f 2 | xargs', hide=True).stdout)
+            * 1024
+        )
     else:
-        return _get_rpm_uncompressed_size(ctx, package)
-
-
-def _get_deb_uncompressed_size(ctx, package):
-    # the size returned by dpkg is a number of bytes divided by 1024
-    # so we multiply it back to get the same unit as RPM or stat
-    return (
-        int(ctx.run(f'dpkg-deb --info {package} | grep Installed-Size | cut -d : -f 2 | xargs', hide=True).stdout)
-        * 1024
-    )
-
-
-def _get_rpm_uncompressed_size(ctx, package):
-    return int(ctx.run(f'rpm -qip {package} | grep Size | cut -d : -f 2 | xargs', hide=True).stdout)
+        return int(ctx.run(f'rpm -qip {package} | grep Size | cut -d : -f 2 | xargs', hide=True).stdout)
diff --git a/tasks/libs/package/utils.py b/tasks/libs/package/utils.py
@@ -20,6 +20,7 @@ def __init__(self, arch, flavor, os_name, threshold):
         self.size = 0
         self.ancestor_size = 0
         self.diff = 0
+        self.mb_diff = 0
         self.threshold = threshold
         self.emoji = "✅"
 
@@ -48,14 +49,15 @@ def compare(self, size, ancestor_size):
         self.size = size
         self.ancestor_size = ancestor_size
         self.diff = self.size - self.ancestor_size
+        self.mb_diff = float(f"{self.diff / pow(10, 6):.2f}")
         if self.ko():
             self.emoji = "❌"
-        elif self.diff > 0:
+        elif self.mb_diff > 0:
             self.emoji = "⚠️"
 
     @staticmethod
     def mb(value):
-        return f"{value / 1000000:.2f}MB"
+        return f"{value / pow(10, 6):.2f}MB"
 
     def log(self):
         return f"{self.emoji} - {self.name} size {self.mb(self.size)}: {self.mb(self.diff)} diff[{self.diff}] with previous {self.mb(self.ancestor_size)} (max: {self.mb(self.threshold)})"

diff --git a/tasks/package.py b/tasks/package.py
@@ -8,15 +8,12 @@
 from tasks.libs.common.git import get_default_branch
 from tasks.libs.package.size import (
     PACKAGE_SIZE_TEMPLATE,
-    _get_deb_uncompressed_size,
-    _get_rpm_uncompressed_size,
     compare,
     compute_package_size_metrics,
 )
 from tasks.libs.package.utils import (
     PackageSize,
     display_message,
-    find_package,
     get_ancestor,
     list_packages,
     retrieve_package_sizes,
@@ -61,47 +58,6 @@ def check_size(ctx, filename: str = 'package_sizes.json', dry_run: bool = False)
             raise Exit(code=1)
 
 
-@task
-def compare_size(ctx, new_package, stable_package, package_type, last_stable, threshold):
-    mb = 1000000
-
-    if package_type.endswith('deb'):
-        new_package_size = _get_deb_uncompressed_size(ctx, find_package(new_package))
-        stable_package_size = _get_deb_uncompressed_size(ctx, find_package(stable_package))
-    else:
-        new_package_size = _get_rpm_uncompressed_size(ctx, find_package(new_package))
-        stable_package_size = _get_rpm_uncompressed_size(ctx, find_package(stable_package))
-
-    threshold = int(threshold)
-
-    diff = new_package_size - stable_package_size
-
-    # For printing purposes
-    new_package_size_mb = new_package_size / mb
-    stable_package_size_mb = stable_package_size / mb
-    threshold_mb = threshold / mb
-    diff_mb = diff / mb
-
-    if diff > threshold:
-        print(
-            color_message(
-                f"""{package_type} size increase is too large:
-  New package size is {new_package_size_mb:.2f}MB
-  Stable package ({last_stable}) size is {stable_package_size_mb:.2f}MB
-  Diff is {diff_mb:.2f}MB > {threshold_mb:.2f}MB (max allowed diff)""",
-                "red",
-            )
-        )
-        raise Exit(code=1)
-
-    print(
-        f"""{package_type} size increase is OK:
-  New package size is {new_package_size_mb:.2f}MB
-  Stable package ({last_stable}) size is {stable_package_size_mb:.2f}MB
-  Diff is {diff_mb:.2f}MB (max allowed diff: {threshold_mb:.2f}MB)"""
-    )
-
-
 @task
 def send_size(
     ctx,

diff --git a/tasks/unit_tests/package_lib_tests.py b/tasks/unit_tests/package_lib_tests.py
@@ -238,6 +238,26 @@ def test_on_branch_warning(self, mock_print):
             f"⚠️ - {flavor}-{arch}-{os_name} size 69.00MB: 1.00MB diff[1000000] with previous 68.00MB (max: 70.00MB)"
         )
 
+    @patch.dict(
+        'os.environ',
+        {'OMNIBUS_PACKAGE_DIR_SUSE': 'tasks/unit_tests/testdata/packages', 'CI_COMMIT_REF_NAME': 'pikachu'},
+    )
+    @patch('builtins.print')
+    def test_on_branch_ok_small_diff(self, mock_print):
+        flavor, arch, os_name = 'datadog-agent', 'aarch64', 'suse'
+        s = PackageSize(arch, flavor, os_name, 70000000)
+        c = MockContext(
+            run={
+                'git merge-base HEAD origin/main': Result('25'),
+                f"rpm -qip {self.pkg_root}/{flavor}-7.{arch}.rpm | grep Size | cut -d : -f 2 | xargs": Result(68004999),
+            }
+        )
+        res = compare(c, self.package_sizes, '25', s)
+        self.assertEqual(res.markdown(), "|datadog-agent-aarch64-suse|0.00MB|✅|68.00MB|68.00MB|70.00MB|")
+        mock_print.assert_called_with(
+            f"✅ - {flavor}-{arch}-{os_name} size 68.00MB: 0.00MB diff[4999] with previous 68.00MB (max: 70.00MB)"
+        )
+
     @patch.dict(
         'os.environ', {'OMNIBUS_PACKAGE_DIR': 'tasks/unit_tests/testdata/packages', 'CI_COMMIT_REF_NAME': 'pikachu'}
     )