-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
*: add EKS periodic jobs, upgrade "awstester" vendor #9940
Conversation
instead of kubernetes/eks, this probably need to be under kubernetes/sig-foo does eks belongs to sig-aws? |
@krzyzacy @BenTheElder @spiffxp Yeah, I thought about it while I was writing this based on https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes/sig-gcp/sig-gcp-gke-config.yaml. What do you think @d-nishi ? |
Also instead of a postsubmit, I would run it as a periodic job instead. You shouldn't need to checkout any repo? |
kubernetes/kubernetes: | ||
|
||
# Run Kubernetes e2e tests with EKS prod build 1.10 | ||
- name: pull-kubernetes-e2e-tip-aws-eks-1.10-prod |
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.
we use pull-
for presubmits, this should be something like ci-
kubernetes/kubernetes: | ||
|
||
# Run Kubernetes 1.10 branch e2e tests with EKS prod build 1.10 | ||
- name: pull-kubernetes-e2e-1.10-aws-eks-1.10-prod |
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.
Does this need to be a post-submit? What about a periodic / cron job? |
I see. Yes, then we can switch to periodic jobs. Since this is the very beginning to us, we do not want to use all the computing resources for every merge :) Will update this PR shortly. |
@krzyzacy @BenTheElder Addressed. Now these are periodic jobs. PTAL. Thanks! |
Signed-off-by: Gyuho Lee <[email protected]>
Signed-off-by: Gyuho Lee <[email protected]>
I'm fine with the name change cc @d-nishi ! |
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 would make all shared env into one preset. Also maybe you can merge the three jobs into a single yaml file?
preset-service-account: "true" | ||
preset-k8s-ssh: "true" | ||
preset-ci-kubernetes-e2e-1-10-aws-eks-1-10: "true" | ||
|
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.
babynit: unnecessary new line
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.
Removed. Thanks.
name: ci-kubernetes-e2e-1-10-aws-eks-1-10-prod | ||
labels: | ||
preset-service-account: "true" | ||
preset-k8s-ssh: "true" |
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.
you won't need the ssh key as that's for gce
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.
Removed. Thanks.
|
||
spec: | ||
containers: | ||
- image: 607362164682.dkr.ecr.us-west-2.amazonaws.com/awstester-e2e:20181029-141d7d02b498 |
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.
is this image public?
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 will be accessible with the credential that's used here. We can make it public if that's required :)
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.
Here's the Dockerfile for that image https://github.com/aws/awstester/blob/master/scripts/awstester-e2e.Dockerfile.
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.
hummm, wonder how prow will be able to access it as the credential is still in the secret. ImagePulling happens in pod init time.
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.
or we can try and see :-)
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.
@krzyzacy Just listed it as a public image. Let's try! Thanks!
Signed-off-by: Gyuho Lee <[email protected]>
Signed-off-by: Gyuho Lee <[email protected]>
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.
/lgtm
LGTM label has been added. Git tree hash: 7eb26c82a316e6c2d27ce735b5c610fb0da6efaf
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gyuho, krzyzacy 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 |
@gyuho: Updated the
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. |
ref. #9814
/cc @krzyzacy @d-nishi