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

E2E testcases for role,clusterrole,rolebinding,clusterrolebinding,secret,service,storageclass resources for backup and restore feature #116

Merged
merged 3 commits into from
Nov 28, 2022

Conversation

vineela1999
Copy link
Contributor

@vineela1999 vineela1999 commented Nov 25, 2022

What type of PR is this?
kind new feature

What this PR does / why we need it:
This PR contains E2E testcases for role,clusterrole,rolebinding,clusterrolebinding,secret,service,storageclass resources for backup and restore feature using ginkgo framework

Which issue(s) this PR fixes:

Fixes #

Test Report Added?:
kind TESTED

Test Report:

Screenshot from 2022-11-25 23-44-05

Special notes for your reviewer:

@sushanthakumar
Copy link
Collaborator

Please add test scripts running snapshot


//testcase for E2E deployment backup and restore
var _ = Describe("rbacBackup", Label("rbac"), func() {
Describe("rbacE2ETest", func() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, Describe section can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"github.com/soda-cdm/kahu/controllers/app/options"
k8s "github.com/soda-cdm/kahu/test/e2e/util/k8s"
kahu "github.com/soda-cdm/kahu/test/e2e/util/kahu"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please maintain logical grouping of import statements


optManager, err := options.NewOptionsManager()
Expect(err).To(BeNil())
if err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, 'err' is already checked with asserts.
Please handle other places as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

//Create rbac to test
ns := "default"
ctx := context.TODO()
UUIDgen, _ := uuid.NewRandom()
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, Its better add assets for err here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

kahuClient, err := f.KahuClient()

//Create rbac to test
ns := "default"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please initialise namespace constants/variable in setup and cleanup in teardown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

_, err = kahuClient.KahuV1beta1().Backups().Create(ctx, backup, opts)
Expect(err).To(BeNil())
log.Infof("backup of rbac is done\n")
time.Sleep(40 * time.Second)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe, Wait/poll for backup to finish is little better than sleep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Expect(err).To(BeNil())

err = k8s.DeletePod(ctx, kubeClient, nsRestore, podName)
Expect(err).To(BeNil())
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, If all operation are done in a separate namespace. Deleting namespace will handle deleting all other resources

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Signed-off-by: vineela1999 <vineelapachipulusu@gmail.com>
Copy link
Collaborator

@sushanthakumar sushanthakumar left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@AmitRoushan AmitRoushan left a comment

Choose a reason for hiding this comment

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

LGTM

@AmitRoushan AmitRoushan merged commit 4cd7a43 into soda-cdm:development Nov 28, 2022
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.

None yet

3 participants