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

cluster-api: introduce prowjob-generator configuration and generate prowjobs #31581

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

chrischdi
Copy link
Member

@chrischdi chrischdi commented Jan 9, 2024

Part of kubernetes-sigs/cluster-api#9937

Introduces config + templates for prowjob-gen and generate configurations

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/config Issues or PRs related to code in /config area/jobs sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jan 9, 2024
@chrischdi chrischdi changed the title cluster-api: introduce prowjob-generator configuration and generate prowjobs [wip] cluster-api: introduce prowjob-generator configuration and generate prowjobs Jan 9, 2024
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 9, 2024
@chrischdi chrischdi force-pushed the pr-capi-prowjob-generator branch 3 times, most recently from c9b8eb4 to 54933a4 Compare January 10, 2024 18:08
@chrischdi chrischdi changed the title [wip] cluster-api: introduce prowjob-generator configuration and generate prowjobs cluster-api: introduce prowjob-generator configuration and generate prowjobs Jan 11, 2024
@chrischdi chrischdi marked this pull request as ready for review January 11, 2024 07:19
@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 Jan 11, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 11, 2024
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 12, 2024
@sbueringer
Copy link
Member

/cc

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 12, 2024
Comment on lines 1 to 2
# This is the configuration file for cluster-apis prowjob-generator which is located
# at: https://github.com/kubernetes-sigs/cluster-api/blob/main/hack/tools/prowjob-generator/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# This is the configuration file for cluster-apis prowjob-generator which is located
# at: https://github.com/kubernetes-sigs/cluster-api/blob/main/hack/tools/prowjob-generator/
# This is the configuration file for cluster-apis prowjob-gen which is located
# at: https://github.com/kubernetes-sigs/cluster-api/blob/main/hack/tools/prowjob-gen/

# be defined without conflicting with unknown field validation.
prow_ignored:
# Branch specific configuration: for each configured branch the generator will create three
# prowjob configuration files (presubmits, periodics, peridocs for upgrade tests)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# prowjob configuration files (presubmits, periodics, peridocs for upgrade tests)
# prowjob configuration files (presubmits, periodics, periodics for upgrade tests)

main:
kubekinsImage: "gcr.io/k8s-staging-test-infra/kubekins-e2e:v20240111-cf1d81388e-1.29"
interval: "2h"
# This value determines the minimum Kubernetes supported version for Cluster API management cluster
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# This value determines the minimum Kubernetes supported version for Cluster API management cluster
# This value should be the minimum Kubernetes supported version for Cluster API management cluster

It wouldn't say that it determines what the min supported version is. It's more the other way around

# prowjob configuration files (presubmits, periodics, peridocs for upgrade tests)
branches:
main:
kubekinsImage: "gcr.io/k8s-staging-test-infra/kubekins-e2e:v20240111-cf1d81388e-1.29"
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a comment here. The kubekins "minor version" should match the minor version of Kubernetes dependencies used on this branch

(not yet the case on main because we're in the middle of bumping, but that is okay)

# The generator will run each template per branch and use a filename friendly
# branch variable to format the target filename.
templates:
- name: "cluster-api-periodics.yaml.tpl"
Copy link
Member

Choose a reason for hiding this comment

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

(Please in a follow-up PR)

I thin I would change the output format.

Today the resulting grouping is a bit strange

image

Basically first all periodics then grouped by branch then normal + upgrade
Then all presubmits

I think I would put the branch at the beginning cluster-api-{branch}-periodics / cluster-api-{branch}-periodics-upgrades cluster-api-{branch}-presubmits

Comment on lines 36 to 39
- name: ETCD_VERSION_UPGRADE_TO
value: "{{ index (index $.versions $upgrade.From) "etcd" }}"
- name: COREDNS_VERSION_UPGRADE_TO
value: "{{ index (index $.versions $upgrade.From) "coreDNS" }}"
Copy link
Member

Choose a reason for hiding this comment

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

This should be upgrade.To

Probably also means that the config has to be updated

Comment on lines 400 to 403
- name: ETCD_VERSION_UPGRADE_TO
value: "{{ index (index $.versions ((last $.config.Upgrades).From)) "etcd" }}"
- name: COREDNS_VERSION_UPGRADE_TO
value: "{{ index (index $.versions ((last $.config.Upgrades).From)) "coreDNS" }}"
Copy link
Member

Choose a reason for hiding this comment

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

Should be also .To

@sbueringer
Copy link
Member

lgtm +/- the linter findings

I would merge as soon as the findings are addressed (I want to avoid getting churn through an auto-bump)

@sbueringer
Copy link
Member

Thank you!

Merging for now. Let's open follow-up PRs if we want to change something (e.g. if the core CAPI PR changes)

As mentioned, I want to avoid a rebase if Prow gets bumped.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 17, 2024
@chrischdi
Copy link
Member Author

/hold

Should I squash?

@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 Jan 17, 2024
@sbueringer
Copy link
Member

/hold

@sbueringer
Copy link
Member

No idea in this repo

Can you fix #31581 (comment)?

Otherwise the link won't work

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 17, 2024
@sbueringer
Copy link
Member

Thx!

/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 Jan 17, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrischdi, sbueringer

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

@sbueringer
Copy link
Member

/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 Jan 17, 2024
@k8s-ci-robot k8s-ci-robot merged commit dfaabec into kubernetes:master Jan 17, 2024
7 checks passed
@k8s-ci-robot
Copy link
Contributor

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

  • key cluster-api-periodics-main-upgrades.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-periodics-main-upgrades.yaml
  • key cluster-api-periodics-main.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-periodics-main.yaml
  • key cluster-api-periodics-release-1-4-upgrades.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-periodics-release-1-4-upgrades.yaml
  • key cluster-api-periodics-release-1-4.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-periodics-release-1-4.yaml
  • key cluster-api-periodics-release-1-5-upgrades.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-periodics-release-1-5-upgrades.yaml
  • key cluster-api-periodics-release-1-5.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-periodics-release-1-5.yaml
  • key cluster-api-periodics-release-1-6-upgrades.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-periodics-release-1-6-upgrades.yaml
  • key cluster-api-periodics-release-1-6.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-periodics-release-1-6.yaml
  • key cluster-api-presubmits-main.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-presubmits-main.yaml
  • key cluster-api-presubmits-release-1-4.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-presubmits-release-1-4.yaml
  • key cluster-api-presubmits-release-1-5.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-presubmits-release-1-5.yaml
  • key cluster-api-presubmits-release-1-6.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-presubmits-release-1-6.yaml
  • key cluster-api-prowjob-gen.yaml using file config/jobs/kubernetes-sigs/cluster-api/cluster-api-prowjob-gen.yaml

In response to this:

Part of kubernetes-sigs/cluster-api#9937

Introduces config + templates for prowjob-gen and generate configurations

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/jobs 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/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants