Add e2e tests for neg naming and neg crd#1207
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @swetharepakula. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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/test-infra repository. |
k8s-ci-robot
added
the
size/XL
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
swetharepakula
force-pushed
the
neg-crd-e2e
branch
from
6662cd4 to
0ef8853
Compare
cmd/e2e-test/neg_test.go
Outdated
| }) | ||
| } | ||
|
|
||
| func TestEnableNegCRD(t *testing.T) { |
There was a problem hiding this comment.
Suggestion: TestNegCRDTransitions
pkg/e2e/helpers.go
Outdated
| } | ||
|
|
||
| // CheckDeletedNegCRs verifies that the provided neg list does not have negs that are associated with the provided neg atrributes | ||
| func CheckDeletedNegCRs(negs *negv1beta1.ServiceNetworkEndpointGroupList, deletedNegAttrs, negConfigs map[string]string) bool { |
pkg/e2e/helpers.go
Outdated
| } | ||
|
|
||
| // WaitForStandaloneNegDeletion waits for all NEGs associated with a GCLB to be deleted via GC | ||
| func WaitForStandaloneNegDeletion(ctx context.Context, c cloud.Cloud, s *Sandbox, expectedDeletedNegCRs, negConfigs map[string]string, zones []string, expectDeletion bool) error { |
There was a problem hiding this comment.
negConfigs => map port -> negName
expectedDeleteNegCRs => map port -> name
cmd/e2e-test/neg_test.go
Outdated
| }) | ||
| } | ||
|
|
||
| func TestNegCRDUserActions(t *testing.T) { |
There was a problem hiding this comment.
E2e test is quite heavy. I would recommend not adding e2e test for negative cases.
Unless, it is a very important negative case.
Let us discuss if this is worth keeping
cmd/e2e-test/neg_test.go
Outdated
| desc string | ||
| annotations annotations.NegAnnotation | ||
| expectedNegAttrs map[string]string | ||
| gcNegAttrs map[string]string |
There was a problem hiding this comment.
might be worth adding another parameter: replicas?
I think WaitForNegs should already check the number of endpoints in the NEGs.
pkg/e2e/helpers.go
Outdated
| } | ||
|
|
||
| // CheckCustomNegNameStatus checks if the NEG Status annotation is present and in the expected state | ||
| func CheckCustomNegNameStatus(svc *v1.Service, expectedNegAttrs map[string]string) (annotations.NegStatus, error) { |
There was a problem hiding this comment.
add a comment about expectedNegAttrs parameter
pkg/e2e/helpers.go
Outdated
| return nil | ||
| } | ||
|
|
||
| // WaitForStandaloneNegDeletion waits for all NEGs associated with a GCLB to be deleted via GC |
There was a problem hiding this comment.
// WaitForStandaloneNegDeletion waits for standalone NEGs and corresponding CRDs are deleted via GC.
There was a problem hiding this comment.
add comments about expectedDelteNegsCRs, negConfigs
consider:
- pass negConfigs and zones together as pastNegStatus.
- pass the NEG name which expected to be deleted. So that making the function simpler only validate if one NEG and CRD has been delete .
swetharepakula
force-pushed
the
neg-crd-e2e
branch
from
0ef8853 to
36d7216
Compare
k8s-ci-robot
added
size/L
swetharepakula
force-pushed
the
neg-crd-e2e
branch
from
36d7216 to
ed31781
Compare
swetharepakula
force-pushed
the
neg-crd-e2e
branch
from
ed31781 to
1bae87d
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: freehan, swetharepakula The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
3 participants
No description provided.