Skip to content

Fix OCP manifest start/end dates to reflect full range across all generators #623

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.

Sign up for GitHub

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

Open
esebesto wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix OCP manifest start/end dates to reflect full range across all generators #623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion nise/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from .helpers import gcp_calculate_persistent_disk_usage_amount
from .helpers import gcp_calculate_usage_amount_in_pricing

__version__ = "5.4.1"
__version__ = "5.4.2"
VERSION = __version__.split(".")
13 changes: 10 additions & 3 deletions nise/report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,8 @@ def ocp_create_report(options): # noqa: C901

monthly_files = []
monthly_ros_files = []
manifest_start_date = month.get("end")
manifest_end_date = month.get("start")
Comment on lines +927 to +928
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The initialization of manifest_start_date and manifest_end_date to the month's boundaries in reverse order is a functional way to find the min/max within the loop. However, if no generators are processed or if all are skipped (lines 936-939), the manifest will be generated with swapped start and end dates (e.g., start at the end of the month and end at the start).

Additionally, note that gen_start_date is still used outside the generator loop at line 976 (for the filename year) and line 1095 (for the payload key). To fully align with the PR's objective of reflecting the full reporting range and to avoid relying on loop-leaked variables, those instances should also be updated to use manifest_start_date.

for generator in generators:
generator_cls = generator.get("generator")
attributes = generator.get("attributes")
Expand All @@ -938,6 +940,11 @@ def ocp_create_report(options): # noqa: C901

gen_start_date, gen_end_date = _create_generator_dates_from_yaml(attributes, month)

if gen_start_date < manifest_start_date:
manifest_start_date = gen_start_date
if gen_end_date > manifest_end_date:
manifest_end_date = gen_end_date

gen = generator_cls(
gen_start_date, gen_end_date, attributes, ros_ocp_info, constant_values_ros_ocp, ros_only
)
Expand Down Expand Up @@ -978,7 +985,7 @@ def ocp_create_report(options): # noqa: C901
if insights_upload or minio_upload:
# Generate manifest for all files
ocp_assembly_id = uuid4()
report_datetime = gen_start_date
report_datetime = manifest_start_date
temp_files = {}
temp_ros_files = {}
for num_file in range(len(monthly_files)):
Expand Down Expand Up @@ -1057,8 +1064,8 @@ def ocp_create_report(options): # noqa: C901
"uuid": str(ocp_assembly_id),
"date": report_datetime.isoformat(timespec="microseconds"),
"files": manifest_file_names,
"start": gen_start_date.isoformat(timespec="microseconds"),
"end": gen_end_date.isoformat(timespec="microseconds"),
"start": manifest_start_date.isoformat(timespec="microseconds"),
"end": manifest_end_date.isoformat(timespec="microseconds"),
"version": __version__,
"certified": False,
"cr_status": cr_status,
Expand Down
74 changes: 74 additions & 0 deletions tests/test_report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
)
from nise.generators.ocp.ocp_generator import COST_OCP_REPORT_TYPE_TO_COLS
from nise.generators.ocp.ocp_generator import OCP_REPORT_TYPE_TO_COLS
from nise.manifest import ocp_generate_manifest as real_ocp_generate_manifest
from nise.report import _convert_bytes
from nise.report import _create_generator_dates_from_yaml
from nise.report import _create_month_list
Expand Down Expand Up @@ -1340,6 +1341,79 @@ def test_ocp_create_report_with_local_dir_static_generation_with_dates(self):

shutil.rmtree(local_insights_upload)

def test_ocp_create_report_manifest_dates_span_all_generators(self):
"""Manifest start/end must be min/max across all generators, not just the last one."""

now = datetime.datetime.now().replace(microsecond=0, second=0, minute=0, hour=0)
two_days_ago = now - datetime.timedelta(days=2)
yesterday = now - datetime.timedelta(days=1)
local_insights_upload = mkdtemp()
cluster_id = "11112222"
pod_spec = {
"pod": None,
"pod_name": "pod1",
"cpu_request": 1,
"mem_request_gig": 1,
"cpu_limit": 1,
"mem_limit_gig": 1,
}
static_ocp_data = {
"generators": [
{
"OCPGenerator": {
"start_date": str(two_days_ago.date()),
"end_date": str(now.date()),
"nodes": [
{
"node": None,
"node_name": "alpha",
"cpu_cores": 2,
"memory_gig": 4,
"namespaces": {"ns_a": {"pods": [pod_spec]}},
}
],
}
},
{
"OCPGenerator": {
"start_date": str(yesterday.date()),
"end_date": str(yesterday.date()),
"nodes": [
{
"node": None,
"node_name": "beta",
"cpu_cores": 2,
"memory_gig": 4,
"namespaces": {"ns_b": {"pods": [pod_spec]}},
}
],
}
},
]
}
options = {
"start_date": two_days_ago,
"end_date": now,
"insights_upload": local_insights_upload,
"ocp_cluster_id": cluster_id,
"static_report_data": static_ocp_data,
"write_monthly": True,
}
fix_dates(options, "ocp")
with patch("nise.report.ocp_generate_manifest", wraps=real_ocp_generate_manifest) as mock_manifest:
ocp_create_report(options)
mock_manifest.assert_called_once()
manifest_values = mock_manifest.call_args[0][0]
manifest_start = datetime.datetime.fromisoformat(manifest_values["start"])
manifest_end = datetime.datetime.fromisoformat(manifest_values["end"])

# Generator 1 spans two_days_ago→now (wider), generator 2 spans yesterday→yesterday (narrower).
# The manifest must cover the union: two_days_ago → now.
self.assertEqual(manifest_start.date(), two_days_ago.date())
self.assertEqual(manifest_end.date(), now.date())

shutil.rmtree(local_insights_upload)

@patch.dict(os.environ, {"INSIGHTS_USER": "12345", "INSIGHTS_PASSWORD": "54321"})
@patch("nise.report.requests.post")
def test_ocp_route_file(self, mock_post):
Expand Down
Loading