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

merge mock and hostpath driver #247

Closed
pohly opened this issue Feb 9, 2021 · 6 comments · Fixed by kubernetes-csi/csi-test#351
Closed

merge mock and hostpath driver #247

pohly opened this issue Feb 9, 2021 · 6 comments · Fixed by kubernetes-csi/csi-test#351
Assignees

Comments

@pohly
Copy link
Contributor

pohly commented Feb 9, 2021

Both mock and hostpath driver are used for testing, just in different configurations and with different capabilities of the driver. It's not clear whether there actually is a reason to maintain two different drivers. Perhaps they should be merged into one.

Existing capabilities:

  • check credentials (mock)
  • optional attach (mock)
  • snapshots (both?)
  • error injection via JavaScript hooks (mock - may become obsolete via e2e storage: embedded csi mock driver kubernetes/kubernetes#97069)
  • raw block volumes (hostpath)
  • actual storage of data, including snapshots (hostpath)
  • simulating capacity (hostpath)
  • multi-node deployment (hostpath)

Missing capabilities:

@pohly
Copy link
Contributor Author

pohly commented Mar 16, 2021

We discussed this and came to the conclusion that as long as we only need to maintain one driver it doesn't matter too much which one we pick. However, the hostpath driver seems to be a better starting point because it is more widely used (several Prow jobs), is more realistic (actual mounts) and more complete (raw block support), and can be released independently because it is in its own repo. The mock driver currently must be released together with csi-sanity.

So here's a tentative plan:

  • move functionality from the mock driver to hostpath:
    • JSON call logging (https://github.com/kubernetes-csi/csi-test/blob/c4bffc15d5e956e1dee5ba88d1c1e007960774c2/driver/driver.go#L216-L243)
    • proxy (kubernetes-csi/csi-test@ef07baf)
    • implement ControllerPublishVolume and ControllerUnpublishVolume (
      func (hp *hostPath) ControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
      return nil, status.Error(codes.Unimplemented, "")
      }
      func (hp *hostPath) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) {
      return nil, status.Error(codes.Unimplemented, "")
      }
      ,
      func (hp *hostPath) ControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
      return nil, status.Error(codes.Unimplemented, "")
      }
      func (hp *hostPath) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) {
      return nil, status.Error(codes.Unimplemented, "")
      }
      ), with checking that the volume lifecycle is followed by the CO (CreateVolume -> ControllerPublishVolume -> NodeStageVolume -> NodePublishVolume -> NodeUnpublishVolume -> NodeUnstageVolume -> ControllerUnpublishVolume -> DeleteVolume). This must be an optional feature of the driver, to support testing with and with external-attacher.
  • replace Kubernetes E2E tests that currently use mock driver with that enhanced hostpath driver or with embedded error injection (for credential checking)
  • vendor the enhanced hostpath driver into Kubernetes to get rid of the mock driver fork
  • remove mock driver from csi-test

@avalluri
Copy link
Contributor

With #269 and the below changes in Kubernetes e2e, I could able to run mock csi storage tests:

diff --git a/test/e2e/storage/drivers/csi.go b/test/e2e/storage/drivers/csi.go
index 68c3705d607..06a93f4f658 100644
--- a/test/e2e/storage/drivers/csi.go
+++ b/test/e2e/storage/drivers/csi.go
@@ -553,10 +553,10 @@ func (m *mockCSIDriver) PrepareTest(f *framework.Framework) (*storageframework.P
        } else {
                // When using the mock driver inside the cluster it has to be reconfigured
                // via command line parameters.
-               containerArgs = append(containerArgs, "--name=csi-mock-"+f.UniqueName)
+               containerArgs = append(containerArgs, "--drivername=csi-mock-"+f.UniqueName)
 
-               if !m.attachable {
-                       containerArgs = append(containerArgs, "--disable-attach")
+               if m.attachable {
+                       containerArgs = append(containerArgs, "--enable-attach")
                }
 
                if m.enableTopology {
diff --git a/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml b/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml
index fe3f062afb4..ad9280c4bdd 100644
--- a/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml
+++ b/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml
@@ -15,7 +15,7 @@ spec:
       serviceAccountName: csi-mock
       containers:
         - name: csi-provisioner
-          image: k8s.gcr.io/sig-storage/csi-provisioner:v2.1.0
+          image: k8s.gcr.io/sig-storage/csi-provisioner:v2.1.1
           args:
             - "--csi-address=$(ADDRESS)"
             # Topology support is needed for the pod rescheduling test
@@ -53,10 +53,12 @@ spec:
           - mountPath: /registration
             name: registration-dir
         - name: mock
-          image: k8s.gcr.io/sig-storage/mock-driver:v4.1.0
+          image: localhost/hostpathplugin:latest
           args:
-            - "--name=mock.storage.k8s.io"
-            - "-v=3" # enabled the gRPC call logging
+            - "--drivername=mock.storage.k8s.io"
+            - "--nodeid=$(KUBE_NODE_NAME)"
+            - "--endpoint=/csi/csi.sock"
+            - "-v=5" # enabled the gRPC call logging
           env:
             - name: CSI_ENDPOINT
               value: /csi/csi.sock
@@ -74,6 +76,10 @@ spec:
               name: kubelet-pods-dir
             - mountPath: /var/lib/kubelet/plugins/kubernetes.io/csi
               name: kubelet-csi-dir
+            - mountPath: /csi-data-dir
+              name: csi-data-dir
+            - mountPath: /dev
+              name: dev-dir
       volumes:
         - hostPath:
             path: /var/lib/kubelet/plugins/csi-mock
@@ -93,3 +99,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/
+            type: DirectoryOrCreate
+          name: csi-data-dir
+        - hostPath:
+            path: /dev
+            type: Directory
+          name: dev-dir
diff --git a/test/e2e/testing-manifests/storage-csi/mock/csi-mock-proxy.yaml b/test/e2e/testing-manifests/storage-csi/mock/csi-mock-proxy.yaml
index 7892392931e..abf5d8e1f26 100644
--- a/test/e2e/testing-manifests/storage-csi/mock/csi-mock-proxy.yaml
+++ b/test/e2e/testing-manifests/storage-csi/mock/csi-mock-proxy.yaml
@@ -15,7 +15,7 @@ spec:
       serviceAccountName: csi-mock
       containers:
         - name: csi-provisioner
-          image: k8s.gcr.io/sig-storage/csi-provisioner:v2.1.0
+          image: k8s.gcr.io/sig-storage/csi-provisioner:v2.1.1
           args:
             - "--csi-address=$(ADDRESS)"
             # Topology support is needed for the pod rescheduling test
@@ -53,16 +53,23 @@ spec:
           - mountPath: /registration
             name: registration-dir
         - name: mock
-          image: k8s.gcr.io/sig-storage/mock-driver:v4.1.0
+          image: localhost/hostpathplugin:latest
           args:
             # -v3 shows when connections get established. Higher log levels print information about
             # transferred bytes, but cannot print message content (no gRPC parsing), so this is usually
             # not interesting.
-            - -v=3
+            - -v=5
+            - -nodeid=$(KUBE_NODE_NAME)
+            - -endpoint=/csi/csi.sock
             - -proxy-endpoint=tcp://:9000
           env:
             - name: CSI_ENDPOINT
               value: /csi/csi.sock
+            - name: KUBE_NODE_NAME
+              valueFrom:
+                fieldRef:
+                  apiVersion: v1
+                  fieldPath: spec.nodeName
           ports:
             - containerPort: 9000
               name: socat
$ kubetest --provider=local --test --test_args="--ginkgo.focus='\[sig-storage\] CSI mock volume' --test.v"

Mar 29 12:49:33.605: INFO: Running AfterSuite actions on node 1
{"msg":"Test Suite completed","total":38,"completed":38,"skipped":5707,"failed":0}

Ran 38 of 5745 Specs in 3896.168 seconds
SUCCESS! -- 38 Passed | 0 Failed | 0 Pending | 5707 Skipped
--- PASS: TestE2E (3896.18s)
=== RUN   TestViperConfig
--- PASS: TestViperConfig (0.00s)
PASS

Ginkgo ran 1 suite in 1h4m57.73308344s
Test Suite Passed
2021/03/29 12:49:33 process.go:155: Step './hack/ginkgo-e2e.sh --ginkgo.focus=\[sig-storage\].CSI.mock.volume --test.v' finished in 1h4m57.833580413s

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 27, 2021
@pohly
Copy link
Contributor Author

pohly commented Jun 27, 2021

/reopen
/assign

All functionality was moved over into csi-driver-host-path and only that driver is used now.

However, we still need to remove the mock driver source code and then at some point do a csi-test release without it.

@k8s-triage-robot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

@k8s-ci-robot 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 Jul 27, 2021
@pohly
Copy link
Contributor Author

pohly commented Aug 2, 2021

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 2, 2021
humblec added a commit to humblec/csi-driver-host-path that referenced this issue May 24, 2024
4967685 Merge pull request kubernetes-csi#254 from bells17/add-github-actions
d9bd160 Update skip list in codespell GitHub Action
adb3af9 Merge pull request kubernetes-csi#252 from bells17/update-go-version
f5aebfc Add GitHub Actions workflows
b82ee38 Merge pull request kubernetes-csi#253 from bells17/fix-typo
c317456 Fix typo
0a78505 Bump to Go 1.22.3
edd89ad Merge pull request kubernetes-csi#251 from jsafrane/add-logcheck
043fd09 Add test-logcheck target
d7535ae Merge pull request kubernetes-csi#250 from jsafrane/go-1.22
b52e7ad Update go to 1.22.2
14fdb6f Merge pull request kubernetes-csi#247 from msau42/prow
9b4352e Update release playbook
c7bb972 Fix release notes script to use fixed tags
463a0e9 Add script to update specific go modules

git-subtree-dir: release-tools
git-subtree-split: 4967685
TerryHowe pushed a commit to TerryHowe/csi-driver-host-path that referenced this issue Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants