-
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
Adding PR jobs for kube-deploy #6761
Conversation
/hold It should be noted I have no idea what I'm doing. 😄. Is there a way to have the test job run, but not block merge? We need to fix tests. |
prow/config.yaml
Outdated
trigger: "/test( all| pull-kube-deploy-build)" | ||
spec: | ||
containers: | ||
- image: gcr.io/k8s-testimages/gcloud-in-go:v20171113-192bec25 |
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 just picked an image that seemed like it might work. I don't know how to test this. The only thing these jobs depend on is go 1.9+
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.
ref https://github.com/kubernetes/test-infra/blob/master/images/gcloud/Dockerfile, yeah it's go 1.9.3 with an ancient gcloud
/test pull-test-infra-verify-spelling |
Needed to rebase to pick up the spelling test script. |
/skip |
wanna add them to testgrid as well? (add your repo here) |
@krzyzacy Sure I can add test-grid as well. I assume that I would have to create periodic jobs as well? Also, is there a way to run the tests on PRs but not have them block? (we have failures we need to fix right now) |
@krousey how does merge work in your repo? |
Guessing you are using |
@krzyzacy Done. Should I add periodic CI jobs too? and what about alerting? I'm unsure what that does. |
@krousey up to you if you want to add a CI job or not. alert will send you email alerts when your job fails x times in a row |
ab9c183
to
c479b8f
Compare
Fixed latest test errors and added CI jobs too. |
prow/config.yaml
Outdated
@@ -4935,6 +4965,29 @@ postsubmits: | |||
- name: docker-graph | |||
hostPath: | |||
path: /mnt/disks/ssd0/docker-graph | |||
|
|||
kubernetes/kube-deploy: |
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 add them as periodic jobs rather than postsubmits?
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.
Ok. What is the purpose of post-submits? Just stuff you don't want to run on every commit in every PR?
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.
It's the opposite right? Postsubmits are the stuff you do want to run on every commit in every PR :-)
there are jobs like config updater, need to run right after a push event...
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.
What does this job do exactly? post-submits are best used for things like uploading a new config after the config change is merged, IE some post-merge publish step.
Continuous tests are probably best as periodics and/or presubmits.
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.
Got it. It's just CI builds... periodic it is.
/hold cancel |
prow/config.yaml
Outdated
containers: | ||
- image: gcr.io/k8s-testimages/gcloud-in-go:v20171113-192bec25 | ||
args: | ||
- "--repo=k8s.io/$(REPO_NAME)" |
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.
periodics don't have $(REPO_NAME), so specify your checkout path :-)
and also add labels: preset-service-account: true
for log/aftifacts upload stuff
fbbc3e2
to
65d9e26
Compare
@krousey: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
testgrid/config.yaml
Outdated
@@ -156,6 +156,10 @@ test_groups: | |||
gcs_prefix: kubernetes-jenkins/logs/ci-ingress-gce-upgrade-e2e | |||
- name: ci-ingress-gce-downgrade-e2e | |||
gcs_prefix: kubernetes-jenkins/logs/ci-ingress-gce-downgrade-e2e | |||
- name: ci-kube-deploy-build | |||
gcs_prefix: kubernetes-jenkins/logs/ci-kube-deploy-build | |||
- name: ci-kube-deploy-build |
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.
s/build/test
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.
Done
I think you'll also need to enable trigger in https://github.com/kubernetes/test-infra/blob/master/prow/plugins.yaml#L188-L190 cc @cjwagner sounds like we want a unit-test to ensure all repo has prow presubmits has trigger enabled? otherwise lgtm |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: krousey, 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 OWNERS Files:
Approvers can indicate their approval by writing |
@krousey: Updated the following 2 configmaps:
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. |
cc @kubernetes/kube-deploy-maintainers