kubetest: add EKS plugin #9811

gyuho · 2018-10-15T20:39:45Z

https://github.com/aws/awstester implements

Lines 223 to 230 in 9a88e32

    
           type deployer interface { 
        
           	Up() error 
        
           	IsUp() error 
        
           	DumpClusterLogs(localPath, gcsPath string) error 
        
           	TestSetup() error 
        
           	Down() error 
        
           	GetClusterCreated(gcpProject string) (time.Time, error) 
        
           }

Example Prow configuration for ALB Ingress Controller:

presets:
- env:
  - name: AWSTESTER_EKS_KUBETEST_EMBEDDED_BINARY
    value: "true"
  - name: AWSTESTER_EKS_WAIT_BEFORE_DOWN
    value: 1m0s
  - name: AWSTESTER_EKS_DOWN
    value: "true"
  - name: AWSTESTER_EKS_ENABLE_NODE_SSH
    value: "true"
  - name: AWSTESTER_EKS_AWSTESTER_IMAGE
    value: 607362164682.dkr.ecr.us-west-2.amazonaws.com/awstester:180919694a39
  - name: AWSTESTER_EKS_WORKER_NODE_INSTANCE_TYPE
    value: m3.xlarge
  - name: AWSTESTER_EKS_WORKER_NODE_ASG_MIN
    value: "1"
  - name: AWSTESTER_EKS_WORKER_NODE_ASG_MAX
    value: "1"
  - name: AWSTESTER_EKS_ALB_ENABLE
    value: "true"
  - name: AWSTESTER_EKS_ALB_ALB_INGRESS_CONTROLLER_IMAGE
    value: quay.io/coreos/alb-ingress-controller:1.0-beta.7
  - name: AWSTESTER_EKS_ALB_TARGET_TYPE
    value: instance
  - name: AWSTESTER_EKS_ALB_TEST_MODE
    value: nginx
  - name: AWSTESTER_EKS_ALB_TEST_SCALABILITY
    value: "true"
  - name: AWSTESTER_EKS_ALB_TEST_SERVER_REPLICAS
    value: "1"
  - name: AWSTESTER_EKS_ALB_TEST_SERVER_ROUTES
    value: "1"
  - name: AWSTESTER_EKS_ALB_TEST_CLIENTS
    value: "20"
  - name: AWSTESTER_EKS_ALB_TEST_CLIENT_REQUESTS
    value: "2000"
  - name: AWSTESTER_EKS_ALB_TEST_RESPONSE_SIZE
    value: "40960"
  - name: AWSTESTER_EKS_ALB_TEST_CLIENT_ERROR_THRESHOLD
    value: "500"
  - name: AWSTESTER_EKS_ALB_TEST_EXPECT_QPS
    value: "100.000000"
  - name: AWS_SHARED_CREDENTIALS_FILE
    value: /etc/aws-cred-awstester/aws-cred-awstester
  labels:
    preset-e2e-aws-alb-ingress-controller: "true"
  volumeMounts:
  - mountPath: /etc/aws-cred-awstester
    name: aws-cred-awstester
    readOnly: true
  volumes:
  - name: aws-cred-awstester
    secret:
      secretName: aws-cred-awstester

presubmits:
  gyuho/aws-alb-ingress-controller:
  - agent: kubernetes
    always_run: true
    context: test-presubmit
    labels:
      preset-dind-enabled: "true"
      preset-e2e-aws-alb-ingress-controller: "true"
    name: pull-ingress-aws-alb-build
    rerun_command: /test pull-ingress-aws-alb-build
    run_after_success:
    - agent: kubernetes
      always_run: true
      context: test-presubmit
      labels:
        preset-dind-enabled: "true"
        preset-e2e-aws-alb-ingress-controller: "true"
      name: pull-ingress-aws-alb-e2e
      rerun_command: /test pull-ingress-aws-alb-e2e
      spec:
        containers:
        - command:
          - tests/alb-e2e.sh
          image: 607362164682.dkr.ecr.us-west-2.amazonaws.com/awstester:db7d4d8ed97c
          name: ""
          resources: {}
          securityContext:
            privileged: true
      trigger: (?m)^/test pull-ingress-aws-alb-e2e
    spec:
      containers:
      - command:
        - /bin/echo
        - pull-ingress-aws-alb-build
        image: alpine
        name: ""
        resources: {}
        securityContext:
          privileged: true
    trigger: (?m)^/test pull-ingress-aws-alb-build

The configuration above has been working well with our internal Prow cluster (see https://github.com/gyuho/aws-alb-ingress-controller/pull/3 for example output).

Now, we would like to integrate with upstream.

Action items (with following PRs):

Change DumpClusterLogs and Publish methods to use upstream Google Cloud Storage?
Mount AWS test account credential to upstream Prow cluster
Figure out how to use "dind" for building aws-alb-ingress-controller images
- Just like containerd-cri-presubmit-jobs.yaml
Add Prow job for aws-alb-ingress-controller
Add aws-alb-ingress-controller tests to upstream test grid
Add Prow job for aws-ebs-csi-driver
Add aws-ebs-csi-driver tests to upstream test grid
Add Prow job for cloud-provider-aws
Add cloud-provider-aws tests to upstream test grid
Add pull-kubernetes-e2e-aws-eks Prow job

k8s-ci-robot · 2018-10-15T20:39:47Z

Welcome @gyuho! It looks like this is your first PR to kubernetes/test-infra 🎉🎉

krzyzacy · 2018-10-16T18:18:21Z

/assign
remove WIP when you think this is ready for review

gyuho · 2018-10-16T18:30:26Z

@krzyzacy Please review :) And let me know if you have any feedback!

krzyzacy · 2018-10-17T04:07:41Z

kubetest/eks/eks.go

+}
+
+var (
+	timeout = 3 * time.Hour


kubetest has a timeout flag, just pass it down?

krzyzacy · 2018-10-17T04:07:54Z

kubetest/eks/eks.go

+
+var (
+	timeout = 3 * time.Hour
+	verbose = true


(also verbose flag)

krzyzacy · 2018-10-17T04:10:53Z

kubetest/eks/eks.go

+	return tr.cfg.ClusterState.Created, nil
+}
+
+// DumpClusterLogs uploads local cluster logs to S3.


humm our infra already supports upload logs to gcs, which works with gubernator & testgrid etc.. I would dump the log to local $ARTIFACTS and let our tooling layer handle that, otherwise you can also choose to support make test-infra handle artifacts from s3.

We want to use the GCS first. And then migrate later, if needed. Will change the code to dump logs to $ARTIFACTS, and address other comments within this week.

choose to support make test-infra handle artifacts from s3

Sounds good. We can write some proposal later to do this.

Thanks!

krzyzacy · 2018-10-19T04:48:11Z

kubetest/eks/eks.go

+func (tr *tester) DumpClusterLogs(localPath, s3Path string) error {
+	if !tr.cfg.KubetestEnableDumpClusterLogs {
+		// let default kubetest log dumper handle all artifact uploads.
+		// See https://github.com/kubernetes/test-infra/pull/9811/files#r225776067.


our util will handle your pod logs, but you'll still need to generate things like kubelet logs and master logs somehow :-)

@justinsb already wrote https://github.com/kubernetes/test-infra/blob/master/kubetest/dump.go for kops-aws, maybe you can utilize that?

@krzyzacy I see. Thanks for the pointer. I will try to integrate the log dumper by today!

krzyzacy · 2018-10-19T04:49:22Z

kubetest/main.go

+		cfg.KubetestControlTimeout = timeout
+		cfg.KubetestVerbose = verbose
+		// always let default log dumper handle artifacts
+		cfg.KubetestEnableDumpClusterLogs = false


similar to above, I think we should respect the --dump flag, and skip dump if that's set to false

krzyzacy · 2018-10-19T04:58:38Z

kubetest/eks/eks.go

+			time.NewTimer(tr.cfg.KubetestControlTimeout),
+			time.NewTimer(tr.cfg.KubetestControlTimeout),
+			tr.cfg.KubetestVerbose,
+		)


why not pass in control from main.go and you can avoid creating new control for all the calls?

krzyzacy · 2018-10-19T04:58:45Z

/lint

k8s-ci-robot

@krzyzacy: 0 warnings.

In response to this:

/lint

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.

gyuho · 2018-10-22T16:41:01Z

@krzyzacy All addressed. PTAL. Thanks!

krzyzacy

also add an OWNERS file for kubetest/eks, put people from your project as reviewers?

krzyzacy · 2018-10-24T18:11:17Z

kubetest/main.go

@@ -246,6 +248,23 @@ func getDeployer(o *options) (deployer, error) {
 		return dind.NewDeployer(o.kubecfg, o.dindImage, control)
 	case "gke":
 		return newGKE(o.provider, o.gcpProject, o.gcpZone, o.gcpRegion, o.gcpNetwork, o.gcpNodeImage, o.gcpImageFamily, o.gcpImageProject, o.cluster, &o.testArgs, &o.upgradeArgs)
+	case "eks":
+		cfg := eksconfig.NewDefault()


I feel like all the eksconfig call can happen inside the deployer, so main won't have eks dependencies. wdyt

(In the future we will want to provide a deployer register function, and you can move all deployer codes to your k-sigs repo)

you can move all deployer codes to your k-sigs repo

This sounds awesome!

Yes, let me move everything to "kubetest/eks" package, so main.go does not import anything other than "kubetest/eks".

krzyzacy · 2018-10-24T18:18:56Z

kubetest/eks/eks.go

+	return err
+}
+
+func (tr *tester) TestALBMetrics() (err error) {


also those test functions are not used? plans?

leakingtapan · 2018-10-24T20:22:32Z

kubetest/eks/eks.go

+}
+
+func (tr *tester) GetWorkerNodeLogs() error {
+	tr.LoadConfig()


Im a bit confused about why tr.LoadConfig() is called each time in GetWorkerNodeLogs and DumpClusterLogs but not in other methods?

leakingtapan · 2018-10-24T20:24:05Z

kubetest/eks/eks.go

+	return err
+}
+
+func (tr *tester) Stop() {


Where is Stop() used? Or how will it be used in future?

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho · 2018-10-24T21:51:12Z

PTAL.

@krzyzacy All addressed: only kept the ones needed for deployer interface (TestALBMetrics won't be used inside kubetest repository so had it defined in the other interface), moved everything into eks package, added OWNER file with @leakingtapan.

@leakingtapan Added godoc comments to explain LoadConfig and Stop methods.

Thanks!

leakingtapan · 2018-10-24T22:01:45Z

/lgtm

k8s-ci-robot · 2018-10-24T22:01:47Z

LGTM label has been added.

Git tree hash: b2066b7bba70e601fe2f28b4c127208a73899f28

krzyzacy · 2018-10-24T22:03:23Z

/lint

k8s-ci-robot

@krzyzacy: 0 warnings.

In response to this:

/lint

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.

krzyzacy · 2018-10-24T22:05:34Z

/lgtm
/this-is-unbearable

k8s-ci-robot · 2018-10-24T22:05:35Z

@krzyzacy:

In response to this:

/lgtm
/this-is-unbearable

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.

k8s-ci-robot · 2018-10-24T22:05:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gyuho, krzyzacy, leakingtapan

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:

~~OWNERS~~ [krzyzacy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 15, 2018

k8s-ci-robot requested review from amwat and spiffxp October 15, 2018 20:39

gyuho mentioned this pull request Oct 15, 2018

kubetest: add "EKS" provider #9814

Closed

12 tasks

k8s-ci-robot assigned krzyzacy Oct 16, 2018

gyuho changed the title ~~[WIP do not merge] kubetest: add EKS plugin~~ kubetest: add EKS plugin Oct 16, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 16, 2018

krzyzacy reviewed Oct 17, 2018

View reviewed changes

kubetest/eks/eks.go Outdated

}

var (

timeout = 3 * time.Hour

Copy link

Member

krzyzacy Oct 17, 2018

Choose a reason for hiding this comment

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

kubetest has a timeout flag, just pass it down?

krzyzacy reviewed Oct 17, 2018

View reviewed changes

kubetest/eks/eks.go Outdated

var (

timeout = 3 * time.Hour

verbose = true

Copy link

Member

krzyzacy Oct 17, 2018

Choose a reason for hiding this comment

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

(also verbose flag)

krzyzacy reviewed Oct 17, 2018

View reviewed changes

krzyzacy reviewed Oct 19, 2018

View reviewed changes

k8s-ci-robot reviewed Oct 19, 2018

View reviewed changes

krzyzacy reviewed Oct 24, 2018

View reviewed changes

leakingtapan reviewed Oct 24, 2018

View reviewed changes

kubetest/eks/eks.go Outdated

return err

}

func (tr *tester) Stop() {

Copy link

leakingtapan Oct 24, 2018

Choose a reason for hiding this comment

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

Where is Stop() used? Or how will it be used in future?

gyuho added 4 commits October 24, 2018 14:44

kubetest/eks: initial commit

44d0986

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

kubetest/eks: add "OWNERS" file

c28dc29

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

kubetest: add "eks" deployer

7891d06

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

*: vendor "github.com/aws/awstester"

1a5858a

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

k8s-ci-robot assigned leakingtapan Oct 24, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 24, 2018

k8s-ci-robot reviewed Oct 24, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2018

k8s-ci-robot merged commit 095d311 into kubernetes:master Oct 24, 2018

gyuho deleted the eks-plugin branch October 24, 2018 22:18

dims mentioned this pull request Apr 28, 2023

[aws] Need a new AWS account to replace 607362164682 kubernetes/k8s.io#5194

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubetest: add EKS plugin #9811

kubetest: add EKS plugin #9811

gyuho commented Oct 15, 2018 •

edited

Loading

k8s-ci-robot commented Oct 15, 2018

krzyzacy commented Oct 16, 2018

gyuho commented Oct 16, 2018

krzyzacy Oct 17, 2018

krzyzacy Oct 17, 2018

krzyzacy Oct 17, 2018

gyuho Oct 17, 2018

krzyzacy Oct 19, 2018

gyuho Oct 19, 2018

krzyzacy Oct 19, 2018

krzyzacy Oct 19, 2018

krzyzacy commented Oct 19, 2018

k8s-ci-robot left a comment

gyuho commented Oct 22, 2018

krzyzacy left a comment

krzyzacy Oct 24, 2018

gyuho Oct 24, 2018

krzyzacy Oct 24, 2018

leakingtapan Oct 24, 2018

leakingtapan Oct 24, 2018

gyuho commented Oct 24, 2018

leakingtapan commented Oct 24, 2018

k8s-ci-robot commented Oct 24, 2018

krzyzacy commented Oct 24, 2018

k8s-ci-robot left a comment

krzyzacy commented Oct 24, 2018

k8s-ci-robot commented Oct 24, 2018

k8s-ci-robot commented Oct 24, 2018

	type deployer interface {
	Up() error
	IsUp() error
	DumpClusterLogs(localPath, gcsPath string) error
	TestSetup() error
	Down() error
	GetClusterCreated(gcpProject string) (time.Time, error)
	}

kubetest: add EKS plugin #9811

kubetest: add EKS plugin #9811

Conversation

gyuho commented Oct 15, 2018 • edited Loading

k8s-ci-robot commented Oct 15, 2018

krzyzacy commented Oct 16, 2018

gyuho commented Oct 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzyzacy commented Oct 19, 2018

k8s-ci-robot left a comment

Choose a reason for hiding this comment

gyuho commented Oct 22, 2018

krzyzacy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gyuho commented Oct 24, 2018

leakingtapan commented Oct 24, 2018

k8s-ci-robot commented Oct 24, 2018

krzyzacy commented Oct 24, 2018

k8s-ci-robot left a comment

Choose a reason for hiding this comment

krzyzacy commented Oct 24, 2018

k8s-ci-robot commented Oct 24, 2018

k8s-ci-robot commented Oct 24, 2018

gyuho commented Oct 15, 2018 •

edited

Loading