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.

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

config: split wg-k8s-infra trusted jobs/dashboards up, add apps jobs #22970

Merged
merged 4 commits into from
Jul 21, 2021

Conversation

spiffxp
Copy link
Member

@spiffxp spiffxp commented Jul 21, 2021

Addresses part of: kubernetes/k8s.io#2151

Two main things here.

First, split out jobs/dashboards for wg-k8s-infra jobs that run on the trusted cluster, to set a better pattern to follow:

  • wg-k8s-infra-foo.yaml holds jobs related to foo, and they appear on a wg-k8s-infra-foo testgrid dashboard
  • sig-foo-trusted.yaml holds jobs owned by sig-foo that need to run on the trusted cluster

This results in the following files:

  • sig-security-trusted
  • wg-k8s-infra-groups
  • wg-k8s-infra-k8sio
  • wg-k8s-infra-prow

I did not reconcile releng/releng-trusted.yaml to this pattern, yet. But I'm inclined to head in that direction soon.

Next, add a wg-k8s-infra-apps set of jobs, generated from script:

  • post-k8sio-deploy-app-gcsweb
  • post-k8sio-deploy-app-k8s-io
  • post-k8sio-deploy-app-kubernetes-external-secrets
  • post-k8sio-deploy-app-node-perf-dash
  • post-k8sio-deploy-app-perfdash
  • post-k8sio-deploy-app-prow
  • post-k8sio-deploy-app-publishing-bot
  • post-k8sio-deploy-app-sippy
  • post-k8sio-deploy-app-slack-infra
  • post-k8sio-deploy-app-triageparty-release

There will need to be permissions changes on the k8s.io side to allow the prow-deployer service account to successfully run these jobs. The PR for that is kubernetes/k8s.io#2384, though neither of these strictly depend on the other to merge

As a followup, we could make a service-account per-app to do this, but some of the current apps span namespaces (e.g. cert-manager) and so constraining the service accounts is going to take more boilerplate than seems necessary at this stage.

@k8s-ci-robot 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. area/config Issues or PRs related to code in /config area/testgrid sig/testing Categorizes an issue or PR as relevant to SIG Testing. wg/k8s-infra labels Jul 21, 2021
@k8s-ci-robot k8s-ci-robot requested review from chases2 and thockin July 21, 2021 19:16
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 21, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: spiffxp

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 Jul 21, 2021
@spiffxp spiffxp force-pushed the split-k8s-infra-trusted-jobs branch 2 times, most recently from 2af5d91 to 67e157d Compare July 21, 2021 19:43
@spiffxp spiffxp force-pushed the split-k8s-infra-trusted-jobs branch from 67e157d to aecb2a2 Compare July 21, 2021 19:59
@spiffxp spiffxp changed the title [wip] config: split wg-k8s-infra trusted jobs/dashboards up, add apps jobs config: split wg-k8s-infra trusted jobs/dashboards up, add apps jobs Jul 21, 2021
@k8s-ci-robot 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 Jul 21, 2021
spiffxp added 3 commits July 21, 2021 15:26
Before wg-k8s-infra-trusted.yaml grows too large, let's split it up into
files that try to group similar jobs together.

Specifically, split:

- k8s-infra-prow jobs to wg-k8s-infra-prow
- groups jobs to wg-k8s-infra-groups
- snyk job to sig-security-trusted

Leave the remainder of misc infra manage jobs in wg-k8s-infra-trusted
@spiffxp spiffxp force-pushed the split-k8s-infra-trusted-jobs branch from 3bbd364 to 489dbb5 Compare July 21, 2021 22:28
@spiffxp spiffxp force-pushed the split-k8s-infra-trusted-jobs branch from 489dbb5 to 3a0586e Compare July 21, 2021 22:35
decorate: true
max_concurrency: 1
# intended for ignoring changes to README.md or OWNERS
run_if_changed: '^apps\/${app}\/(.*.yaml|deploy.sh)$'
Copy link
Member Author

@spiffxp spiffxp Jul 21, 2021

Choose a reason for hiding this comment

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

If I could specify "not OWNERS and not README" I would, but this will have to do

$ find apps/ -type f | xargs -n1 basename | sort | uniq -c | grep -v \.yaml$
   8 OWNERS
  10 README.md
   9 deploy.sh

testgrid-create-test-group: 'true'
testgrid-dashboards: wg-k8s-infra-apps
testgrid-description: 'runs https://git.k8s.io/k8s.io/apps/${app}/deploy.sh if files change in kubernetes/k8s.io/apps/${app}'
testgrid-alert-email: k8s-infra-rbac-${app}[email protected], [email protected]
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll drop k8s-infra-alerts in a followup PR when I've confirmed these all work

Comment on lines +70 to +72
# TODO: sig-specific team in charge of this app
# - org: kubernetes
# slug: sig-foo-bar
Copy link
Member Author

Choose a reason for hiding this comment

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

For followup: either we make a github team that can be easily guessed at by this script, or I'm going to need to make something uglier than the nice bash array this script uses

@ameukam
Copy link
Member

ameukam commented Jul 21, 2021

/lgtm
/hold
Remove hold when ready to deploy.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Jul 21, 2021

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2021
@k8s-ci-robot k8s-ci-robot merged commit 5df7636 into kubernetes:master Jul 21, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone Jul 21, 2021
@k8s-ci-robot
Copy link
Contributor

@spiffxp: Updated the job-config configmap in namespace default at cluster test-infra-trusted using the following files:

  • key sig-security-trusted.yaml using file config/jobs/kubernetes/wg-k8s-infra/trusted/sig-security-trusted.yaml
  • key wg-k8s-infra-apps.yaml using file config/jobs/kubernetes/wg-k8s-infra/trusted/wg-k8s-infra-apps.yaml
  • key wg-k8s-infra-groups.yaml using file config/jobs/kubernetes/wg-k8s-infra/trusted/wg-k8s-infra-groups.yaml
  • key wg-k8s-infra-k8sio.yaml using file config/jobs/kubernetes/wg-k8s-infra/trusted/wg-k8s-infra-k8sio.yaml
  • key wg-k8s-infra-prow.yaml using file config/jobs/kubernetes/wg-k8s-infra/trusted/wg-k8s-infra-prow.yaml
  • key wg-k8s-infra-trusted.yaml using file ``

In response to this:

Addresses part of: kubernetes/k8s.io#2151

Two main things here.

First, split out jobs/dashboards for wg-k8s-infra jobs that run on the trusted cluster, to set a better pattern to follow:

  • wg-k8s-infra-foo.yaml holds jobs related to foo, and they appear on a wg-k8s-infra-foo testgrid dashboard
  • sig-foo-trusted.yaml holds jobs owned by sig-foo that need to run on the trusted cluster

This results in the following files:

  • sig-security-trusted
  • wg-k8s-infra-groups
  • wg-k8s-infra-k8sio
  • wg-k8s-infra-prow

I did not reconcile releng/releng-trusted.yaml to this pattern, yet. But I'm inclined to head in that direction soon.

Next, add a wg-k8s-infra-apps set of jobs, generated from script:

  • post-k8sio-deploy-app-gcsweb
  • post-k8sio-deploy-app-k8s-io
  • post-k8sio-deploy-app-kubernetes-external-secrets
  • post-k8sio-deploy-app-node-perf-dash
  • post-k8sio-deploy-app-perfdash
  • post-k8sio-deploy-app-prow
  • post-k8sio-deploy-app-publishing-bot
  • post-k8sio-deploy-app-sippy
  • post-k8sio-deploy-app-slack-infra
  • post-k8sio-deploy-app-triageparty-release

There will need to be permissions changes on the k8s.io side to allow the prow-deployer service account to successfully run these jobs. The PR for that is kubernetes/k8s.io#2384, though neither of these strictly depend on the other to merge

As a followup, we could make a service-account per-app to do this, but some of the current apps span namespaces (e.g. cert-manager) and so constraining the service accounts is going to take more boilerplate than seems necessary at this stage.

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
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
Development

Successfully merging this pull request may close these issues.

3 participants