Skip to content

Simplify prow configs by removing options with good defaults#982

Merged
sebastienvas merged 3 commits intoistio:masterfrom
sebastienvas:update_k8s_vendor
Sep 27, 2018
Merged

Simplify prow configs by removing options with good defaults#982
sebastienvas merged 3 commits intoistio:masterfrom
sebastienvas:update_k8s_vendor

Conversation

@sebastienvas
Copy link
Contributor

trigger, context, re_run_command and agent have good default now.

  • Default Trigger and RerunCommand to match /test .
  • Default Context to the the job's Name.
  • Default Agent to kubernetes.

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: sebastienvas

If they are not already assigned, you can assign the PR to them by writing /assign @sebastienvas in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Sep 26, 2018
@sebastienvas
Copy link
Contributor Author

cc @cjwagner

@sebastienvas sebastienvas changed the title Simplify configs with good default Simplify prow configs by removing options with good defaults Sep 26, 2018

- name: daily-unit-tests
agent: kubernetes
context: prow/daily-unit-tests.sh
Copy link
Member

Choose a reason for hiding this comment

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

This will change the context name from prow/daily-unit-tests.sh to the default of daily-unit-tests. You probably don't want to change the context name as that would require a status context migration and changing repo required context settings.
I would leave the context field as is for now and just drop the other fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its automatic we have branch protection set up. but yes I would prefer something like "prow: daily-unit-tests" to be consistent with circle-ci. I'll take your suggestion and do the rename of context in a different PR.

Copy link
Member

Choose a reason for hiding this comment

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

Branch protection definitely makes things easier, but you may still need to deprecate any existing statuses for the old context to avoid confusing users. We have this tool for that: https://github.com/kubernetes/test-infra/tree/master/maintenance/migratestatus

context: prow/daily-e2e-bookinfoTests.sh
branches: *branch_spec
always_run: true
rerun_command: "/test daily-e2e-bookinfo"
Copy link
Member

Choose a reason for hiding this comment

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

This will now default to /test daily-e2e-bookinfoTests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noted

context: prow/daily-e2e-simpleTests.sh
branches: *branch_spec
always_run: true
rerun_command: "/test daily-e2e-simple"
Copy link
Member

Choose a reason for hiding this comment

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

This will default to /test daily-e2e-simpleTests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that s fine. Prow will tell people what to do on the PR

@cjwagner
Copy link
Member

I only reviewed the first few files, but the comments I left should be applicable to them all.

@sebastienvas sebastienvas merged commit e91e4cc into istio:master Sep 27, 2018
@istio-testing
Copy link
Collaborator

@sebastienvas: Updated the job-config configmap using the following files:

  • key istio.istio.release-1.0.yaml using file prow/cluster/jobs/istio/istio/istio.istio.release-1.0.yaml
  • key istio.proxy.sds_api.yaml using file prow/cluster/jobs/istio/proxy/istio.proxy.sds_api.yaml
  • key istio-releases.daily-release.release-0.8.yaml using file prow/cluster/jobs/istio-releases/daily-release/istio-releases.daily-release.release-0.8.yaml
  • key istio-releases.daily-release.release-1.0.yaml using file prow/cluster/jobs/istio-releases/daily-release/istio-releases.daily-release.release-1.0.yaml
  • key all-periodics.yaml using file prow/cluster/jobs/all-periodics.yaml
  • key istio-releases.daily-release.master.yaml using file prow/cluster/jobs/istio-releases/daily-release/istio-releases.daily-release.master.yaml
  • key istio.istio.collab-gcp-identity.yaml using file prow/cluster/jobs/istio/istio/istio.istio.collab-gcp-identity.yaml
  • key istio.istio.release-0.8.yaml using file prow/cluster/jobs/istio/istio/istio.istio.release-0.8.yaml
  • key istio.proxy.release-1.0.yaml using file prow/cluster/jobs/istio/proxy/istio.proxy.release-1.0.yaml
  • key istio-releases.daily-release.collab-gcp-identity.yaml using file prow/cluster/jobs/istio-releases/daily-release/istio-releases.daily-release.collab-gcp-identity.yaml
  • key istio.api.master.yaml using file prow/cluster/jobs/istio/api/istio.api.master.yaml
  • key istio.api.release-0.8.yaml using file prow/cluster/jobs/istio/api/istio.api.release-0.8.yaml
  • key istio.istio.master.yaml using file prow/cluster/jobs/istio/istio/istio.istio.master.yaml
  • key istio.proxy.master.yaml using file prow/cluster/jobs/istio/proxy/istio.proxy.master.yaml
  • key istio.test-infra.master.yaml using file prow/cluster/jobs/istio/test-infra/istio.test-infra.master.yaml
Details

In response to this:

trigger, context, re_run_command and agent have good default now.

  • Default Trigger and RerunCommand to match /test .
  • Default Context to the the job's Name.
  • Default Agent to kubernetes.

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.

@sebastienvas sebastienvas deleted the update_k8s_vendor branch September 27, 2018 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants