-
Notifications
You must be signed in to change notification settings - Fork 109
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: Verify removing partial policy works #717
e2e: Verify removing partial policy works #717
Conversation
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 9567867714Details
💛 - Coveralls |
7903ee8
to
3e43dfe
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 10009093796Details
💛 - Coveralls |
3e43dfe
to
9873c9a
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
9873c9a
to
3de63d6
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work!
var sriovInfos *cluster.EnabledNodes | ||
|
||
BeforeAll(func() { | ||
isSingleNode, err := cluster.IsSingleNode(clients) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the same as in the other file can we just have an internal function that will call BeforeAll?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored to a common BeforeSuite
section. It's the best we can do IMO because these BeforeAll sections modify shared variables. WDYT?
WaitForSRIOVStable() | ||
|
||
By("waiting for the resources to be available") | ||
Eventually(func() int64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not specially to this PR but can you create a function for this we use it in multiple places already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored to a common waitForPodRunning(...)
. Got rid of a lot of code
Thanks for your PR,
To skip the vendors CIs use one of:
|
b0d9977
to
da8a59f
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
da8a59f
to
789af14
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
789af14
to
8032e93
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
8032e93
to
7fa2b8b
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
7fa2b8b
to
774d7ca
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
can you please take a look on the CI they both failed on the same test |
774d7ca
to
0286da5
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
…n file To improve readability, reduce the sizeof `test_sriov_operator.go` by moving part of the test cases to a different file. It's important to keep the same test name, which are composed by the combination of each parent `Context`/`Describe`/`It` clause. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
Add an end-to-end test to verify removing a partial SriovNetworkNodePolicy does not trigger any issue in the operator's deployment. Refs: - https://issues.redhat.com/browse/OCPBUGS-34934 - k8snetworkplumbingwg#710 Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
Move the device discovery and `BeforeAll` checks in the `BeforeSuite` section of the test suite. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
0286da5
to
32123e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
This PR got approval from different sides. Merging |
Add an end-to-end test to verify removing a partial
SriovNetworkNodePolicy does not trigger any issue in
the operator's deployment.
Refs:
This PR also contains a refactor of the conformance test suite