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

Volume delete & Storage Response check. #213

Merged
merged 1 commit into from
Feb 14, 2023
Merged

Volume delete & Storage Response check. #213

merged 1 commit into from
Feb 14, 2023

Conversation

IsaiasA1
Copy link
Contributor

@IsaiasA1 IsaiasA1 commented Feb 13, 2023

Description

This fixes a defect where deleted volumes are getting pulled, and a Bad Gateway Error caused by misplaced command.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#408

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Backward compatibility is not broken

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

  • Tested on my local system with powerflex:
    2023/02/13 17:32:28 Starting volume test
    Feature: Karavi Authorization
    As a consumer of the Karavi Authorization
    I want to test karavi authorization
    So that they are known to work

    Scenario: Show Volumes # features/volume.feature:7
    Given Karavi Authorization is available and configured # :1 -> *feature
    And I have # :1 -> *feature
    When I create a request from with storage capacity # :1 -> *feature
    Then I can request a pod to consume # :1 -> *feature
    And is bound to storage # :1 -> *feature
    And volume is created in backend storage # :1 -> *feature
    And dellctl volume list # :1 -> *feature
    And I delete the pod and # :1 -> *feature
    And is deleted from kubernetes # :1 -> *feature
    And volume is deleted from backend storage # :1 -> *feature

    Examples:
    | storageclass | size | pvc |
    | "vxflexos-xfs" | "2Mi" | "pineapple" |
    | "vxflexos-xfs" | "4Mi" | "greenapple" |
    | "vxflexos-xfs" | "1Mi" | "redapple" |

3 scenarios (3 passed)
30 steps (30 passed)
2m15.085141885s
2023/02/13 17:34:43 Volume test finished
PASS
status 0
ok karavi-testing/karavi-authorization/auth-test 135.154s

@IsaiasA1
Copy link
Contributor Author

run e2e test

@@ -738,15 +740,14 @@ func volumesHandler(roleServ *roleClientService, storageServ *storageClientServi
}

storageResp, err = storageServ.storageClient.GetPowerflexVolumes(r.Context(), powerflexVolumesRequest)

volumeList = append(volumeList, storageResp.Volume...)

if err != nil {
w.WriteHeader(http.StatusInternalServerError)
log.WithError(err).Println("unable to get powerflex volumes")
return
Copy link
Contributor

Choose a reason for hiding this comment

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

If there is an error, the client will only get the status code and no error message. We should write an error similar to how we do in other places in this handler with web.JSONErrorResponse.

@IsaiasA1 IsaiasA1 merged commit 660418f into main Feb 14, 2023
IsaiasA1 added a commit that referenced this pull request Feb 14, 2023
@IsaiasA1 IsaiasA1 deleted the Volume-delete-bug branch March 9, 2023 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants