Enable e2e testing on EKS, parallelism, and make optional EFS creation/driver deployment #160
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wongma7 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 |
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
the
size/L
| switch t := err.(type) { | ||
| case *efs.FileSystemAlreadyExists: |
There was a problem hiding this comment.
Could you help me understand what does case *efs.FileSystemAlreadyExists do?
type FileSystemAlreadyExists struct {
_ struct{} `type:"structure"`
RespMetadata protocol.ResponseMetadata `json:"-" xml:"-"`
// ErrorCode is a required field
ErrorCode *string `min:"1" type:"string" required:"true"`
// FileSystemId is a required field
FileSystemId *string `type:"string" required:"true"`
Message_ *string `locationName:"Message" type:"string"`
}
There was a problem hiding this comment.
Error() is an an implementation to the pointer receiver of FileSystemAlreadyExists.
func (s *FileSystemAlreadyExists) Error() string {
| groupId, err := c.getKopsSecurityGroup(clusterName) | ||
| if err != nil { | ||
| fmt.Printf("error getting kops node security group: %v\n", err) | ||
| } else { | ||
| return groupId, nil | ||
| } | ||
|
|
||
| groupid, err := c.getEksSecurityGroup(clusterName) |
There was a problem hiding this comment.
Consider running them in parallel.
| for _, sg := range response.SecurityGroups { | ||
| if strings.Contains(strings.ToLower(*sg.GroupName), "node") { | ||
| groupId = aws.StringValue(sg.GroupId) | ||
| } | ||
| } |
| var _ = ginkgo.SynchronizedBeforeSuite(func() []byte { | ||
| // Validate parameters |
There was a problem hiding this comment.
For my notes:
SynchronizedBeforeSuite blocks are primarily meant to solve the problem of setting up singleton external resources shared across nodes when running tests in parallel. For example, say you have a shared database that you can only start one instance of that must be used in your tests. When running in parallel, only one node should set up the database and all other nodes should wait until that node is done before running.
SynchronizedBeforeSuite accomplishes this by taking two function arguments. The first is only run on parallel node #1. The second is run on all nodes, but only after the first function completes successfully. Ginkgo also makes it possible to send data from the first function (on Node 1) to the second function (on all the other nodes).
https://godoc.org/github.com/onsi/ginkgo#SynchronizedBeforeSuite
|
@jqmichael: changing LGTM is restricted to collaborators In response to this:
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/test-infra repository. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Is this a bug fix or adding new feature? fix
What is this PR about? / Why do we need it? When I tried to run the tests on EFS in parallel I ran into a couple of issues. This is the sum of fixes I needed to make to get the tests to pass.
EFS creation queries kops-specific tags, so change cloud.go a bit to also check for EKS-style tags.
Since multiple tests can be run if I run by doing
ginkgo -p, I found it's best to move EFS creation/driver deployment to SynchronizedBefore/AfterSuite. Moreover, I made it optional since outside of the CI environment there may be an existing EFS/driver that the test can use.What testing is done? I ran the tests against my EKS cluster.