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.

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

Jump to bottom

refactor(keel): use SpinnakerRetrofitErrorHandler with KeelService #4636

Merged
merged 4 commits into from
Feb 27, 2024
Merged

refactor(keel): use SpinnakerRetrofitErrorHandler with KeelService #4636

merged 4 commits into from
Feb 27, 2024

Commits on Feb 27, 2024

  1. test(keel): Test to verify the timestamps in SpringHttpError 

    across different time units when any 4xx http error has occured. These tests ensures the upcoming changes on KeelService with SpinnakerRetrofitErrorHandler, will not modify/break the existing functionality.

Tests covers positive and negative cases with different units of timestamps. In addition, another test added to verify when the error response body doesn't have timestamp field.
    @Pranav-b-7
    Pranav-b-7 committed Feb 27, 2024
    Configuration menu
    Copy the full SHA
    5d711ab View commit details
    Browse the repository at this point in the history

  2. test(keel): demonstrate behavior of ImportDeliveryConfigTask on http … 

    …errors and network errors

Test cases added to verify the upcoming changes when KeelService is configured with SpinnakerRetrofitErrorHandler.

These tests verifies the behaviour of the method handleRetryableFailures() when 4xx with empty error body and 5xx http erros are thrown, and also during network errors.

NOTE : These tests only verifies the scenario when the error response body is empty/null, and network errors.
    @Pranav-b-7
    Pranav-b-7 committed Feb 27, 2024
    Configuration menu
    Copy the full SHA
    cde3b39 View commit details
    Browse the repository at this point in the history

  3. test(keel): remove duplicate tests from ImportDeliveryConfigTaskTests 

    The deleted tests are the cases/scenarios which are already covered as part of the commit: 5d711ab.

These tests covers up the http 4xx error cases of the API : keelService.publishDeliveryConfig.

Tests deleted are : 1. 'keel access denied error', 2. 'delivery config parsing error'.
    @Pranav-b-7
    Pranav-b-7 committed Feb 27, 2024
    Configuration menu
    Copy the full SHA
    11b5774 View commit details
    Browse the repository at this point in the history

  4. refactor(keel): use SpinnakerRetrofitErrorHandler with KeelService 

    This PR lays the foundational work for upgrading the retrofit version to 2.x, specifically focusing on refactoring the exception handling for KeelService

The tests modified as part of this PR will verify the new changes with the scenarios:- Reading the Http error response body and building the TaskResult by instantiating SpringHttpError.

Note, there's a behaviour change on the Task Results error message format when KeelService API throws any 4xx/5xx http errors with empty error body.

- On any 4xx http errors with empty error body:

  before:

  11:56:19.324 [Test worker] ERROR com.netflix.spinnaker.orca.keel.task.ImportDeliveryConfigTask - {message=Non-retryable HTTP response 400 received from downstream service: HTTP 400 http://localhost:62130/delivery-configs/: 400 Bad Request}

  after:

  12:00:02.018 [Test worker] ERROR com.netflix.spinnaker.orca.keel.task.ImportDeliveryConfigTask - {message=Non-retryable HTTP response 400 received from downstream service: HTTP 400 http://localhost:62275/delivery-configs/: Status: 400, URL: http://localhost:62275/delivery-configs/, Message: Bad Request}

- On any 5xx http errors with empty error body:

  before:

  TaskResult(status=RUNNING, context={repoType=stash, projectKey=SPKR, repositorySlug=keeldemo, directory=., manifest=spinnaker.yml, ref=refs/heads/master, attempt=2, maxRetries=5, errorFromLastAttempt=Retryable HTTP response 500 received from downstream  service: HTTP 500 http://localhost:65311/delivery-configs/: 500 Server Error}, outputs={})

  after:

  TaskResult(status=RUNNING, context={repoType=stash, projectKey=SPKR, repositorySlug=keeldemo, directory=., manifest=spinnaker.yml, ref=refs/heads/master, attempt=1, maxRetries=5, errorFromLastAttempt=Retryable HTTP response 500 received from downstream  service: HTTP 500 http://localhost:49862/delivery-configs/: Status: 500, URL: http://localhost:49862/delivery-configs/, Message: Server Error}, outputs={})
    @Pranav-b-7
    Pranav-b-7 committed Feb 27, 2024
    Configuration menu
    Copy the full SHA
    c417922 View commit details
    Browse the repository at this point in the history