Skip to content

Start defaulting csi-e2e version for all sidecars#35137

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
gnufied:fix-gen-job-csi-e2e
Jul 15, 2025
Merged

Start defaulting csi-e2e version for all sidecars#35137
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
gnufied:fix-gen-job-csi-e2e

Conversation

@gnufied
Copy link
Copy Markdown
Member

@gnufied gnufied commented Jul 14, 2025

Fix e2e tests because we were running older builds of tests

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 14, 2025
@k8s-ci-robot k8s-ci-robot requested review from pohly and xing-yang July 14, 2025 18:28
@k8s-ci-robot k8s-ci-robot added area/config Issues or PRs related to code in /config area/jobs sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jul 14, 2025
gnufied
gnufied commented Jul 14, 2025
View reviewed changes
Comment thread ...ig/jobs/kubernetes-csi/volume-data-source-validator/volume-data-source-validator-config.yaml
# that something changes in master which breaks the pre-merge check.
cluster: eks-prow-build-cluster
always_run: true
always_run: false
Copy link
Copy Markdown
Member Author

@gnufied gnufied Jul 14, 2025

Choose a reason for hiding this comment

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

Does anyone know why is this set to true manually?

Copy link
Copy Markdown
Contributor

@sunnylovestiramisu sunnylovestiramisu Jul 14, 2025

Choose a reason for hiding this comment

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

So these tests are not for presubmits? But external-resizer always run these tests, where is that set?

Copy link
Copy Markdown
Member Author

@gnufied gnufied Jul 14, 2025

Choose a reason for hiding this comment

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

So in general, the tests that run against k8s master branch do not always run, because the idea is a unrelated change in k8s-master can break things. Comment say so as well:

# 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.

So, do we want to always run these tests? external-resizer also doesn't run these always. Only branch specific tests are always run.

Copy link
Copy Markdown
Contributor

@xing-yang xing-yang Jul 15, 2025

Choose a reason for hiding this comment

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

I think you are right. It's fine not to always run on master. There are two jobs not on master that also got changed from true to false. See my other comments.

Copy link
Copy Markdown
Contributor

@xing-yang xing-yang Jul 15, 2025

Choose a reason for hiding this comment

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

The other two jobs are alpha jobs so they are also considered unstable.

xing-yang
xing-yang reviewed Jul 15, 2025
View reviewed changes
Comment thread config/jobs/kubernetes-csi/lib-volume-populator/lib-volume-populator-config.yaml
- name: pull-kubernetes-csi-lib-volume-populator-alpha-1-32-on-kubernetes-1-32
cluster: eks-prow-build-cluster
always_run: true
always_run: false
Copy link
Copy Markdown
Contributor

@xing-yang xing-yang Jul 15, 2025

Choose a reason for hiding this comment

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

This one should always run? This is not on master.

Copy link
Copy Markdown
Contributor

@xing-yang xing-yang Jul 15, 2025

Choose a reason for hiding this comment

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

Never mind. Alpha jobs are also considered not stable so it is fine to stay as "false".

Comment thread ...ig/jobs/kubernetes-csi/volume-data-source-validator/volume-data-source-validator-config.yaml
- name: pull-kubernetes-csi-volume-data-source-validator-alpha-1-32-on-kubernetes-1-32
cluster: eks-prow-build-cluster
always_run: true
always_run: false
Copy link
Copy Markdown
Contributor

@xing-yang xing-yang Jul 15, 2025

Choose a reason for hiding this comment

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

This is not on master. We should always run this one?

Copy link
Copy Markdown
Contributor

@xing-yang xing-yang Jul 15, 2025

Choose a reason for hiding this comment

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

Never mind. Alpha jobs are also considered not stable so it is fine to stay as "false".

Comment thread ...ig/jobs/kubernetes-csi/volume-data-source-validator/volume-data-source-validator-config.yaml
# that something changes in master which breaks the pre-merge check.
cluster: eks-prow-build-cluster
always_run: true
always_run: false
Copy link
Copy Markdown
Contributor

@xing-yang xing-yang Jul 15, 2025

Choose a reason for hiding this comment

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

I think you are right. It's fine not to always run on master. There are two jobs not on master that also got changed from true to false. See my other comments.

xing-yang
xing-yang approved these changes Jul 15, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@xing-yang xing-yang left a comment

Choose a reason for hiding this comment

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

/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 Jul 15, 2025
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 15, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied, xing-yang

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

@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 15, 2025
@k8s-ci-robot k8s-ci-robot merged commit 71bd2b0 into kubernetes:master Jul 15, 2025
7 checks passed
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 15, 2025

@gnufied: Updated the job-config configmap in namespace default at cluster test-infra-trusted 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-resizer-config.yaml using file config/jobs/kubernetes-csi/external-resizer/external-resizer-config.yaml
  • key external-snapshot-metadata-config.yaml using file config/jobs/kubernetes-csi/external-snapshot-metadata/external-snapshot-metadata-config.yaml
  • key external-snapshotter-config.yaml using file config/jobs/kubernetes-csi/external-snapshotter/external-snapshotter-config.yaml
  • key lib-volume-populator-config.yaml using file config/jobs/kubernetes-csi/lib-volume-populator/lib-volume-populator-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
  • key volume-data-source-validator-config.yaml using file config/jobs/kubernetes-csi/volume-data-source-validator/volume-data-source-validator-config.yaml
Details

In response to this:

Fix e2e tests because we were running older builds of tests

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.

@liangyuanpeng liangyuanpeng mentioned this pull request Jul 15, 2025
Closed
@gnufied gnufied mentioned this pull request Jul 15, 2025
Merged
@iPraveenParihar iPraveenParihar mentioned this pull request Jul 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pohly pohly Awaiting requested review from pohly

2 more reviewers

@sunnylovestiramisu sunnylovestiramisu sunnylovestiramisu left review comments

@xing-yang xing-yang xing-yang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@xing-yang xing-yang

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/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gnufied @k8s-ci-robot @xing-yang @sunnylovestiramisu