WIP: test/e2e/storage: replace mock driver with hostpath driver #101672

avalluri · 2021-04-30T15:32:58Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This is a first step towards removing the mock CSI driver completely from
e2e testing in favor of hostpath plugin. With the recent hostpath plugin
changes(PR #260, #269), it supports all the features supported by the mock
csi driver.

Using hostpath-plugin for testing also covers CSI persistent feature
usecases.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

The PR is in WIP as it requires a hostpath-plugin release with the needed changes. Currently, it is using hostpath 'canary' images.

Does this PR introduce a user-facing change?

No

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

k8s-ci-robot · 2021-04-30T15:33:00Z

@avalluri: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

k8s-ci-robot · 2021-04-30T15:33:06Z

@avalluri: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

k8s-ci-robot · 2021-04-30T15:33:07Z

Hi @avalluri. 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.

pohly · 2021-04-30T16:20:46Z

test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml

@@ -93,3 +103,12 @@ spec:
            path: /var/lib/kubelet/plugins_registry
            type: Directory
          name: registration-dir
+        - hostPath:
+            # 'path' is where PV data is persisted on host.
+            path: /var/lib/csi-hostpath-data/


These tests are going to run in parallel, potentially on the same hosts. Therefore we can't use /var/lib/csi-hostpath-data on the host.

I don't think any of the mock tests rely on persisting the driver state across pod restarts. Therefore let's not use any hostpath volume. Instead pass -statedir=/tmp/csi-hostpath-data as parameter.

https://github.com/kubernetes-csi/csi-driver-host-path/blob/d0d44abf58958cbf2105a56da4968db2cee26291/cmd/hostpathplugin/main.go#L49

I changed to use --statedir for saving the state.

pohly · 2021-04-30T19:42:18Z

/ok-to-test

pohly · 2021-05-02T08:14:15Z

test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml

+            - "--drivername=mock.storage.k8s.io"
+            - "--nodeid=$(KUBE_NODE_NAME)"
+            - "--endpoint=/csi/csi.sock"
+            - "--statedir=/csi-data-dir"


My proposal was to use /tmp/csi-hostpath-data, not /csi-data-dir. It makes the intent clearer (don't persist) and might also be a bit more efficient (tmpfs vs overlayfs with actual disk IO).

Changed statedir to /tmp/csi-hostpath-data, as suggested.

pohly · 2021-05-02T08:15:11Z

test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml

+          # having below chagnes;
+          #  - https://github.com/kubernetes-csi/csi-driver-host-path/pull/269
+          #  - https://github.com/kubernetes-csi/csi-driver-host-path/pull/260          #
+          image: k8s.gcr.io/sig-storage/hostpathplugin:canary


The "canary" images don't get promoted to k8s.gcr.io/sig-storage. You have to use docker pull gcr.io/k8s-staging-sig-storage/hostpathplugin:canary.

This patch enables pulling that image:

diff --git a/test/utils/image/manifest.go b/test/utils/image/manifest.go index a8f60a8de85..068ef120031 100644 --- a/test/utils/image/manifest.go +++ b/test/utils/image/manifest.go @@ -30,18 +30,19 @@ import ( // RegistryList holds public and private image registries type RegistryList struct { - GcAuthenticatedRegistry string `yaml:"gcAuthenticatedRegistry"` - E2eRegistry string `yaml:"e2eRegistry"` - PromoterE2eRegistry string `yaml:"promoterE2eRegistry"` - BuildImageRegistry string `yaml:"buildImageRegistry"` - InvalidRegistry string `yaml:"invalidRegistry"` - GcEtcdRegistry string `yaml:"gcEtcdRegistry"` - GcRegistry string `yaml:"gcRegistry"` - SigStorageRegistry string `yaml:"sigStorageRegistry"` - GcrReleaseRegistry string `yaml:"gcrReleaseRegistry"` - PrivateRegistry string `yaml:"privateRegistry"` - SampleRegistry string `yaml:"sampleRegistry"` - MicrosoftRegistry string `yaml:"microsoftRegistry"` + GcAuthenticatedRegistry string `yaml:"gcAuthenticatedRegistry"` + E2eRegistry string `yaml:"e2eRegistry"` + PromoterE2eRegistry string `yaml:"promoterE2eRegistry"` + BuildImageRegistry string `yaml:"buildImageRegistry"` + InvalidRegistry string `yaml:"invalidRegistry"` + GcEtcdRegistry string `yaml:"gcEtcdRegistry"` + GcRegistry string `yaml:"gcRegistry"` + SigStorageRegistry string `yaml:"sigStorageRegistry"` + SigStorageCanaryRegistry string `yaml:"sigStorageCanaryRegistry"` + GcrReleaseRegistry string `yaml:"gcrReleaseRegistry"` + PrivateRegistry string `yaml:"privateRegistry"` + SampleRegistry string `yaml:"sampleRegistry"` + MicrosoftRegistry string `yaml:"microsoftRegistry"` } // Config holds an images registry, name, and version @@ -68,18 +69,19 @@ func (i *Config) SetVersion(version string) { func initReg() RegistryList { registry := RegistryList{ - GcAuthenticatedRegistry: "gcr.io/authenticated-image-pulling", - E2eRegistry: "gcr.io/kubernetes-e2e-test-images", - PromoterE2eRegistry: "k8s.gcr.io/e2e-test-images", - BuildImageRegistry: "k8s.gcr.io/build-image", - InvalidRegistry: "invalid.com/invalid", - GcEtcdRegistry: "k8s.gcr.io", - GcRegistry: "k8s.gcr.io", - SigStorageRegistry: "k8s.gcr.io/sig-storage", - PrivateRegistry: "gcr.io/k8s-authenticated-test", - SampleRegistry: "gcr.io/google-samples", - GcrReleaseRegistry: "gcr.io/gke-release", - MicrosoftRegistry: "mcr.microsoft.com", + GcAuthenticatedRegistry: "gcr.io/authenticated-image-pulling", + E2eRegistry: "gcr.io/kubernetes-e2e-test-images", + PromoterE2eRegistry: "k8s.gcr.io/e2e-test-images", + BuildImageRegistry: "k8s.gcr.io/build-image", + InvalidRegistry: "invalid.com/invalid", + GcEtcdRegistry: "k8s.gcr.io", + GcRegistry: "k8s.gcr.io", + SigStorageRegistry: "k8s.gcr.io/sig-storage", + SigStorageCanaryRegistry: "gcr.io/k8s-staging-sig-storage", + PrivateRegistry: "gcr.io/k8s-authenticated-test", + SampleRegistry: "gcr.io/google-samples", + GcrReleaseRegistry: "gcr.io/gke-release", + MicrosoftRegistry: "mcr.microsoft.com", } repoList := os.Getenv("KUBE_TEST_REPO_LIST") if repoList == "" { @@ -105,18 +107,19 @@ var ( PrivateRegistry = registry.PrivateRegistry // Preconfigured image configs - dockerLibraryRegistry = "docker.io/library" - e2eRegistry = registry.E2eRegistry - promoterE2eRegistry = registry.PromoterE2eRegistry - buildImageRegistry = registry.BuildImageRegistry - gcAuthenticatedRegistry = registry.GcAuthenticatedRegistry - gcEtcdRegistry = registry.GcEtcdRegistry - gcRegistry = registry.GcRegistry - sigStorageRegistry = registry.SigStorageRegistry - gcrReleaseRegistry = registry.GcrReleaseRegistry - invalidRegistry = registry.InvalidRegistry - sampleRegistry = registry.SampleRegistry - microsoftRegistry = registry.MicrosoftRegistry + dockerLibraryRegistry = "docker.io/library" + e2eRegistry = registry.E2eRegistry + promoterE2eRegistry = registry.PromoterE2eRegistry + buildImageRegistry = registry.BuildImageRegistry + gcAuthenticatedRegistry = registry.GcAuthenticatedRegistry + gcEtcdRegistry = registry.GcEtcdRegistry + gcRegistry = registry.GcRegistry + sigStorageRegistry = registry.SigStorageRegistry + sigStorageCanaryRegistry = registry.SigStorageCanaryRegistry + gcrReleaseRegistry = registry.GcrReleaseRegistry + invalidRegistry = registry.InvalidRegistry + sampleRegistry = registry.SampleRegistry + microsoftRegistry = registry.MicrosoftRegistry imageConfigs, originalImageConfigs = initImageConfigs() ) @@ -393,6 +396,8 @@ func ReplaceRegistryInImageURL(imageURL string) (string, error) { registryAndUser = gcRegistry case "k8s.gcr.io/sig-storage": registryAndUser = sigStorageRegistry + case "gcr.io/k8s-staging-sig-storage": + registryAndUser = sigStorageCanaryRegistry case "gcr.io/k8s-authenticated-test": registryAndUser = PrivateRegistry case "gcr.io/google-samples":

I wonder whether we should add that permanently and not just while experimenting with a canary image. I remember having to add and remove this already a few times.

@msau42 any thoughts on this?

@avalluri : put it into a separate commit for now, please.

This is a first step towards removing the mock CSI driver completely from e2e testing in favor of hostpath plugin. With the recent hostpath plugin changes(PR kubernetes#260, kubernetes#269), it supports all the features supported by the mock csi driver. Using hostpath-plugin for testing also covers CSI persistent feature usecases.

k8s-ci-robot · 2021-05-03T09:31:09Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: avalluri
To complete the pull request process, please assign saad-ali after the PR has been reviewed.
You can assign the PR to them by writing /assign @saad-ali in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

This allows to test using sigstorage canary images.

k8s-ci-robot · 2021-05-13T10:30:52Z

@avalluri: 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/test-infra repository.

k8s-triage-robot · 2021-08-11T10:56:19Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2021-09-10T11:00:24Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot · 2021-10-10T11:32:32Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen
Mark this issue or PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2021-10-10T11:32:47Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen

Mark this issue or PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Apr 30, 2021

k8s-ci-robot requested review from davidz627 and gnufied April 30, 2021 15:33

pohly suggested changes Apr 30, 2021

View reviewed changes

avalluri force-pushed the replace-mock-driver branch from 3d396bb to 4c0a76c Compare April 30, 2021 19:11

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 30, 2021

pohly reviewed May 2, 2021

View reviewed changes

avalluri force-pushed the replace-mock-driver branch from 4c0a76c to 45c53e1 Compare May 2, 2021 18:26

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 3, 2021

test: add gcr.io/k8s-staging-sig-storage to registry list

be39333

This allows to test using sigstorage canary images.

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2021

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 11, 2021

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 10, 2021

k8s-ci-robot closed this Oct 10, 2021

pohly mentioned this pull request Dec 2, 2021

storage e2e: replace mock driver, update images #106791

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: test/e2e/storage: replace mock driver with hostpath driver #101672

WIP: test/e2e/storage: replace mock driver with hostpath driver #101672

avalluri commented Apr 30, 2021

k8s-ci-robot commented Apr 30, 2021

k8s-ci-robot commented Apr 30, 2021

k8s-ci-robot commented Apr 30, 2021

pohly Apr 30, 2021 •

edited

Loading

avalluri Apr 30, 2021

pohly commented Apr 30, 2021

pohly May 2, 2021

avalluri May 4, 2021

pohly May 2, 2021

pohly May 3, 2021

avalluri May 4, 2021

k8s-ci-robot commented May 3, 2021

k8s-ci-robot commented May 13, 2021

k8s-triage-robot commented Aug 11, 2021

k8s-triage-robot commented Sep 10, 2021

k8s-triage-robot commented Oct 10, 2021

k8s-ci-robot commented Oct 10, 2021

WIP: test/e2e/storage: replace mock driver with hostpath driver #101672

WIP: test/e2e/storage: replace mock driver with hostpath driver #101672

Conversation

avalluri commented Apr 30, 2021

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 Apr 30, 2021

k8s-ci-robot commented Apr 30, 2021

k8s-ci-robot commented Apr 30, 2021

pohly Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

avalluri Apr 30, 2021

Choose a reason for hiding this comment

pohly commented Apr 30, 2021

pohly May 2, 2021

Choose a reason for hiding this comment

avalluri May 4, 2021

Choose a reason for hiding this comment

pohly May 2, 2021

Choose a reason for hiding this comment

pohly May 3, 2021

Choose a reason for hiding this comment

avalluri May 4, 2021

Choose a reason for hiding this comment

k8s-ci-robot commented May 3, 2021

k8s-ci-robot commented May 13, 2021

k8s-triage-robot commented Aug 11, 2021

k8s-triage-robot commented Sep 10, 2021

k8s-triage-robot commented Oct 10, 2021

k8s-ci-robot commented Oct 10, 2021

pohly Apr 30, 2021 •

edited

Loading