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

Update console_link logic on clearing snapshots with missing repo #1219

Merged
merged 3 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from 2 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
7 changes: 5 additions & 2 deletions .github/workflows/full_pr_e2e_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ permissions:
contents: read # to fetch code (actions/checkout)

jobs:
full-es68-e2e-aws-test:
e2e-tests:
runs-on: ubuntu-latest
strategy:
matrix:
job-name: [full-es68source-e2e-test, rfs-default-e2e-test]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a few concerns here and would like to wait on enabling another Jenkins job on PRs:

  1. What is the additional tax on PR wait time
  2. Is this providing much value that our full test isn't covering? We would need to have additional environments on standby like we do for the full test in our current model as well to support this

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed for now, we can revisit later

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure happy to chat more around this sometime

steps:
- name: Sanitize branch and repo names
env:
Expand All @@ -34,6 +37,6 @@ jobs:
uses: lewijacn/[email protected]
with:
jenkins_url: "https://migrations.ci.opensearch.org"
job_name: "full-es68source-e2e-test"
job_name: "${{ matrix.job-name }}"
api_token: "${{ secrets.JENKINS_MIGRATIONS_GENERIC_WEBHOOK_TOKEN }}"
job_params: "GIT_REPO_URL=${{ steps.sanitize-input.outputs.pr_repo_url }},GIT_BRANCH=${{ steps.sanitize-input.outputs.branch_name }}"
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from console_link.models.cluster import Cluster, HttpMethod
from dataclasses import dataclass
import logging
from requests.exceptions import HTTPError

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -68,6 +69,7 @@ def clear_cluster(cluster: Cluster):


def clear_snapshots(cluster: Cluster, repository: str) -> None:
logger.info(f"Clearing snapshots from repository '{repository}'")
"""
Clears all snapshots from the specified repository.

Expand All @@ -79,7 +81,9 @@ def clear_snapshots(cluster: Cluster, repository: str) -> None:
# List all snapshots in the repository
snapshots_path = f"/_snapshot/{repository}/_all"
response = call_api(cluster, snapshots_path, raise_error=True)
logger.debug(f"Raw response: {response.json()}")
snapshots = response.json().get("snapshots", [])
logger.info(f"Found {len(snapshots)} snapshots in repository '{repository}'.")

if not snapshots:
logger.info(f"No snapshots found in repository '{repository}'.")
Expand All @@ -94,9 +98,11 @@ def clear_snapshots(cluster: Cluster, repository: str) -> None:

except Exception as e:
# Handle 404 errors specifically for missing repository
if "repository_missing_exception" in str(e):
logger.info(f"Repository '{repository}' is missing. Skipping snapshot clearing.")
else:
# Re-raise other errors
logger.error(f"Error clearing snapshots from repository '{repository}': {e}")
raise e
if isinstance(e, HTTPError) and e.response.status_code == 404:
error_details = e.response.json().get('error', {})
if error_details.get('type') == 'repository_missing_exception':
logger.info(f"Repository '{repository}' is missing. Skipping snapshot clearing.")
return
# Re-raise other errors
logger.error(f"Error clearing snapshots from repository '{repository}': {e}")
raise e
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import pytest
from unittest.mock import MagicMock
from console_link.middleware.clusters import clear_snapshots
from console_link.models.cluster import Cluster
import requests
import logging


# Helper to mock HTTPError with response
def create_http_error_mock(status_code, json_data):
response_mock = MagicMock()
response_mock.status_code = status_code
response_mock.json.return_value = json_data
return requests.exceptions.HTTPError(response=response_mock)


@pytest.fixture
def mock_cluster_with_missing_repo(mocker):
cluster = MagicMock(spec=Cluster)
# Simulate repository missing exception
error_mock = create_http_error_mock(
status_code=404,
json_data={
'error': {
'type': 'repository_missing_exception',
'reason': '[migration_assistant_repo] missing'
}
}
)
mocker.patch.object(cluster, 'call_api', side_effect=error_mock)
return cluster


@pytest.fixture
def mock_cluster_with_snapshots(mocker):
cluster = MagicMock(spec=Cluster)
# Mock the response for listing snapshots
mock_response = MagicMock()
mock_response.json.return_value = {
'snapshots': [
{'snapshot': 'snapshot_1'},
{'snapshot': 'snapshot_2'}
]
}
mock_response.status_code = 200

def mock_call_api(path, *args, **kwargs):
if "_all" in path:
return mock_response
elif "snapshot_1" in path or "snapshot_2" in path:
return MagicMock() # Simulate successful deletion
raise ValueError(f"Unexpected path: {path}")

mocker.patch.object(cluster, 'call_api', side_effect=mock_call_api)
return cluster


def test_clear_snapshots_repository_missing(mock_cluster_with_missing_repo, caplog):
with caplog.at_level(logging.INFO, logger='console_link.middleware.clusters'):
clear_snapshots(mock_cluster_with_missing_repo, 'migration_assistant_repo')
assert "Repository 'migration_assistant_repo' is missing. Skipping snapshot clearing." in caplog.text


def test_clear_snapshots_success(mock_cluster_with_snapshots, caplog):
with caplog.at_level(logging.INFO, logger='console_link.middleware.clusters'):
clear_snapshots(mock_cluster_with_snapshots, 'migration_assistant_repo')
assert "Deleted snapshot: snapshot_1 from repository 'migration_assistant_repo'." in caplog.text
assert "Deleted snapshot: snapshot_2 from repository 'migration_assistant_repo'." in caplog.text
Loading