Skip to content

Commit

Permalink
manila-csi-plugin: Deprecate --nodeid, --nodeaz flags
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
stephenfin committed Dec 4, 2024
1 parent 3b659db commit 2b20adb
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 0 additions & 6 deletions charts/manila-csi-plugin/templates/nodeplugin-daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
4 changes: 0 additions & 4 deletions charts/manila-csi-plugin/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""
Expand Down
2 changes: 1 addition & 1 deletion cmd/cinder-csi-plugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
13 changes: 7 additions & 6 deletions cmd/manila-csi-plugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,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)
Expand Down Expand Up @@ -131,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")

Expand Down
2 changes: 1 addition & 1 deletion docs/cinder-csi-plugin/using-cinder-csi-plugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
9 changes: 4 additions & 5 deletions docs/manila-csi-plugin/using-manila-csi-plugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.

Expand Down
6 changes: 2 additions & 4 deletions manifests/manila-csi-plugin/csi-controllerplugin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,14 @@ spec:
image: registry.k8s.io/provider-os/manila-csi-plugin:v1.31.2
command: ["/bin/sh", "-c",
'/bin/manila-csi-plugin
--nodeid=$(NODE_ID)
--endpoint=$(CSI_ENDPOINT)
--drivername=$(DRIVER_NAME)
--share-protocol-selector=$(MANILA_SHARE_PROTO)
--fwdendpoint=$(FWD_CSI_ENDPOINT)
--pvc-annotations'
# To enable topology awareness and retrieve compute node AZs from the OpenStack Metadata Service, add the following flags:
# To enable topology awareness and retrieve compute node AZs from the OpenStack Metadata Service, add the following flag:
# --with-topology
# --nodeaz=$(curl http://169.254.169.254/openstack/latest/meta_data.json | jq -r .availability_zone)
# Those flags need to be added to csi-nodeplugin.yaml as well.
# This flag needs to be added to csi-nodeplugin.yaml as well.
]
env:
- name: DRIVER_NAME
Expand Down
6 changes: 2 additions & 4 deletions manifests/manila-csi-plugin/csi-nodeplugin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,13 @@ spec:
image: registry.k8s.io/provider-os/manila-csi-plugin:v1.31.2
command: ["/bin/sh", "-c",
'/bin/manila-csi-plugin
--nodeid=$(NODE_ID)
--endpoint=$(CSI_ENDPOINT)
--drivername=$(DRIVER_NAME)
--share-protocol-selector=$(MANILA_SHARE_PROTO)
--fwdendpoint=$(FWD_CSI_ENDPOINT)'
# To enable topology awareness and retrieve compute node AZs from the OpenStack Metadata Service, add the following flags:
# To enable topology awareness and retrieve compute node AZs from the OpenStack Metadata Service, add the following flag:
# --with-topology
# --nodeaz=$(curl http://169.254.169.254/openstack/latest/meta_data.json | jq -r .availability_zone)
# Those flags need to be added to csi-controllerplugin.yaml as well.
# This flag needs to be added to csi-controllerplugin.yaml as well.
]
env:
- name: DRIVER_NAME
Expand Down
48 changes: 22 additions & 26 deletions pkg/csi/manila/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,31 +37,13 @@ import (
"k8s.io/klog/v2"
)

type DriverOpts struct {
DriverName string
NodeID string
NodeAZ string
WithTopology bool
ShareProto string
ClusterID string

ServerCSIEndpoint string
FwdCSIEndpoint string

ManilaClientBuilder manilaclient.Builder
CSIClientBuilder csiclient.Builder

PVCLister v1.PersistentVolumeClaimLister
}

type Driver struct {
nodeID string
nodeAZ string
name string
fqVersion string // Fully qualified version in format {driverVersion}@{CPO version}
shareProto string
clusterID string

withTopology bool
name string
fqVersion string // Fully qualified version in format {driverVersion}@{CPO version}
shareProto string
clusterID string

serverEndpoint string
fwdEndpoint string
Expand All @@ -80,6 +62,22 @@ type Driver struct {
pvcLister v1.PersistentVolumeClaimLister
}

type DriverOpts struct {
DriverName string
ShareProto string
ClusterID string

WithTopology bool

ServerCSIEndpoint string
FwdCSIEndpoint string

ManilaClientBuilder manilaclient.Builder
CSIClientBuilder csiclient.Builder

PVCLister v1.PersistentVolumeClaimLister
}

type nonBlockingGRPCServer struct {
wg sync.WaitGroup
server *grpc.Server
Expand Down Expand Up @@ -118,8 +116,6 @@ func NewDriver(o *DriverOpts) (*Driver, error) {

d := &Driver{
fqVersion: fmt.Sprintf("%s@%s", driverVersion, version.Version),
nodeID: o.NodeID,
nodeAZ: o.NodeAZ,
withTopology: o.WithTopology,
name: o.DriverName,
serverEndpoint: o.ServerCSIEndpoint,
Expand All @@ -139,7 +135,7 @@ func NewDriver(o *DriverOpts) (*Driver, error) {
klog.Infof("Operating on %s shares", d.shareProto)

if d.withTopology {
klog.Infof("Topology awareness enabled, node availability zone: %s", d.nodeAZ)
klog.Infof("Topology awareness enabled")
} else {
klog.Info("Topology awareness disabled")
}
Expand Down
2 changes: 0 additions & 2 deletions tests/sanity/manila/sanity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ func TestDriver(t *testing.T) {
d, err := manila.NewDriver(
&manila.DriverOpts{
DriverName: "fake.manila.csi.openstack.org",
NodeID: "node",
WithTopology: true,
NodeAZ: "fake-az",
ShareProto: "NFS",
ServerCSIEndpoint: endpoint,
FwdCSIEndpoint: fwdEndpoint,
Expand Down

0 comments on commit 2b20adb

Please sign in to comment.