manila-csi-plugin: Auto-configure --nodeid, --nodeaz parameters (#2734)

* Stop setting GOPROXY in Dockerfile

We're seeing failures in CI related to goproxy.io. There's no need to
set this so simply don't.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* docs: Remove references to the csc tool

This has not seen any updates in a long time and does not appear to be
maintained. Our own doc is out-of-date and references removed options.
Just remove the whole thing.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* manila-csi-plugin: Enable auto-detection of topology, node ID

Same as we do for Cinder. A separate change will deprecate the
respective options.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* manila-csi-plugin: Deprecate --nodeid, --nodeaz flags

These are no longer used or necessary, now that we retrieve this
information from the metadata service.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* cinder-csi-plugin: Trivial variable rename

To clarify their meaning.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

---------

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

Author: stephenfin

charts/manila-csi-plugin/Chart.yaml

@@ -2,7 +2,7 @@ apiVersion: v1
 appVersion: v1.31.2
 description: Manila CSI Chart for OpenStack
 name: openstack-manila-csi
-version: 2.31.2
+version: 2.31.3
 home: http://github.com/kubernetes/cloud-provider-openstack
 icon: https://github.com/kubernetes/kubernetes/blob/master/logo/logo.png
 maintainers:

charts/manila-csi-plugin/templates/controllerplugin-statefulset.yaml

@@ -86,10 +86,8 @@ spec:
           command: ["/bin/sh", "-c",
             '/bin/manila-csi-plugin
             -v={{ $.Values.logVerbosityLevel }}
-            --nodeid=$(NODE_ID)
             {{- if $.Values.csimanila.topologyAwarenessEnabled }}
             --with-topology
-            --nodeaz={{ $.Values.csimanila.nodeAZ }}
             {{- end }}
             {{- if $.Values.csimanila.runtimeConfig.enabled }}
             --runtime-config-file=/runtimeconfig/runtimeconfig.json
@@ -109,10 +107,6 @@ spec:
           env:
             - name: DRIVER_NAME
               value: {{ printf "%s.%s" .protocolSelector $.Values.driverName | lower }}
-            - name: NODE_ID
-              valueFrom:
-                fieldRef:
-                  fieldPath: spec.nodeName
             - name: CSI_ENDPOINT
               value: "unix:///var/lib/kubelet/plugins/{{ printf "%s.%s" .protocolSelector $.Values.driverName | lower }}/csi-controllerplugin.sock"
             - name: FWD_CSI_ENDPOINT

charts/manila-csi-plugin/templates/nodeplugin-daemonset.yaml

@@ -50,13 +50,11 @@ spec:
           command: ["/bin/sh", "-c",
             '/bin/manila-csi-plugin
             -v={{ $.Values.logVerbosityLevel }}
-            --nodeid=$(NODE_ID)
             {{- if $.Values.csimanila.runtimeConfig.enabled }}
             --runtime-config-file=/runtimeconfig/runtimeconfig.json
             {{- end }}
             {{- if $.Values.csimanila.topologyAwarenessEnabled }}
             --with-topology
-            --nodeaz={{ $.Values.csimanila.nodeAZ }}
             {{- end }}
             --endpoint=$(CSI_ENDPOINT)
             --drivername=$(DRIVER_NAME)
@@ -67,10 +65,6 @@ spec:
           env:
             - name: DRIVER_NAME
               value: {{ printf "%s.%s" .protocolSelector $.Values.driverName | lower }}
-            - name: NODE_ID
-              valueFrom:
-                fieldRef:
-                  fieldPath: spec.nodeName
             - name: CSI_ENDPOINT
               value: "unix:///var/lib/kubelet/plugins/{{ printf "%s.%s" .protocolSelector $.Values.driverName | lower }}/csi.sock"
             - name: FWD_CSI_ENDPOINT

charts/manila-csi-plugin/values.yaml

@@ -35,10 +35,6 @@ csimanila:
         }
       }
 
-  # Availability zone for each node. topologyAwarenessEnabled must be set to true for this option to have any effect.
-  # If your Kubernetes cluster runs atop of Nova and want to use Nova AZs as AZs for the nodes of the cluster, uncomment the line below:
-  # nodeAZ: "$(curl http://169.254.169.254/openstack/latest/meta_data.json | jq -r .availability_zone)"
-
   # You may set ID of the cluster where manila-csi is deployed. This value will be appended
   # to share metadata in newly provisioned shares as `manila.csi.openstack.org/cluster=<cluster ID>`.
   clusterID: ""

cmd/cinder-csi-plugin/main.go

@@ -76,7 +76,7 @@ func main() {
 	csi.AddPVCFlags(cmd)
 
 	cmd.PersistentFlags().StringVar(&nodeID, "nodeid", "", "node id")
-	if err := cmd.PersistentFlags().MarkDeprecated("nodeid", "This flag would be removed in future. Currently, the value is ignored by the driver"); err != nil {
+	if err := cmd.PersistentFlags().MarkDeprecated("nodeid", "This option is now ignored by the driver. It will be removed in a future release."); err != nil {
 		klog.Fatalf("Unable to mark flag nodeid to be deprecated: %v", err)
 	}
 
@@ -129,7 +129,7 @@ func handle() {
 	}
 
 	if provideNodeService {
-		//Initialize mount
+		// Initialize mount
 		mount := mount.GetMountProvider()
 
 		cfg, err := openstack.GetConfigFromFiles(cloudConfig)
@@ -138,7 +138,7 @@ func handle() {
 			return
 		}
 
-		//Initialize Metadata
+		// Initialize Metadata
 		metadata := metadata.GetMetadataProvider(cfg.Metadata.SearchOrder)
 
 		d.SetupNodeService(mount, metadata, cfg.BlockStorage, additionalTopologies)

cmd/manila-csi-plugin/main.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/cloud-provider-openstack/pkg/csi/manila/csiclient"
 	"k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient"
 	"k8s.io/cloud-provider-openstack/pkg/csi/manila/runtimeconfig"
+	"k8s.io/cloud-provider-openstack/pkg/util/metadata"
 	"k8s.io/cloud-provider-openstack/pkg/version"
 	"k8s.io/component-base/cli"
 	"k8s.io/klog/v2"
@@ -90,11 +91,6 @@ func main() {
 				PVCLister:           csi.GetPVCLister(),
 			}
 
-			if provideNodeService {
-				opts.NodeID = nodeID
-				opts.NodeAZ = nodeAZ
-			}
-
 			d, err := manila.NewDriver(opts)
 			if err != nil {
 				klog.Fatalf("Driver initialization failed: %v", err)
@@ -108,7 +104,10 @@ func main() {
 			}
 
 			if provideNodeService {
-				err = d.SetupNodeService()
+				// Initialize metadata
+				metadata := metadata.GetMetadataProvider("")
+
+				err = d.SetupNodeService(metadata)
 				if err != nil {
 					klog.Fatalf("Driver node service initialization failed: %v", err)
 				}
@@ -127,9 +126,15 @@ func main() {
 
 	cmd.PersistentFlags().StringVar(&driverName, "drivername", "manila.csi.openstack.org", "name of the driver")
 
-	cmd.PersistentFlags().StringVar(&nodeID, "nodeid", "", "this node's ID. This value is required if the node service is provided by this CSI driver instance.")
+	cmd.PersistentFlags().StringVar(&nodeID, "nodeid", "", "this node's ID")
+	if err := cmd.PersistentFlags().MarkDeprecated("nodeid", "This option is now ignored by the driver. It will be removed in a future release."); err != nil {
+		klog.Fatalf("Unable to mark flag nodeid to be deprecated: %v", err)
+	}
 
 	cmd.PersistentFlags().StringVar(&nodeAZ, "nodeaz", "", "this node's availability zone")
+	if err := cmd.PersistentFlags().MarkDeprecated("nodeaz", "This option is now ignored by the driver. It will be removed in a future release."); err != nil {
+		klog.Fatalf("Unable to mark flag nodeaz to be deprecated: %v", err)
+	}
 
 	cmd.PersistentFlags().StringVar(&runtimeConfigFile, "runtime-config-file", "", "path to the runtime configuration file")
 

docs/cinder-csi-plugin/csc-tool.md

@@ -56,7 +56,7 @@ In addition to the standard set of klog flags, `cinder-csi-plugin` accepts the f
 <dl>
   <dt>--nodeid &lt;node id&gt;</dt>
   <dd>
-  This argument is deprecated, will be removed in future.
+  This argument is deprecated. It will be removed in future.
 
   An identifier for the current node which will be used in OpenStack API calls.  This can be either the UUID or name of the OpenStack server, but note that if using name it must be unique.
   </dd>
@@ -335,8 +335,6 @@ Run sanity tests for cinder CSI driver using:
 $ make test-cinder-csi-sanity
 ```
 
-Optionally, to test the driver csc tool could be used. please refer, [usage guide](./csc-tool.md) for more info.
-
 ## In-tree Cinder provisioner to cinder CSI Migration
 
 Starting from Kubernetes 1.21, OpenStack Cinder CSI migration is supported as beta feature and is `ON` by default. Cinder CSI driver must be installed on clusters on OpenStack for Cinder volumes to work. If you have persistence volumes that are created with in-tree `kubernetes.io/cinder` plugin, you could migrate to use `cinder.csi.openstack.org` Container Storage Interface (CSI) Driver.

docs/cinder-csi-plugin/using-cinder-csi-plugin.md

@@ -34,8 +34,8 @@ Option | Default value | Description
 -------|---------------|------------
 `--endpoint` | `unix:///tmp/csi.sock` | CSI Manila's CSI endpoint
 `--drivername` | `manila.csi.openstack.org` | Name of this driver
-`--nodeid` | _none_ | ID of this node
-`--nodeaz` | _none_ | Availability zone of this node
+`--nodeid` | _none_ | **DEPRECATED** ID of this node. This value is now automatically retrieved from the metadata service.
+`--nodeaz` | _none_ | **DEPRECATED** Availability zone of this node. This value is now automatically retrieved from the metadata service.
 `--runtime-config-file` | _none_ | Path to the [runtime configuration file](#runtime-configuration-file)
 `--with-topology` | _none_ | CSI Manila is topology-aware. See [Topology-aware dynamic provisioning](#topology-aware-dynamic-provisioning) for more info
 `--share-protocol-selector` | _none_ | Specifies which Manila share protocol to use for this instance of the driver. See [supported protocols](#share-protocol-support-matrix) for valid values.
@@ -103,7 +103,7 @@ With topology awareness enabled, administrators can specify the mapping between
 Doing so will instruct the CO scheduler to place the workloads+shares only on nodes that are able to reach the underlying storage.
 
 CSI Manila uses `topology.manila.csi.openstack.org/zone` _topology key_ to identify node's affinity to a certain compute availability zone.
-Each node of the cluster then gets labeled with a key/value pair of `topology.manila.csi.openstack.org/zone` / value of [`--nodeaz`](#command-line-arguments) cmd arg.
+Each node of the cluster then gets labeled with the `topology.manila.csi.openstack.org/zone` where the value is the value of the AZ retrieved from the Nova metadata service.
 
 This label may be used as a node selector when defining topology constraints for dynamic provisioning.
 Administrators are also free to pass arbitrary labels, and as long as they are valid node selectors, they will be honored by the scheduler.
@@ -258,11 +258,10 @@ To test the deployment further, see `examples/csi-manila-plugin`.
 
 If you're deploying CSI Manila with Helm:
 1. Set `csimanila.topologyAwarenessEnabled` to `true`
-2. Set `csimanila.nodeAZ`. This value will be sourced into the [`--nodeaz`](#command-line-arguments) cmd flag. Bash expressions are also allowed.
 
 If you're deploying CSI Manila manually:
 1. Run the [external-provisioner](https://github.com/kubernetes-csi/external-provisioner) with `--feature-gates=Topology=true` cmd flag.
-2. Run CSI Manila with [`--with-topology`](#command-line-arguments) and set [`--nodeaz`](#command-line-arguments) to node's availability zone. For Nova, the zone may be retrieved via the Metadata service like so: `--nodeaz=$(curl http://169.254.169.254/openstack/latest/meta_data.json | jq -r .availability_zone)`
+2. Run CSI Manila with [`--with-topology`](#command-line-arguments).
 
 See `examples/csi-manila-plugin/nfs/topology-aware` for examples on defining topology constraints.