turning on the CSIMigrationGCE feature flag #104722

leiyiz · 2021-09-01T23:29:19Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

KEP-625
This PR turns on the CSIMigrationGCE feature flag, fixed the unit test failures introduced by such change.
This PR also removed default storage class from gce kube-up scripts because it won't work without pdcsi driver installed by default and with CSIMigrationGCE feature flag on.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

CSIMigrationGCE feature flag is turned ON by default

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2021-09-01T23:29:27Z

Hi @leiyiz. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

leiyiz · 2021-09-01T23:29:59Z

@mattcary can you add ok-to-test

mattcary · 2021-09-01T23:32:20Z

/ok-to-test
/hold

For others reading this, we're testing which presubmit tests are affected by CSI Migration.

leiyiz · 2021-11-16T18:09:46Z

@wojtek-t

wojtek-t · 2021-11-16T19:15:35Z

pkg/features/kube_features.go

@@ -830,7 +830,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	TopologyManager:                                {Default: true, PreRelease: featuregate.Beta},
 	StorageObjectInUseProtection:                   {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.25
 	CSIMigration:                                   {Default: true, PreRelease: featuregate.Beta},
-	CSIMigrationGCE:                                {Default: false, PreRelease: featuregate.Beta}, // Off by default (requires GCE PD CSI Driver)
+	CSIMigrationGCE:                                {Default: true, PreRelease: featuregate.Beta}, // Off by default (requires GCE PD CSI Driver)


nit: please update the comment - no longer off by default

wojtek-t · 2021-11-16T19:16:37Z

/approve

/hold - to let you apply the nit

k8s-ci-robot · 2021-11-16T19:17:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leiyiz, msau42, wojtek-t

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:

~~cluster/gce/OWNERS~~ [wojtek-t]
~~pkg/controller/volume/attachdetach/OWNERS~~ [msau42,wojtek-t]
~~pkg/controller/volume/persistentvolume/OWNERS~~ [msau42,wojtek-t]
~~pkg/features/OWNERS~~ [msau42,wojtek-t]
~~pkg/kubelet/OWNERS~~ [wojtek-t]
~~test/e2e/storage/OWNERS~~ [msau42,wojtek-t]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

comment change to on by default in 1.23

disable CSIMigrationGCE in some unit tests

…ault storage class

leiyiz · 2021-11-16T19:28:43Z

/hold cancel
reason: rebased and comment addressed

mattcary · 2021-11-16T19:32:06Z

/lgtm

k8s-triage-robot · 2021-11-16T21:32:33Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

spiffxp · 2021-11-18T22:06:15Z

A bunch of the upgrade jobs started failing (more) after this landed, e.g. https://testgrid.k8s.io/google-gce-upgrade#gce-stable1-latest-upgrade-cluster-new

They run on a slower cadence than release-blocking tests so it'll take a bit to see if #106503 fixed that

msau42 · 2021-11-18T22:22:15Z

We specifically added some Skips in the test-job configs. I think some of these will need to be added to upgrade jobs. @leiyiz were you able to find a way to skip only statefulset-upgrade or pv-upgrade tests instead of the entire upgrade suite?

leiyiz · 2021-11-19T18:28:31Z

were you able to find a way to skip only statefulset-upgrade or pv-upgrade tests instead of the entire upgrade suite?

I think statefulset upgrades I looked at used default storage class, I'll take a look at these 2 and see if they can be skipped due to no default storage class.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 1, 2021

k8s-ci-robot requested review from smarterclayton and thockin September 1, 2021 23:33

k8s-ci-robot added the do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. label Sep 17, 2021

leiyiz force-pushed the migration branch from 09abfb9 to 39c2e2a Compare September 17, 2021 23:57

leiyiz marked this pull request as ready for review September 18, 2021 00:09

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 Sep 18, 2021

leiyiz marked this pull request as draft September 18, 2021 00:10

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 18, 2021

wojtek-t reviewed Nov 16, 2021

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2021

leiyiz added 4 commits November 16, 2021 19:26

turning on the CSIMigrationGCE feature flag

3bd7954

comment change to on by default in 1.23

fixing unit test failures induced by turning on CSIMigrationGCE

275fdf0

disable CSIMigrationGCE in some unit tests

turning off default storage class in kube-up

3c91436

skip in-tree and default Dynamic Provisioner test when there's no def…

2298dc6

…ault storage class

leiyiz force-pushed the migration branch from fd7ccaa to 2298dc6 Compare November 16, 2021 19:27

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2021

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2021

k8s-ci-robot merged commit 6805e6e into kubernetes:master Nov 16, 2021

SIG Node CI/Test Board automation moved this from Triage to Done Nov 16, 2021

SIG Node PR Triage automation moved this from Waiting on Author to Done Nov 16, 2021

k8s-ci-robot added this to the v1.23 milestone Nov 16, 2021

leiyiz mentioned this pull request Nov 17, 2021

skip tests that uses default storage class #106503

Merged

mauriciopoppe mentioned this pull request Nov 17, 2021

[Failing test][sig-cloud-provider] gce-cos-master-serial #106008

Closed

leiyiz mentioned this pull request Nov 19, 2021

[Failing test][sig-scalability] gce-master-scale-correctness #106459

Closed

mauriciopoppe mentioned this pull request Nov 22, 2021

Add status count to the existing storage_operation_duration_seconds histogram, remove volume_provision metric check #106609

Merged

leiyiz mentioned this pull request Feb 10, 2022

REQUEST: New membership for leiyiz kubernetes/org#3246

Closed

9 tasks

carlory mentioned this pull request Jan 7, 2024

fix incorrect volume doc kubernetes/website#44657

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

turning on the CSIMigrationGCE feature flag #104722

turning on the CSIMigrationGCE feature flag #104722

leiyiz commented Sep 1, 2021 •

edited

k8s-ci-robot commented Sep 1, 2021

leiyiz commented Sep 1, 2021

mattcary commented Sep 1, 2021

leiyiz commented Nov 16, 2021

wojtek-t Nov 16, 2021

leiyiz Nov 16, 2021

wojtek-t commented Nov 16, 2021

k8s-ci-robot commented Nov 16, 2021

leiyiz commented Nov 16, 2021 •

edited

mattcary commented Nov 16, 2021

k8s-triage-robot commented Nov 16, 2021

spiffxp commented Nov 18, 2021 •

edited

msau42 commented Nov 18, 2021

leiyiz commented Nov 19, 2021

turning on the CSIMigrationGCE feature flag #104722

turning on the CSIMigrationGCE feature flag #104722

Conversation

leiyiz commented Sep 1, 2021 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Sep 1, 2021

leiyiz commented Sep 1, 2021

mattcary commented Sep 1, 2021

leiyiz commented Nov 16, 2021

wojtek-t Nov 16, 2021

Choose a reason for hiding this comment

leiyiz Nov 16, 2021

Choose a reason for hiding this comment

wojtek-t commented Nov 16, 2021

k8s-ci-robot commented Nov 16, 2021

leiyiz commented Nov 16, 2021 • edited

mattcary commented Nov 16, 2021

k8s-triage-robot commented Nov 16, 2021

spiffxp commented Nov 18, 2021 • edited

msau42 commented Nov 18, 2021

leiyiz commented Nov 19, 2021

leiyiz commented Sep 1, 2021 •

edited

leiyiz commented Nov 16, 2021 •

edited

spiffxp commented Nov 18, 2021 •

edited