Skip to content
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.

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

fix(reana-dev): update container image labels when releasing (#765) #765

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions reana/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,9 @@
TIMEOUT = 900
"""Maximum timeout to wait for results when running demo analyses in CI."""

DOCKER_VERSION_FILE = "Dockerfile"
"""Docker version file."""

HELM_VERSION_FILE = "Chart.yaml"
"""Helm package version file."""

Expand Down
10 changes: 7 additions & 3 deletions reana/reana_dev/git.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# -*- coding: utf-8 -*-
#
# This file is part of REANA.
# Copyright (C) 2020, 2021 CERN.
# Copyright (C) 2020, 2021, 2023, 2024 CERN.
#
# REANA is free software; you can redistribute it and/or modify it
# under the terms of the MIT License; see LICENSE file for more details.
Expand All @@ -18,6 +18,7 @@
from reana.config import (
COMPONENTS_USING_SHARED_MODULE_COMMONS,
COMPONENTS_USING_SHARED_MODULE_DB,
DOCKER_VERSION_FILE,
GIT_DEFAULT_BASE_BRANCH,
HELM_VERSION_FILE,
JAVASCRIPT_VERSION_FILE,
Expand Down Expand Up @@ -135,7 +136,9 @@
if not next_version:
# bump current version depending on whether it is semver2 or pep440
current_version = get_current_component_version_from_source_files(component)
if version_files.get(HELM_VERSION_FILE):
if version_files.get(DOCKER_VERSION_FILE):
next_version = bump_semver2_version(current_version)
elif version_files.get(HELM_VERSION_FILE):

Check warning on line 141 in reana/reana_dev/git.py

View check run for this annotation

Codecov / codecov/patch

reana/reana_dev/git.py#L139-L141

Added lines #L139 - L141 were not covered by tests
next_version = bump_semver2_version(current_version)
elif version_files.get(PYTHON_VERSION_FILE):
next_version = bump_pep440_version(current_version)
Expand All @@ -146,7 +149,8 @@
else:
# provided next_version is always in pep440 version
if (
HELM_VERSION_FILE in version_files
DOCKER_VERSION_FILE in version_files
or HELM_VERSION_FILE in version_files
or JAVASCRIPT_VERSION_FILE in version_files
):
next_version = translate_pep440_to_semver2(next_version)
Expand Down
25 changes: 22 additions & 3 deletions reana/reana_dev/utils.py
Copy link
Member

Choose a reason for hiding this comment

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

How do we want to update the date in the Dockerfile, also considering that we are switching to release-please?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we'd have to update it ourselves at the day we are releasing and compiling the user-facing summary to be put into CHANGELOG before the regular news items generated by Release Please.

Might be nice to script it further, we would probably need to read upstream/release-please--branches--master branch content, enrich it and push back. Perhaps a job for a new command?

Copy link
Member

@mdonadoni mdonadoni Feb 9, 2024

Choose a reason for hiding this comment

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

But the user-facing summary would only be for the general reana repository, wouldn't it? I would avoid having an user-facing summary for each component, as I think that 1) that would not be very useful to the users in any case 2) it would cancel the benefit of using conventional commits and release-please by introducing a manual step in what could otherwise be a fully automatic release.

Regarding the second point, if we want to script it then I guess we could remove the label from the Dockerfile itself, and then set it from the CLI when building the image (either with reana-dev or with our custom GitHub action). In this way we would not need a new command, and we could still use release-please PRs without having to modify them.

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

WRT user-facing summaries, fully agree about doing it only for the user-facing repositories, such as reana and perhaps some like reana-client, reana-client-go. We may not even need them for every patchlevel release, only for "big" releases (major, and perhaps minor).

WRT updating also date, (a) I could amend git-create-release-commit command to do this too; (b) removing this label from Dockerfile could also be interesting at a slight con of not having full labels in the source code repository; (c) BTW we also do need to be aware of updating the date in the Release Please's made pull request, if the release is not happening the same day. We can see IRL about pros/cons?

Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import importlib.util
import json
import os
import re
import subprocess
import sys
from concurrent import futures
Expand All @@ -28,6 +29,7 @@
CLUSTER_DEPLOYMENT_MODES,
COMPONENTS_USING_SHARED_MODULE_COMMONS,
COMPONENTS_USING_SHARED_MODULE_DB,
DOCKER_VERSION_FILE,
GIT_DEFAULT_BASE_BRANCH,
HELM_VERSION_FILE,
JAVASCRIPT_VERSION_FILE,
Expand Down Expand Up @@ -613,6 +615,7 @@
"""Get a dictionary with all component's version files."""
version_files = {}
for file_ in [
DOCKER_VERSION_FILE,
HELM_VERSION_FILE,
OPENAPI_VERSION_FILE,
JAVASCRIPT_VERSION_FILE,
Expand Down Expand Up @@ -651,7 +654,17 @@
all_version_files = {version_file: all_version_files[version_file]}

version = ""
if all_version_files.get(HELM_VERSION_FILE):
if all_version_files.get(DOCKER_VERSION_FILE):
with open(all_version_files.get(DOCKER_VERSION_FILE)) as f:
for line in f.readlines():
match = re.match(

Check warning on line 660 in reana/reana_dev/utils.py

View check run for this annotation

Codecov / codecov/patch

reana/reana_dev/utils.py#L657-L660

Added lines #L657 - L660 were not covered by tests
r'LABEL org.opencontainers.image.version="(.*?)"', line
)
if match:
version = match.groups(1)[0]
break

Check warning on line 665 in reana/reana_dev/utils.py

View check run for this annotation

Codecov / codecov/patch

reana/reana_dev/utils.py#L663-L665

Added lines #L663 - L665 were not covered by tests

elif all_version_files.get(HELM_VERSION_FILE):

Check warning on line 667 in reana/reana_dev/utils.py

View check run for this annotation

Codecov / codecov/patch

reana/reana_dev/utils.py#L667

Added line #L667 was not covered by tests
with open(all_version_files.get(HELM_VERSION_FILE)) as f:
chart_yaml = yaml.safe_load(f.read())
version = chart_yaml["version"]
Expand Down Expand Up @@ -864,7 +877,11 @@
component, version_file=file_type
)

if file_type in [HELM_VERSION_FILE, JAVASCRIPT_VERSION_FILE]:
if file_type in [

Check warning on line 880 in reana/reana_dev/utils.py

View check run for this annotation

Codecov / codecov/patch

reana/reana_dev/utils.py#L880

Added line #L880 was not covered by tests
DOCKER_VERSION_FILE,
HELM_VERSION_FILE,
JAVASCRIPT_VERSION_FILE,
]:
new_version = (
translate_pep440_to_semver2(next_version)
if next_version
Expand All @@ -889,7 +906,9 @@
next_version_per_file_type[file_type] = new_version

# depending on a component, return proper component version
if HELM_VERSION_FILE in next_version_per_file_type.keys():
if DOCKER_VERSION_FILE in next_version_per_file_type.keys():
return next_version_per_file_type[DOCKER_VERSION_FILE], updated_files
elif HELM_VERSION_FILE in next_version_per_file_type.keys():

Check warning on line 911 in reana/reana_dev/utils.py

View check run for this annotation

Codecov / codecov/patch

reana/reana_dev/utils.py#L909-L911

Added lines #L909 - L911 were not covered by tests
return next_version_per_file_type[HELM_VERSION_FILE], updated_files
elif JAVASCRIPT_VERSION_FILE in next_version_per_file_type:
return next_version_per_file_type[JAVASCRIPT_VERSION_FILE], updated_files
Expand Down
Loading