add daemonset e2e test #366
Conversation
openshift-ci
bot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: asm582 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/hold for testing on OCP |
openshift-ci
bot
added
the
do-not-merge/hold
| @@ -631,6 +631,119 @@ var _ = Describe("controller", Ordered, func() { | |||
| fmt.Sprintf("%s on node does not match total GPU memory in Instaslice object", controller.QuotaResourceName)) | |||
| } | |||
| }) | |||
There was a problem hiding this comment.
I think you missed the step of Generating fake instaslice object only in case of emulated mode set to true
https://github.com/openshift/instaslice-operator/pull/361/files#diff-d5d12b6ea07b0a78c9ad1987e1486c565ecf16839f77a93e91b09475cf6c0e99R153-R159
There was a problem hiding this comment.
I do it in BeforeEach, will that not work?
There was a problem hiding this comment.
Thanks, I missed that. I thought you were making the change as part of the earlier PR which is closed and missed it here
| } | ||
| err := k8sClient.List(ctx, instasliceObjs, &client.ListOptions{Namespace: namespace}) | ||
| Expect(err).NotTo(HaveOccurred(), "Failed to retrieve Instaslice object") | ||
| referenceLen := len(instasliceObjs.Items[0].Spec.MigGPUUUID) |
There was a problem hiding this comment.
Better to add a condition to check the length of the obtained Instaslice object list and then proceed with accessing the Items
There was a problem hiding this comment.
valid InstaSlice object should always exist, per the BeforeEach contract; I am not sure what we buy with len check.
There was a problem hiding this comment.
I totally agree that the respective objects are present in our testcases. I suggested this change as per the coding guidance/style to avoid unnecessary panics for ex: (may be) lets say we delete instaslice in a new testcase to test how the system works without it. It may not be a big deal/blocking change and the rest looks good to me. Please unhold if you feel the same and keep the code as is.
/lgtm
/hold
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@asm582: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Add ginkgo-based daemonset test cases.
make test, make lint and make test-emulated-gpu and gpu test on kind cluster pass.