Skip to content

CSPL-957: Automation tests to verify deletion of C3 SVA & refactoring of test cases to align with the model of "CRUD of SVAs" #301

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
Merged
merged 2 commits into from
Apr 2, 2021
Merged

CSPL-957: Automation tests to verify deletion of C3 SVA & refactoring of test cases to align with the model of "CRUD of SVAs" #301

merged 2 commits into from
Apr 2, 2021

Conversation

jambrosiano
Copy link
Contributor

@jambrosiano jambrosiano commented Mar 30, 2021 •
edited
Loading

  • Adding new test for the bug described CSPL-780:

    • "crcrud: can verify IDXC, CM and SHC PVCs are correctly deleted after the CRs deletion"
    • Steps for this new test:
    • Deploy C3 SVA (CM,IDXC,SHC with deployer)
    • Once everything is ready, verify PVCs (etc and var) exists for CM,IDXC,SHC, deployer
    • Delete the SHC,IDXC,CM
    • verify PVCs (etc and var) have been deleted for CM,IDXC,SHC, deployer

  • Refactor of the 'custom_resource_crud' folder:

    • Split test cases into individual files based on SVA (S1, C3, M4)

Tests runs:
New test:
image

Refactored tests:
image
image
image

@jambrosiano jambrosiano force-pushed the CSPL-957 branch 2 times, most recently from b87a6cc to 171426c Compare March 30, 2021 02:42
pdhanoya-splunk
pdhanoya-splunk suggested changes Mar 30, 2021
View reviewed changes
sgontla
sgontla reviewed Mar 30, 2021
View reviewed changes
@jambrosiano jambrosiano force-pushed the CSPL-957 branch 4 times, most recently from a588970 to 79d0af2 Compare March 31, 2021 23:24
@pdhanoya-splunk
Copy link
Contributor

@jambrosiano Can you please provide detailed test steps and also passing runs as the fix has been merged.

pdhanoya-splunk
pdhanoya-splunk reviewed Mar 31, 2021
View reviewed changes
test/testenv/verificationutils.go Outdated
}
}

// VerifyPVCExists verifies if PVC exist
Copy link
Contributor

Choose a reason for hiding this comment

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

VerifyPVCExists vs VerifyPVC as we testing existing or not existing depending on value of expectedtoExist

pdhanoya-splunk
pdhanoya-splunk reviewed Mar 31, 2021
View reviewed changes
test/testenv/verificationutils.go Outdated
result = true
logf.Log.Info("PVC status (exists or not) is as expected: ", "PVC", pvcName, "STATUS", pvcexists, "EXPECTED", expectedtoExist)
} else {
err := errors.New("incorrect PVC status (exists when it shouldn't or the opposite): ")
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be just an INFO Log. As test will fail in this scenario. Also instead of using logf
testenvInstance.Log.Info("Incorrect PVC Status", "PVC NAME", pvcName, "STATUS", pvcexists, "EXPECTED", expectedtoExist)

pdhanoya-splunk
pdhanoya-splunk reviewed Mar 31, 2021
View reviewed changes
test/testenv/verificationutils.go Outdated
}

// VerifyPVCExists verifies if PVC exist
func VerifyPVCExists(deployment *Deployment, testenvInstance *TestEnv, ns string, pvcName string, expectedtoExist bool, verificationTimeout time.Duration) {
Copy link
Contributor

Choose a reason for hiding this comment

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

expectedtoExist vs expectedToExist

pdhanoya-splunk
pdhanoya-splunk reviewed Mar 31, 2021
View reviewed changes
test/testenv/verificationutils.go Outdated
func VerifyPVCExists(deployment *Deployment, testenvInstance *TestEnv, ns string, pvcName string, expectedtoExist bool, verificationTimeout time.Duration) {
gomega.Eventually(func() bool {
result := false
pvcexists := false
Copy link
Contributor

Choose a reason for hiding this comment

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

pvcexists vs pvcExists

pdhanoya-splunk
pdhanoya-splunk reviewed Mar 31, 2021
View reviewed changes
test/testenv/verificationutils.go Outdated
PvcName = fmt.Sprintf(PVCString, "var", deployment.GetName(), deploymentType, 0)
VerifyPVCExists(deployment, testenvInstance, testenvInstance.GetName(), PvcName, expectedtoExist, verificationTimeout)
} else {
for i := 0; i <= 2; i++ {
Copy link
Contributor

@pdhanoya-splunk pdhanoya-splunk Mar 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Will this work if deployment has more/less than 2. I think its not good to hardcode number of nodes here ?

This can be accepted as instances parameter and the whole function can be refactored into a simple for loop.

pvcKind := []string{"etc", "var"}
for i := 0, i < instances ; i++ {
   for _, pvcVolumeKind := range  pvcKind {
     pvcName := fmt.Sprintf(PVCString, pvcVolumeKind, deployment.GetName(), deploymentType, i)
     VerifyPVCExists(deployment, testenvInstance, testenvInstance.GetName(), PvcName, expectedtoExist, verificationTimeout)

}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed

pdhanoya-splunk
pdhanoya-splunk reviewed Apr 1, 2021
View reviewed changes
test/testenv/verificationutils.go Outdated
// VerifyPVCsPerDeployment verifies for a given deployment if PVCs (etc and var) exists
func VerifyPVCsPerDeployment(deployment *Deployment, testenvInstance *TestEnv, deploymentType string, expectedtoExist bool, verificationTimeout time.Duration) {
if deploymentType == "shc-deployer" || deploymentType == "cluster-master" {
PvcName := fmt.Sprintf(PVCString, "etc", deployment.GetName(), deploymentType, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

PvcName first letter should be capital for only exported kinds.

pdhanoya-splunk
pdhanoya-splunk reviewed Apr 1, 2021
View reviewed changes
test/custom_resource_crud/custom_resource_crud_c3_test.go Outdated
testenv.VerifyRFSFMet(deployment, testenvInstance)

// Verify CPU limits on Indexers before updating the CR
for i := 0; i <= 2; i++ {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of hardcoding 2 this can be stored in variable called indexerCount

pdhanoya-splunk
pdhanoya-splunk reviewed Apr 1, 2021
View reviewed changes
test/custom_resource_crud/custom_resource_crud_c3_test.go Outdated
testenv.SearchHeadClusterReady(deployment, testenvInstance)

// Verify CPU limits after updating the CR
for i := 0; i <= 2; i++ {
Copy link
Contributor

Choose a reason for hiding this comment

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

Store number of search heads asa searchHeadCount

@jambrosiano jambrosiano force-pushed the CSPL-957 branch 2 times, most recently from 63557dc to e52780c Compare April 1, 2021 01:13
@jambrosiano jambrosiano force-pushed the CSPL-957 branch 2 times, most recently from 74de6cf to 919b785 Compare April 1, 2021 01:41
@jambrosiano jambrosiano force-pushed the CSPL-957 branch 7 times, most recently from ff1c3fc to 8d0e425 Compare April 1, 2021 19:17
pdhanoya-splunk
pdhanoya-splunk reviewed Apr 1, 2021
View reviewed changes
pdhanoya-splunk
pdhanoya-splunk reviewed Apr 1, 2021
View reviewed changes
test/testenv/verificationutils.go Outdated
logf.Log.Info("PVC status (exists or not) is as expected: ", "PVC", pvcName, "STATUS", pvcExists, "EXPECTED", expectedToExist)
} else {
testenvInstance.Log.Info("Incorrect PVC Status", "PVC NAME", pvcName, "STATUS", pvcExists, "EXPECTED", expectedToExist)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Different log objects used on line 524 vs 526

@jambrosiano jambrosiano force-pushed the CSPL-957 branch 3 times, most recently from 90dda03 to a56e757 Compare April 1, 2021 20:43
smohan-splunk
smohan-splunk approved these changes Apr 1, 2021
View reviewed changes
Copy link
Contributor

@smohan-splunk smohan-splunk left a comment

Choose a reason for hiding this comment

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

As discussed, a new PR will cover the Deletion part for SVAs for M4 & S1

@smohan-splunk smohan-splunk changed the title CSPL-957 CSPL-957: Automation tests to verify deletion of C3 SVA & refactoring of test cases to align with the model of "CRUD of SVAs" Apr 1, 2021
@smohan-splunk smohan-splunk merged commit 2986b42 into develop Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akondur akondur Awaiting requested review from akondur

@kashok-splunk kashok-splunk Awaiting requested review from kashok-splunk

@mgaldino-splunk mgaldino-splunk Awaiting requested review from mgaldino-splunk

@vebken-splunk vebken-splunk Awaiting requested review from vebken-splunk

@kumarajeet kumarajeet Awaiting requested review from kumarajeet

@jryb jryb Awaiting requested review from jryb

@gaurav-splunk gaurav-splunk Awaiting requested review from gaurav-splunk

@sgontla sgontla Awaiting requested review from sgontla

3 more reviewers

@smohan-splunk smohan-splunk smohan-splunk approved these changes

@pdhanoya-splunk pdhanoya-splunk pdhanoya-splunk approved these changes

@tpereirasplunk tpereirasplunk tpereirasplunk approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jambrosiano @pdhanoya-splunk @smohan-splunk @sgontla @tpereirasplunk