Skip to content
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.

Sign up for GitHub

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

Jump to bottom

kubernetes-csi: split up jobs into unit/alpha/non-alpha #12088

Merged
merged 3 commits into from Apr 8, 2019
Merged

kubernetes-csi: split up jobs into unit/alpha/non-alpha #12088

merged 3 commits into from Apr 8, 2019

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Apr 5, 2019 •
edited

This is meant to help developers who look at tests results understand
more quickly what the root cause of a failure might be because they
don't need to look at the individual test name.

Splitting up the pre-merge jobs also speeds up providing feedback on a
PR, because jobs complete faster.

Naming of jobs is a bit more consistent now, with "kubernetes-csi"
being used also for pre-merge jobs. csi-driver-host-path
now has:

pull-kubernetes-csi-csi-driver-host-path-1-13-on-kubernetes-1-13
pull-kubernetes-csi-csi-driver-host-path-1-13-on-kubernetes-master
pull-kubernetes-csi-csi-driver-host-path-1-14-on-kubernetes-1-14
pull-kubernetes-csi-csi-driver-host-path-1-14-on-kubernetes-master
pull-kubernetes-csi-csi-driver-host-path-alpha-1-13-on-kubernetes-1-13 (*)
pull-kubernetes-csi-csi-driver-host-path-alpha-1-13-on-kubernetes-master (*)
pull-kubernetes-csi-csi-driver-host-path-alpha-1-14-on-kubernetes-1-14 (*)
pull-kubernetes-csi-csi-driver-host-path-alpha-1-14-on-kubernetes-master (*)
pull-kubernetes-csi-csi-driver-host-path-unit (*)

ci-kubernetes-csi-1-13-on-kubernetes-1-13
ci-kubernetes-csi-1-13-on-kubernetes-1-14
ci-kubernetes-csi-1-13-on-kubernetes-master
ci-kubernetes-csi-alpha-1-13-on-kubernetes-1-13 (*)
ci-kubernetes-csi-canary-on-kubernetes-1-13
ci-kubernetes-csi-canary-on-kubernetes-1-14
ci-kubernetes-csi-canary-on-kubernetes-master
ci-kubernetes-csi-alpha-canary-on-kubernetes-master (*)

Jobs marked (*) with are new, other others have been changed to not
run alpha tests.

The "liveness-probe" pre-merge tests were limited to run only on one
Kubernetes version because there is no real dependency on Kubernetes
APIs.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 5, 2019
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/config Issues or PRs related to code in /config area/testgrid sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Apr 5, 2019
@pohly pohly mentioned this pull request Apr 5, 2019
Merged
msau42
msau42 reviewed Apr 5, 2019
View reviewed changes
config/jobs/kubernetes-csi/csi-driver-host-path/csi-driver-host-path-config.yaml Outdated
resources:
requests:
cpu: 2000m
- name: pull-kubernetes-csi-csi-driver-host-path-alpha-1-13-on-kubernetes-master
Copy link
Member

Choose a reason for hiding this comment

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

Alpha suites should not run cross versions

Copy link
Member

Choose a reason for hiding this comment

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

I guess for an alpha pull job, we should only have master->master

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alpha suites should not run cross versions.

They can for known configurations (like 1.13 on 1.14), because for those we can configure which tests are expected to work. But if we don't want that, we can also leave those alpha tests optional.

For all -on-kubernetes-master jobs the intention was to never make these tests mandatory pre-submit checks, because there's no guarantee that this check doesn't get broken by changes in master, see

test-infra/config/jobs/kubernetes-csi/gen-jobs.sh

Lines 204 to 206 in 04c6ebf

# Experimental job, explicitly needs to be started with /test.
# This cannot be enabled by default because there's always the risk
# that something changes in master which breaks the pre-merge check.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should encourage providing more alpha support than our policy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I doubt that many people besides the core team will know that these jobs are possible, so I'm not sure who or what that would encourage. But I don't have a strong opinion about those, so I can also take them out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Also fixed some settings for the canary jobs.

@pohly
kubernetes-csi: split up jobs into unit/alpha/non-alpha
56cfc7f 
This is meant to help developers who look at tests results understand
more quickly what the root cause of a failure might be because they
don't need to look at the individual test name.

Splitting up the pre-merge jobs also speeds up providing feedback on a
PR, because jobs complete faster.

Naming of jobs is a bit more consistent now, with "kubernetes-csi"
being used also for pre-merge jobs. csi-driver-host-path
now has:

pull-kubernetes-csi-csi-driver-host-path-1-13-on-kubernetes-1-13
pull-kubernetes-csi-csi-driver-host-path-1-13-on-kubernetes-master
pull-kubernetes-csi-csi-driver-host-path-1-14-on-kubernetes-1-14
pull-kubernetes-csi-csi-driver-host-path-1-14-on-kubernetes-master
pull-kubernetes-csi-csi-driver-host-path-alpha-1-13-on-kubernetes-1-13 (*)
pull-kubernetes-csi-csi-driver-host-path-alpha-1-14-on-kubernetes-1-14 (*)
pull-kubernetes-csi-csi-driver-host-path-unit (*)

ci-kubernetes-csi-1-13-on-kubernetes-1-13
ci-kubernetes-csi-1-13-on-kubernetes-1-14
ci-kubernetes-csi-1-13-on-kubernetes-master
ci-kubernetes-csi-alpha-1-13-on-kubernetes-1-13 (*)
ci-kubernetes-csi-canary-on-kubernetes-1-13
ci-kubernetes-csi-canary-on-kubernetes-1-14
ci-kubernetes-csi-canary-on-kubernetes-master
ci-kubernetes-csi-alpha-canary-on-kubernetes-master (*)

Jobs marked (*) with are new, other others have been changed to not
run alpha tests.

The "liveness-probe" pre-merge tests were limited to run only on one
Kubernetes version because there is no real dependency on Kubernetes
APIs.
@pohly
kubernetes-csi: fix org for periodic canary jobs
5376d7b 
The jobs still used pohly/csi-driver-host-path instead of
kubernetes-csi/csi-driver-host-path.
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 6, 2019
@pohly
kubernetes-csi: fix canary jobs for 1.13 and 1.14
b430fe8 
They have to specify the full version because that is how we find the
pre-built KinD image.
This was referenced Apr 8, 2019
Merged
Merged
Merged
@msau42
Copy link
Member

msau42 commented Apr 8, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 8, 2019
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: b6f73bb421c38d5e6fd42ae2139ef0d6717d5f3d

@msau42
Copy link
Member

msau42 commented Apr 8, 2019

/assign @michelle192837

@michelle192837
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: michelle192837, msau42, pohly

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 8, 2019
@k8s-ci-robot k8s-ci-robot merged commit c797b2d into kubernetes:master Apr 8, 2019
@k8s-ci-robot
Copy link
Contributor

@pohly: Updated the job-config configmap in namespace default using the following files:

  • key csi-driver-host-path-config.yaml using file config/jobs/kubernetes-csi/csi-driver-host-path/csi-driver-host-path-config.yaml
  • key external-attacher-config.yaml using file config/jobs/kubernetes-csi/external-attacher/external-attacher-config.yaml
  • key external-provisioner-config.yaml using file config/jobs/kubernetes-csi/external-provisioner/external-provisioner-config.yaml
  • key external-snapshotter-config.yaml using file config/jobs/kubernetes-csi/external-snapshotter/external-snapshotter-config.yaml
  • key livenessprobe-config.yaml using file config/jobs/kubernetes-csi/livenessprobe/livenessprobe-config.yaml
  • key node-driver-registrar-config.yaml using file config/jobs/kubernetes-csi/node-driver-registrar/node-driver-registrar-config.yaml

In response to this:

This is meant to help developers who look at tests results understand
more quickly what the root cause of a failure might be because they
don't need to look at the individual test name.

Splitting up the pre-merge jobs also speeds up providing feedback on a
PR, because jobs complete faster.

Naming of jobs is a bit more consistent now, with "kubernetes-csi"
being used also for pre-merge jobs. csi-driver-host-path
now has:

pull-kubernetes-csi-csi-driver-host-path-1-13-on-kubernetes-1-13
pull-kubernetes-csi-csi-driver-host-path-1-13-on-kubernetes-master
pull-kubernetes-csi-csi-driver-host-path-1-14-on-kubernetes-1-14
pull-kubernetes-csi-csi-driver-host-path-1-14-on-kubernetes-master
pull-kubernetes-csi-csi-driver-host-path-alpha-1-13-on-kubernetes-1-13 (*)
pull-kubernetes-csi-csi-driver-host-path-alpha-1-13-on-kubernetes-master (*)
pull-kubernetes-csi-csi-driver-host-path-alpha-1-14-on-kubernetes-1-14 (*)
pull-kubernetes-csi-csi-driver-host-path-alpha-1-14-on-kubernetes-master (*)
pull-kubernetes-csi-csi-driver-host-path-unit (*)

ci-kubernetes-csi-1-13-on-kubernetes-1-13
ci-kubernetes-csi-1-13-on-kubernetes-1-14
ci-kubernetes-csi-1-13-on-kubernetes-master
ci-kubernetes-csi-alpha-1-13-on-kubernetes-1-13 (*)
ci-kubernetes-csi-canary-on-kubernetes-1-13
ci-kubernetes-csi-canary-on-kubernetes-1-14
ci-kubernetes-csi-canary-on-kubernetes-master
ci-kubernetes-csi-alpha-canary-on-kubernetes-master (*)

Jobs marked (*) with are new, other others have been changed to not
run alpha tests.

The "liveness-probe" pre-merge tests were limited to run only on one
Kubernetes version because there is no real dependency on Kubernetes
APIs.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msau42 msau42 msau42 left review comments

@saad-ali saad-ali Awaiting requested review from saad-ali

@spiffxp spiffxp Awaiting requested review from spiffxp

Assignees

@michelle192837 michelle192837

@msau42 msau42

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/config Issues or PRs related to code in /config area/testgrid cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pohly @msau42 @k8s-ci-robot @michelle192837