Issues/173 (#182)

jackiryan · web-flow · commit 7de117bc5e6b · 2026-06-04T14:54:14.000-07:00
* issues/173: changed default resource names to allow multiple deployments. BREAKING CHANGE, requires migrating using the new module inputs (see readme)

* issues/181: Added retry logic for Harmony jobs that fail with 5xx errors
diff --git a/.github/workflows/cicd-pipeline.yml b/.github/workflows/cicd-pipeline.yml
@@ -309,6 +309,13 @@ jobs:
           terraform init -backend=false -upgrade
           terraform validate -no-color
 
+      - name: Log in to the Container registry
+        uses: docker/login-action@v3
+        with:
+          registry: ${{ env.REGISTRY }}
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+
       - name: Deploy to venue
         id: terraform-deploy
         working-directory: examples/cumulus-tf
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,11 +9,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - [issues/150](https://github.com/podaac/bignbit/issues/150): Added support for mixed NRT & Standard collections with configurable regex.
 ### Changed
+- [issues/173](https://github.com/podaac/bignbit/issues/173): Changed resource names (e.g. ECR repo name) to include deployment level prefixes. This allows for deploying multiple copies of bignbit to a single AWS account/venue.
 - Removed MD5 checksum hash computation to optimize performance of `handle_big_result` lambda.
 ### Deprecated
 ### Removed
 ### Fixed
 - [issues/83](https://github.com/podaac/bignbit/issues/83): Use "_" instead of ":" in CNM filenames for Mac and Linux compatibility
+- [issues/181](https://github.com/podaac/bignbit/issues/181): Auto-retry logic for Harmony jobs that fail with transient server errors (5xx errors)
 ### Security
 - [issues/151](https://github.com/podaac/bignbit/issues/151): Added ExpectedBucketOwner parameter when making S3 requests.
 
diff --git a/README.md b/README.md
@@ -115,6 +115,8 @@ This module uses the following input variables:
 | app_name                             | string       |                                                                                                                                  | "bignbit"                                                           |
 | default_tags                         | map(string)  |                                                                                                                                  | {}                                                                  |
 | lambda_container_image_uri           | string       |                                                                                                                                  | ""                                                                  |
+| force_delete_ecr                     | bool         | Force delete the ECR repository and all its images on destroy. Set to true when migrating resource names to avoid manual cleanup. | false                                                               |
+| force_destroy_staging_bucket         | bool         | Force destroy the staging S3 bucket and all its contents on destroy. Set to true when migrating resource names to avoid manual cleanup. | false                                                          |
 | harmony_job_status_interval_seconds  | number       | Interval in seconds for checking Harmony job status                                                                              | 20                                                                  |
 | harmony_job_status_max_attempts      | number       | Maximum number of attempts to check Harmony job status                                                                           | 15                                                                  |
 | harmony_job_status_backoff_rate      | number       | Backoff rate for Harmony job status checks                                                                                       | 1.0                                                                 |
diff --git a/bignbit/get_harmony_job_status.py b/bignbit/get_harmony_job_status.py
@@ -2,6 +2,7 @@
 import logging
 import os
 
+import requests
 from cumulus_logger import CumulusLogger
 from cumulus_process import Process
 from harmony import LinkType
@@ -12,6 +13,13 @@
 CUMULUS_LOGGER = CumulusLogger('get_harmony_job_status')
 
 
+class HarmonyTransientError(Exception):
+    """Exception raised when the Harmony API returns a transient 5xx error"""
+
+    def __init__(self, message):
+        super().__init__(message)
+
+
 class HarmonyJobIncompleteError(Exception):
     """Exception raised when a harmony job is not complete"""
 
@@ -98,12 +106,26 @@ def check_harmony_job(
     """
 
     harmony_client = utils.get_harmony_client(cmr_env)
-    job_status = harmony_client.status(harmony_job_id)
+    try:
+        job_status = harmony_client.status(harmony_job_id)
+    except requests.exceptions.HTTPError as exc:
+        if exc.response is not None and exc.response.status_code >= 500:
+            raise HarmonyTransientError(
+                f'Harmony API returned a transient error checking status of job {harmony_job_id}: {exc}'
+            ) from exc
+        raise
 
     # For a successful job, return the status; for all other states, raise an exception.
     if job_status.get('status') == 'successful':
         # Check that the harmony job returned data to confirm that the job was successful
-        result_urls = list(harmony_client.result_urls(harmony_job_id, link_type=LinkType.s3))
+        try:
+            result_urls = list(harmony_client.result_urls(harmony_job_id, link_type=LinkType.s3))
+        except requests.exceptions.HTTPError as exc:
+            if exc.response is not None and exc.response.status_code >= 500:
+                raise HarmonyTransientError(
+                    f'Harmony API returned a transient error fetching result URLs for job {harmony_job_id}: {exc}'
+                ) from exc
+            raise
         if not result_urls:
             error_msg = (
                 f'Harmony job {harmony_job_id} completed successfully but returned no data for {variable} and {crs}'
diff --git a/examples/cumulus-tf/tfvars/sit.tfvars b/examples/cumulus-tf/tfvars/sit.tfvars
@@ -6,3 +6,6 @@ prefix = "podaac-sit-svc"
 gibs_region="mocked"
 gibs_queue_name="mocked"
 gibs_account_id="mocked"
+
+force_delete_ecr             = false
+force_destroy_staging_bucket = false
diff --git a/terraform/lambda_functions.tf b/terraform/lambda_functions.tf
@@ -3,7 +3,7 @@ data "aws_ecr_authorization_token" "token" {}
 locals {
   lambda_container_image_uri_split = split("/", var.lambda_container_image_uri)
   ecr_image_name_and_tag = split(":", element(local.lambda_container_image_uri_split, length(local.lambda_container_image_uri_split) - 1))
-  ecr_image_name = "${local.environment}-${element(local.ecr_image_name_and_tag, 0)}"
+  ecr_image_name = "${local.aws_resources_name}-${element(local.ecr_image_name_and_tag, 0)}"
   ecr_image_tag = element(local.ecr_image_name_and_tag, 1)
 
   # Truncate all function names to max 64 characters for AWS Lambda
@@ -21,7 +21,8 @@ locals {
 
 
 resource aws_ecr_repository "lambda-image-repo" {
-  name = local.ecr_image_name
+  name         = local.ecr_image_name
+  force_delete = var.force_delete_ecr
 }
 
 
diff --git a/terraform/main.tf b/terraform/main.tf
@@ -21,7 +21,8 @@ locals {
 
   account_id = data.aws_caller_identity.current.account_id
 
-  aws_resources_name = terraform.workspace == "default" ? "svc-${var.app_name}-${var.prefix}" : "svc-${var.app_name}-${var.prefix}-${terraform.workspace}"
+  aws_resources_name = terraform.workspace == "default" ? "${var.prefix}-${var.app_name}" : "${var.prefix}-${var.app_name}-${terraform.workspace}"
+  sns_topic_name = terraform.workspace == "default" ? "${var.app_name}-${var.prefix}" : "${var.app_name}-${var.prefix}-${terraform.workspace}"
 
   default_tags = length(var.default_tags) == 0 ? {
     team : "TVA",
diff --git a/terraform/s3.tf b/terraform/s3.tf
@@ -5,8 +5,9 @@ locals {
 }
 
 resource "aws_s3_bucket" "bignbit_staging_bucket" {
-  count  = local.create_bucket ? 1 : 0
-  bucket = "${local.aws_resources_name}-staging"
+  count         = local.create_bucket ? 1 : 0
+  bucket        = "${local.aws_resources_name}-staging"
+  force_destroy = var.force_destroy_staging_bucket
 
   lifecycle {
     ignore_changes = [
diff --git a/terraform/sqs_sns.tf b/terraform/sqs_sns.tf
@@ -21,7 +21,7 @@ resource "aws_sns_topic_policy" "default" {
 }
 
 resource "aws_sns_topic" "gibs_response_topic" {
-  name   = "${local.aws_resources_name}-gibs-response-topic"
+  name   = "svc-${local.sns_topic_name}-gibs-response-topic"
   lifecycle {
     # GIBS publishes to this topic, so we want to avoid destroying it unless coordinating the change with GIBS
     prevent_destroy = true
diff --git a/terraform/state_machine_definition.tpl b/terraform/state_machine_definition.tpl
@@ -338,6 +338,14 @@
                         "BackoffRate":${HarmonyJobStatusBackoffRate},
                         "MaxDelaySeconds":${HarmonyJobStatusMaxDelaySeconds}
                       },
+                      {
+                        "ErrorEquals":[
+                          "HarmonyTransientError"
+                        ],
+                        "IntervalSeconds":2,
+                        "MaxAttempts":6,
+                        "BackoffRate":2
+                      },
                       {
                         "ErrorEquals":[
                           "Lambda.ServiceException",
diff --git a/terraform/variables.tf b/terraform/variables.tf
@@ -112,6 +112,18 @@ variable "lambda_container_image_uri" {
   default = ""
 }
 
+variable "force_delete_ecr" {
+  type        = bool
+  description = "Force delete the ECR repository and all its images on destroy. Set to true when migrating resource names to avoid manual cleanup."
+  default     = false
+}
+
+variable "force_destroy_staging_bucket" {
+  type        = bool
+  description = "Force destroy the staging S3 bucket and all its contents on destroy. Set to true when migrating resource names to avoid manual cleanup."
+  default     = false
+}
+
 variable "harmony_job_status_interval_seconds" {
     type = number
     description = "Interval in seconds for checking Harmony job status"
diff --git a/tests/test_get_harmony_job_status.py b/tests/test_get_harmony_job_status.py
@@ -1,9 +1,16 @@
 """Unit tests for get_harmony_job_status module"""
+from unittest.mock import MagicMock, patch
+
 import pytest
+import requests
 from moto import mock_s3
 
 import bignbit.utils
-from bignbit.get_harmony_job_status import check_harmony_job, HarmonyJobNoDataError
+from bignbit.get_harmony_job_status import (
+    check_harmony_job,
+    HarmonyJobNoDataError,
+    HarmonyTransientError,
+)
 
 
 @pytest.mark.vcr
@@ -21,4 +28,75 @@ def test_process_results_no_data():
 
     assert 'no data' in str(exc_info.value).lower()
     assert 'test_variable' in str(exc_info.value)
-    assert 'EPSG:4326' in str(exc_info.value)
+    assert 'EPSG:4326' in str(exc_info.value)
+
+
+def _make_http_error(status_code):
+    """Build a requests.exceptions.HTTPError with a mocked response."""
+    response = MagicMock()
+    response.status_code = status_code
+    error = requests.exceptions.HTTPError(response=response)
+    return error
+
+
+@patch('bignbit.get_harmony_job_status.utils.get_harmony_client')
+def test_check_harmony_job_raises_transient_error_on_500(mock_get_client):
+    """Test that HarmonyTransientError is raised when Harmony status returns a 500."""
+    bignbit.utils.ED_USER = 'test'
+    bignbit.utils.ED_PASS = 'test'
+
+    mock_client = MagicMock()
+    mock_client.status.side_effect = _make_http_error(500)
+    mock_get_client.return_value = mock_client
+
+    with pytest.raises(HarmonyTransientError) as exc_info:
+        check_harmony_job('test-job-id', 'uat', 'test_variable', 'EPSG:4326')
+
+    assert 'transient error' in str(exc_info.value).lower()
+    assert 'test-job-id' in str(exc_info.value)
+
+
+@patch('bignbit.get_harmony_job_status.utils.get_harmony_client')
+def test_check_harmony_job_raises_transient_error_on_503(mock_get_client):
+    """Test that HarmonyTransientError is raised when Harmony status returns a 503."""
+    bignbit.utils.ED_USER = 'test'
+    bignbit.utils.ED_PASS = 'test'
+
+    mock_client = MagicMock()
+    mock_client.status.side_effect = _make_http_error(503)
+    mock_get_client.return_value = mock_client
+
+    with pytest.raises(HarmonyTransientError):
+        check_harmony_job('test-job-id', 'uat', 'test_variable', 'EPSG:4326')
+
+
+@patch('bignbit.get_harmony_job_status.utils.get_harmony_client')
+def test_check_harmony_job_reraises_non_5xx_http_error(mock_get_client):
+    """Test that non-5xx HTTPErrors (e.g. 404) are re-raised without wrapping."""
+    bignbit.utils.ED_USER = 'test'
+    bignbit.utils.ED_PASS = 'test'
+
+    mock_client = MagicMock()
+    mock_client.status.side_effect = _make_http_error(404)
+    mock_get_client.return_value = mock_client
+
+    with pytest.raises(requests.exceptions.HTTPError):
+        check_harmony_job('test-job-id', 'uat', 'test_variable', 'EPSG:4326')
+
+
+@patch('bignbit.get_harmony_job_status.utils.get_harmony_client')
+def test_check_harmony_job_result_urls_raises_transient_error_on_500(mock_get_client):
+    """Test that HarmonyTransientError is raised when result_urls call returns a 500."""
+    bignbit.utils.ED_USER = 'test'
+    bignbit.utils.ED_PASS = 'test'
+
+    mock_client = MagicMock()
+    mock_client.status.return_value = {'status': 'successful'}
+    mock_client.result_urls.side_effect = _make_http_error(500)
+    mock_get_client.return_value = mock_client
+
+    with pytest.raises(HarmonyTransientError) as exc_info:
+        check_harmony_job('test-job-id', 'uat', 'test_variable', 'EPSG:4326')
+
+    assert 'result urls' in str(exc_info.value).lower()
+    assert 'test-job-id' in str(exc_info.value)

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ resource "aws_sns_topic_policy" "default" {`
`21`	`21`	`}`
`22`	`22`
`23`	`23`	`resource "aws_sns_topic" "gibs_response_topic" {`
`24`		`- name = "${local.aws_resources_name}-gibs-response-topic"`
	`24`	`+ name = "svc-${local.sns_topic_name}-gibs-response-topic"`
`25`	`25`	`lifecycle {`
`26`	`26`	`# GIBS publishes to this topic, so we want to avoid destroying it unless coordinating the change with GIBS`
`27`	`27`	`prevent_destroy = true`