Shift generated cluster e2e image validation jobs to config-forker/rotator #33927
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
size/XXL
Rajalakshmi-Girish
force-pushed
the
change-generated-jobs
branch
2 times, most recently
from
b631329 to
c649f80
Compare
k8s-ci-robot
added
the
do-not-merge/work-in-progress
Rajalakshmi-Girish
force-pushed
the
change-generated-jobs
branch
from
c649f80 to
214d50b
Compare
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
k8s-ci-robot
added
the
do-not-merge/work-in-progress
Rajalakshmi-Girish
force-pushed
the
change-generated-jobs
branch
from
214d50b to
543ca4c
Compare
|
/retest |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
Rajalakshmi-Girish
force-pushed
the
change-generated-jobs
branch
from
543ca4c to
ace3131
Compare
Rajalakshmi-Girish
force-pushed
the
change-generated-jobs
branch
from
ace3131 to
d7c4cfa
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Rajalakshmi-Girish The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| spec: | ||
| containers: | ||
| - args: | ||
| - --cluster=test-gce-cos-k8sstable3-alphafeatures |
There was a problem hiding this comment.
Prior to this change, this used to be a randomly generated string (I think). Do we care about this change, is there a chance that this might break something?
cc @BenTheElder
There was a problem hiding this comment.
I don't think it will break anything, we are fully on boskos. I don't think we even need to set this flag anymore, but I'd prefer to leave it for now.
There was a problem hiding this comment.
My biggest concern would be generating a value that is too long or improper character set, if we're changing the generation.
There was a problem hiding this comment.
The value for the --cluster flag was previously assigned the SHA1 hash of the job_name, generated by this line.
Since the job names only varied by version markers (e.g., beta, stable1) and remained consistent otherwise, the --cluster data stayed aligned with the job name despite version changes.
Given this consistency, I thought assigning the --cluster value directly as the job name should suffice.
| @@ -0,0 +1,236 @@ | |||
| periodics: | |||
There was a problem hiding this comment.
why is this file called "image-validation" ? isn't this just release-branched jobs?
and they're not really sig-release, even if release is doing the forking, cc @aojea
these are just kubernetes/kubernetes jobs, with multiple SIGs involved.
There was a problem hiding this comment.
replied here
|
It would be nice if we could do a commit with the file split and then further commits with the definition mutations, as-is it's very hard to be confident about which changes landed in the jobs and review them. |
| When a release branch of kubernetes is first cut, the jobs defined in [`cloud_provider_image_validation.yaml`] | ||
| must be forked to use the new release branch. Use [`releng/config-forker`] to | ||
| accomplish this, eg: |
There was a problem hiding this comment.
Some wording improvements:
| When a release branch of kubernetes is first cut, the jobs defined in [`cloud_provider_image_validation.yaml`] | |
| must be forked to use the new release branch. Use [`releng/config-forker`] to | |
| accomplish this, eg: | |
| When cutting a new Kubernetes release branch, the jobs defined in | |
| [`cloud_provider_image_validation.yaml`] must be forked for the new | |
| release branch. Use [`releng/config-forker`] to accomplish this, e.g.: |
| --version 1.27 \ | ||
| --go-version 1.31 \ |
There was a problem hiding this comment.
Let's put placeholders here, e.g.:
| --version 1.27 \ | |
| --go-version 1.31 \ | |
| --version <Kubernetes major.minor version> \ | |
| --go-version <used Go major.minor version> \ |
| --job-config $(pwd)/releng/cloud_provider_image_validation.yaml \ | ||
| --version 1.27 \ | ||
| --go-version 1.31 \ | ||
| --output $(pwd)/config/jobs/kubernetes/sig-release/release-branch-jobs/cloud-provider/image-validation-1.31.yaml |
There was a problem hiding this comment.
| --output $(pwd)/config/jobs/kubernetes/sig-release/release-branch-jobs/cloud-provider/image-validation-1.31.yaml | |
| --output $(pwd)/config/jobs/kubernetes/sig-release/release-branch-jobs/cloud-provider/image-validation-<Kubernetes major.minor version>.yaml |
| --output $(pwd)/config/jobs/kubernetes/sig-release/release-branch-jobs/cloud-provider/image-validation-1.31.yaml | ||
| ``` | ||
|
|
||
| # How to rotate the k8sbeta job to stable1 |
There was a problem hiding this comment.
We should explain why are we doing this.
| ```sh | ||
| # from test-infra root | ||
| $ go run ./releng/config-rotator \ | ||
| --config-file ./config/jobs/kubernetes/sig-release/release-branch-jobs/cloud-provider/image-validation-1.31.yaml \ | ||
| --new stable1 --old beta | ||
| ``` |
There was a problem hiding this comment.
We should also mention that this needs to be done for each release branch.
There was a problem hiding this comment.
Unfortunately, I don't think this really solves the problem. This just shifts the problem from test_config.yaml to a new source that still has to be maintained in addition to the master branch jobs.
Ideally, we'd fork existing master branch jobs and not a dedicated template just for release branches. I can still see maintainers just changing the master branch jobs, and leaving jobs defined in this files as-is.
Also +1 to what @BenTheElder said about file name and ownership.
There was a problem hiding this comment.
Thank you for your feedback 👍
@xmudrii You’re absolutely right that having a dedicated template for release branch jobs introduces the risk of maintainers not keeping it in sync with master branch jobs.
How would you prefer to handle this? Do you think we should configure the jobs in the template to also run against the master branch?
@BenTheElder Could you also suggest an ideal filename and location for these jobs to ensure clarity and ease of maintenance?
There was a problem hiding this comment.
@xmudrii / @BenTheElder
Below are the corresponding jobs I could find for master branch. Kindly let me know if these can be modified to be compatible with config-forker to generate release branch jobs?
Other than the details job wise, there are a bunch of changes that I see when comparing master branch and release branch jobs:
- Release branch jobs have build cluster value
cluster: k8s-infra-prow-buildmentioned in job definitions. - All RB jobs have an extra argument
--clusterpassed. But as per Ben's comment here, that flag can be removed for RB jobs too! - The master branch jobs have extra annotations like
testgrid-alert-email:,testgrid-alert-stale-results-hours:
What could be the next step?
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. 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-sigs/prow repository. |
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
/test pull-cos-containerd-e2e-ubuntu-gce |
|
@Rajalakshmi-Girish: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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-sigs/prow repository. |
This change contains:
fork-per-release-generic-suffixto releng/config-forker/README.mdreleng/cloud_provider_image_validation.yamlto hold cluster e2e cloud provider image validation job definitions.config/jobs/kubernetes/sig-release/release-branch-jobs/cloud-provider/1.32.yamlalphafeatures: removeEventedPLEG=falseandENABLE_AUTH_PROVIDER_GCP=trueenv argsdefault: remove ENABLE_CACHE_MUTATION_DETECTOR=true env argserial: remove[sig-cloud-provider-gcp\]regex from--ginkgo.skiptest_argThe upcoming actions would be to:
releng/test_config.yaml#32340 (comment)@xmudrii @BenTheElder @dims ^^