Skip to content

Commit

Permalink
Revert "YR/Handle long running pipelines, and commits with no pipelin…
Browse files Browse the repository at this point in the history 
…es/CIAC-9386 (#32462)" (#32974)

This reverts commit 29aa622.

Co-authored-by: Michael Yochpaz <[email protected]>
  • Loading branch information
@RosenbergYehuda @MichaelYochpaz @maimorag
2 people authored and maimorag committed Feb 28, 2024
1 parent 212c7ed commit c069fe2
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 330 deletions.
180 changes: 31 additions & 149 deletions Tests/scripts/common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from Tests.scripts.utils import logging_wrapper as logging
from gitlab.v4.objects.pipelines import ProjectPipeline
from gitlab.v4.objects.commits import ProjectCommit
from itertools import pairwise


CONTENT_NIGHTLY = 'Content Nightly'
Expand Down Expand Up @@ -258,73 +257,65 @@ def get_pipelines_and_commits(gitlab_client: Gitlab, project_id,
return pipelines, commits


def get_person_in_charge(commit: ProjectCommit) -> tuple[str, str, str] | tuple[None, None, None]:
def get_person_in_charge(commit):
"""
Returns the name of the person in charge of the commit, the PR link and the beginning of the PR name.
Returns the name, email, and PR link for the author of the provided commit.
Args:
commit: The Gitlab commit object containing author info.
Returns:
name: The name of the commit author.
pr: The GitHub PR link for the Gitlab commit.
beginning_of_pr_name: The beginning of the PR name.
"""
name = commit.author_name
# pr number is always the last id in the commit title, starts with a number sign, may or may not be in parenthesis.
pr_number = commit.title.split("#")[-1].strip("()")
beginning_of_pr_name = commit.title[:20] + "..."
if pr_number.isnumeric():
pr = f"https://github.com/demisto/content/pull/{pr_number}"
return name, pr, beginning_of_pr_name
return name, pr
else:
return None, None, None
return None, None


def are_pipelines_in_order(pipeline_a: ProjectPipeline, pipeline_b: ProjectPipeline) -> bool:
def are_pipelines_in_order(current_pipeline: ProjectPipeline, previous_pipeline: ProjectPipeline) -> bool:
"""
Check if the pipelines are in the same order of their commits.
This function checks if the current pipeline was created after the previous pipeline, to avoid rare conditions
that pipelines are not in the same order as the commits.
Args:
pipeline_a: The first pipeline object.
pipeline_b: The second pipeline object.
current_pipeline: The current pipeline object.
previous_pipeline: The previous pipeline object.
Returns:
bool
"""

pipeline_a_timestamp = parser.parse(pipeline_a.created_at)
pipeline_b_timestamp = parser.parse(pipeline_b.created_at)
return pipeline_a_timestamp > pipeline_b_timestamp
previous_pipeline_timestamp = parser.parse(previous_pipeline.created_at)
current_pipeline_timestamp = parser.parse(current_pipeline.created_at)
return current_pipeline_timestamp > previous_pipeline_timestamp


def is_pivot(current_pipeline: ProjectPipeline, pipeline_to_compare: ProjectPipeline) -> bool | None:
def is_pivot(current_pipeline: ProjectPipeline, previous_pipeline: ProjectPipeline) -> bool | None:
"""
Is the current pipeline status a pivot from the previous pipeline status.
Args:
current_pipeline: The current pipeline object.
pipeline_to_compare: a pipeline object to compare to.
previous_pipeline: The previous pipeline object.
Returns:
True status changed from success to failed
False if the status changed from failed to success
None if the status didn't change or the pipelines are not in order of commits
"""

in_order = are_pipelines_in_order(pipeline_a=current_pipeline, pipeline_b=pipeline_to_compare)
in_order = are_pipelines_in_order(current_pipeline, previous_pipeline)
if in_order:
if pipeline_to_compare.status == 'success' and current_pipeline.status == 'failed':
if previous_pipeline.status == 'success' and current_pipeline.status == 'failed':
return True
if pipeline_to_compare.status == 'failed' and current_pipeline.status == 'success':
if previous_pipeline.status == 'failed' and current_pipeline.status == 'success':
return False
return None


def get_reviewer(pr_url: str) -> str | None:
"""
Get the first reviewer who approved the PR.
Args:
pr_url: The URL of the PR.
Returns:
The name of the first reviewer who approved the PR.
"""
approved_reviewer = None
try:
# Extract the owner, repo, and pull request number from the URL
Expand All @@ -346,14 +337,6 @@ def get_reviewer(pr_url: str) -> str | None:


def get_slack_user_name(name: str | None, name_mapping_path: str) -> str:
"""
Get the slack user name for a given Github name.
Args:
name: The name to convert.
name_mapping_path: The path to the name mapping file.
Returns:
The slack user name.
"""
with open(name_mapping_path) as map:
mapping = json.load(map)
# If the name is the name of the 'docker image update bot' reviewer - return the owner of that bot.
Expand All @@ -364,131 +347,30 @@ def get_slack_user_name(name: str | None, name_mapping_path: str) -> str:


def get_commit_by_sha(commit_sha: str, list_of_commits: list[ProjectCommit]) -> ProjectCommit | None:
"""
Get a commit by its SHA.
Args:
commit_sha: The SHA of the commit.
list_of_commits: A list of commits.
Returns:
The commit object.
"""
return next((commit for commit in list_of_commits if commit.id == commit_sha), None)


def get_pipeline_by_commit(commit: ProjectCommit, list_of_pipelines: list[ProjectPipeline]) -> ProjectPipeline | None:
"""
Get a pipeline by its commit.
Args:
commit: The commit object.
list_of_pipelines: A list of pipelines.
Returns:
The pipeline object.
"""
return next((pipeline for pipeline in list_of_pipelines if pipeline.sha == commit.id), None)


def create_shame_message(suspicious_commits: list[ProjectCommit],
pipeline_changed_status: bool, name_mapping_path: str) -> tuple[str, str, str, str] | None:
def create_shame_message(current_commit: ProjectCommit,
pipeline_changed_status: bool, name_mapping_path: str) -> tuple[str, str, str] | None:
"""
Create a shame message for the person in charge of the commit, or for multiple people in case of multiple suspicious commits.
Args:
suspicious_commits: A list of suspicious commits.
pipeline_changed_status: A boolean indicating if the pipeline status changed.
name_mapping_path: The path to the name mapping file.
Returns:
A tuple of strings containing the message, the person in charge, the PR link and the color of the message.
Create a shame message for the person in charge of the commit.
"""
hi_and_status = person_in_charge = in_this_pr = color = ""
for suspicious_commit in suspicious_commits:
name, pr, beginning_of_pr = get_person_in_charge(suspicious_commit)
if name and pr and beginning_of_pr:
if name == CONTENT_BOT:
name = get_reviewer(pr)
name = get_slack_user_name(name, name_mapping_path)
msg = "broken" if pipeline_changed_status else "fixed"
color = "danger" if pipeline_changed_status else "good"
emoji = ":cry:" if pipeline_changed_status else ":muscle:"
if suspicious_commits.index(suspicious_commit) == 0:
hi_and_status = f"Hi, The build was {msg} {emoji} by:"
person_in_charge = f"@{name}"
in_this_pr = f" That was done in this PR: {slack_link(pr, beginning_of_pr)}"

else:
person_in_charge += f" or @{name}"
in_this_pr = ""

return (hi_and_status, person_in_charge, in_this_pr, color) if hi_and_status and person_in_charge and color else None
name, pr = get_person_in_charge(current_commit)
if name and pr:
if name == CONTENT_BOT:
name = get_reviewer(pr)
name = get_slack_user_name(name, name_mapping_path)
msg = "broke" if pipeline_changed_status else "fixed"
color = "danger" if pipeline_changed_status else "good"
emoji = ":cry:" if pipeline_changed_status else ":muscle:"
return (f"Hi @{name}, You {msg} the build! {emoji} ",
f" That was done in this {slack_link(pr,'PR.')}", color)
return None


def slack_link(url: str, text: str) -> str:
"""
Create a slack link.
Args:
url: The URL to link to.
text: The text to display.
Returns:
The slack link.
"""
return f"<{url}|{text}>"


def was_message_already_sent(commit_index: int, list_of_commits: list, list_of_pipelines: list) -> bool:
"""
Check if a message was already sent for newer commits, this is possible if pipelines of later commits,
finished before the pipeline of the current commit.
Args:
commit_index: The index of the current commit.
list_of_commits: A list of commits.
list_of_pipelines: A list of pipelines.
Returns:
"""
for previous_commit, current_commit in pairwise(reversed(list_of_commits[:commit_index])):
current_pipeline = get_pipeline_by_commit(current_commit, list_of_pipelines)
previous_pipeline = get_pipeline_by_commit(previous_commit, list_of_pipelines)
# in rare cases some commits have no pipeline
if current_pipeline and previous_pipeline and (is_pivot(current_pipeline, previous_pipeline) is not None):
return True
return False


def get_nearest_newer_commit_with_pipeline(list_of_pipelines: list[ProjectPipeline], list_of_commits: list[ProjectCommit],
current_commit_index: int) -> tuple[ProjectPipeline, list] | tuple[None, None]:
"""
Get the nearest newer commit that has a pipeline.
Args:
list_of_pipelines: A list of pipelines.
list_of_commits: A list of commits.
current_commit_index: The index of the current commit.
Returns:
A tuple of the nearest pipeline and a list of suspicious commits that have no pipelines.
"""
suspicious_commits = []
for index in reversed(range(0, current_commit_index - 1)):
next_commit = list_of_commits[index]
suspicious_commits.append(list_of_commits[index + 1])
next_pipeline = get_pipeline_by_commit(next_commit, list_of_pipelines)
if next_pipeline:
return next_pipeline, suspicious_commits
return None, None


def get_nearest_older_commit_with_pipeline(list_of_pipelines: list[ProjectPipeline], list_of_commits: list[ProjectCommit],
current_commit_index: int) -> tuple[ProjectPipeline, list] | tuple[None, None]:
"""
Get the nearest oldest commit that has a pipeline.
Args:
list_of_pipelines: A list of pipelines.
list_of_commits: A list of commits.
current_commit_index: The index of the current commit.
Returns:
A tuple of the nearest pipeline and a list of suspicious commits that have no pipelines.
"""
suspicious_commits = []
for index in range(current_commit_index, len(list_of_commits) - 1):
previous_commit = list_of_commits[index + 1]
suspicious_commits.append(list_of_commits[index])
previous_pipeline = get_pipeline_by_commit(previous_commit, list_of_pipelines)
if previous_pipeline:
return previous_pipeline, suspicious_commits
return None, None
58 changes: 18 additions & 40 deletions Tests/scripts/gitlab_slack_notifier.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
replace_escape_characters
from Tests.scripts.github_client import GithubPullRequest
from Tests.scripts.common import get_pipelines_and_commits, is_pivot, get_commit_by_sha, get_pipeline_by_commit, \
create_shame_message, slack_link, was_message_already_sent, get_nearest_newer_commit_with_pipeline, \
get_nearest_older_commit_with_pipeline
create_shame_message, slack_link
from Tests.scripts.test_modeling_rule_report import calculate_test_modeling_rule_results, \
read_test_modeling_rule_to_jira_mapping, get_summary_for_test_modeling_rule, TEST_MODELING_RULES_TO_JIRA_TICKETS_CONVERTED
from Tests.scripts.test_playbooks_report import read_test_playbook_to_jira_mapping, TEST_PLAYBOOKS_TO_JIRA_TICKETS_CONVERTED
Expand Down Expand Up @@ -361,7 +360,7 @@ def construct_slack_msg(triggering_workflow: str,
pipeline_url: str,
pipeline_failed_jobs: list[ProjectPipelineJob],
pull_request: GithubPullRequest | None,
shame_message: tuple[str, str, str, str] | None) -> tuple[list[dict[str, Any]], list[dict[str, Any]]]:
shame_message: tuple[str, str, str] | None) -> tuple[list[dict[str, Any]], list[dict[str, Any]]]:
# report failing jobs
content_fields = []

Expand Down Expand Up @@ -442,9 +441,9 @@ def construct_slack_msg(triggering_workflow: str,
title += title_append
slack_msg_start = []
if shame_message:
hi_and_status, person_in_charge, in_this_pr, shame_color = shame_message
shame_title, shame_value, shame_color = shame_message
slack_msg_start.append({
"title": f"{hi_and_status}\n{person_in_charge}\n{in_this_pr}",
"title": f"{shame_title}\n{shame_value}",
"color": shame_color
})
return slack_msg_start + [{
Expand Down Expand Up @@ -562,53 +561,32 @@ def main():

pipeline_url, pipeline_failed_jobs = collect_pipeline_data(gitlab_client, project_id, pipeline_id)
shame_message = None
computed_slack_channel = "dmst-build-test"
if options.current_branch == DEFAULT_BRANCH and triggering_workflow == CONTENT_MERGE:
# Check if the current commit's pipeline differs from the previous one. If the previous pipeline is still running,
# compare the next build. For commits without pipelines, compare the current one to the nearest commit with a
# pipeline and all those in between, marking them as suspicious.
# We check if the previous build failed and this one passed, or wise versa.
list_of_pipelines, list_of_commits = get_pipelines_and_commits(gitlab_client=gitlab_client,
project_id=project_id, look_back_hours=LOOK_BACK_HOURS)
current_commit = get_commit_by_sha(commit_sha, list_of_commits)
if current_commit:
current_commit_index = list_of_commits.index(current_commit)

# If the current commit is the last commit in the list, there is no previous commit,
# since commits are in ascending order
# or if we already sent a shame message for newer commits, we don't want to send another one for older commits.
if (current_commit_index != len(list_of_commits) - 1
and not was_message_already_sent(current_commit_index, list_of_commits, list_of_pipelines)):
if current_commit_index != len(list_of_commits) - 1:
previous_commit = list_of_commits[current_commit_index + 1]
current_pipeline = get_pipeline_by_commit(current_commit, list_of_pipelines)

# looking backwards until we find a commit with a pipeline to compare with
previous_pipeline, suspicious_commits = get_nearest_older_commit_with_pipeline(
list_of_pipelines, list_of_commits, current_commit_index)
if previous_pipeline and suspicious_commits and current_pipeline:
pipeline_changed_status = is_pivot(current_pipeline=current_pipeline,
pipeline_to_compare=previous_pipeline)

previous_pipeline = get_pipeline_by_commit(previous_commit, list_of_pipelines)
if current_pipeline and previous_pipeline:
pipeline_changed_status = is_pivot(current_pipeline, previous_pipeline)
logging.info(
f"Checking pipeline id: {current_pipeline.id}, of commit: {current_commit.title}, "
f"after comparing with pipeline id: {previous_pipeline.id},"
f"the change status is: {pipeline_changed_status}")

if pipeline_changed_status is None and current_commit_index > 0:
# looking_forward until we find a commit with a pipeline to compare with
next_pipeline, suspicious_commits = get_nearest_newer_commit_with_pipeline(
list_of_pipelines, list_of_commits, current_commit_index)

if next_pipeline and suspicious_commits:
pipeline_changed_status = is_pivot(current_pipeline=next_pipeline,
pipeline_to_compare=current_pipeline)
logging.info(
f" after comparing with pipeline id: {next_pipeline.id},"
f"the change status is: {pipeline_changed_status}")

f"Checking pipeline {current_pipeline}, the commit is {current_commit} "
f"and the pipeline change status is: {pipeline_changed_status}"
)
if pipeline_changed_status is not None:
shame_message = create_shame_message(suspicious_commits, pipeline_changed_status, # type: ignore
options.name_mapping_path)
shame_message = create_shame_message(
current_commit, pipeline_changed_status, options.name_mapping_path
)
computed_slack_channel = "test_slack_notifier_when_master_is_broken"

else:
computed_slack_channel = "dmst-build-test"
slack_msg_data, threaded_messages = construct_slack_msg(triggering_workflow, pipeline_url, pipeline_failed_jobs, pull_request,
shame_message)

Expand Down
Loading

0 comments on commit c069fe2

Please sign in to comment.