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

Fix some code and add UTs #28

Merged
merged 6 commits into from
Dec 7, 2023
Merged

Fix some code and add UTs #28

merged 6 commits into from
Dec 7, 2023

Conversation

ShellyKa13
Copy link
Collaborator

code fixes:

  • skip volume snapshot class checkup in case of unsupported provisioners
  • handle golden images different errors the same way

Adjust UT accordingly adjust and add negative UTs

In case there are no valid golden images also
in the case where the golden image doesnt have
data source we dont want to exit the checkup we
can continue to the next check which is check existing
VMIs and then on the checks that are dependent on the
golden images we should show the same message and skip.

Signed-off-by: Shelly Kagan <[email protected]>
adjust reporter UT
adjusr checkup UT
add negative UT for checkup

Signed-off-by: Shelly Kagan <[email protected]>
Signed-off-by: Shelly Kagan <[email protected]>
Comment on lines 46 to 59
tests := map[string]struct {
checkup checkupStub
reporter reporterStub
errors []string
}{
"report fails": {checkup: checkupStub{}, reporter: reporterStub{failReport: errReport}, errors: []string{errReport.Error()}},
"setup fails": {checkup: checkupStub{failSetup: errSetup}, reporter: reporterStub{}, errors: []string{errSetup.Error()}},
"run fails": {checkup: checkupStub{failRun: errRun}, reporter: reporterStub{}, errors: []string{errRun.Error()}},
"teardown fails": {checkup: checkupStub{failTeardown: errTeardown}, reporter: reporterStub{}, errors: []string{errTeardown.Error()}},
"setup and 2nd report fail": {checkup: checkupStub{failSetup: errSetup}, reporter: reporterStub{failReport: errReport, failOnSecondReport: true}, errors: []string{errSetup.Error(), errReport.Error()}},
"run and report fail": {checkup: checkupStub{failRun: errRun}, reporter: reporterStub{failReport: errReport, failOnSecondReport: true}, errors: []string{errRun.Error(), errReport.Error()}},
"teardown and report fail": {checkup: checkupStub{failTeardown: errTeardown}, reporter: reporterStub{failReport: errReport, failOnSecondReport: true}, errors: []string{errTeardown.Error(), errReport.Error()}},
"run, teardown and report fail": {checkup: checkupStub{failRun: errRun, failTeardown: errTeardown}, reporter: reporterStub{failReport: errReport, failOnSecondReport: true}, errors: []string{errRun.Error(), errTeardown.Error(), errReport.Error()}},
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

Comment on lines 81 to 83
// FIXME: need to decide of we want to return errors in this cases
// errMissingVolumeSnapshotClass = "There are StorageProfiles missing VolumeSnapshotClass."
// errVMsWithNonVirtRbdStorageClass = "There are VMs using the plain RBD storageclass when the virtualization storageclass exists."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Afaiu, we shoud report these cases in the status but not fail the checkup

},
"storageProfileIncomplete": {
clientConfig: clientConfig{spIncomplete: true},
expectedResults: map[string]string{reporter.StorageProfilesWithEmptyClaimPropertySetsKey: testScName, reporter.StorageProfilesWithSpecClaimPropertySetsKey: testScName, reporter.StorageWithRWXKey: ""},
Copy link
Collaborator

@arnongilboa arnongilboa Dec 5, 2023

Choose a reason for hiding this comment

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

Not sure how can a SC be on both EmptyClaimPropertySets and SpecClaimPropertySets? I guess only in UT it's possible :)

Copy link
Collaborator

@arnongilboa arnongilboa left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@arnongilboa
Copy link
Collaborator

@ShellyKa13 please sign the commits

@ShellyKa13
Copy link
Collaborator Author

@ShellyKa13 please sign the commits

They are signed it seems signed commits by github is not the DCO check we do in other repos. I changed the settings.

@ShellyKa13 ShellyKa13 merged commit 69e8fdb into kiagnose:main Dec 7, 2023
8 checks passed
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.

2 participants