From 7dfe7bf3ac68d394fa35e0d23eda699146f434ae Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Mon, 22 Jun 2026 23:57:23 -0700 Subject: [PATCH] [manila-csi-plugin] Restore --nodeid and --nodeaz as optional overrides Un-deprecate the --nodeid and --nodeaz flags so that they can be used as alternatives to the metadata service for node identity. When set, the flag values are used directly; when unset, the driver falls back to the metadata service as before. This allows Manila CSI to work on nodes that are not OpenStack VMs, such as bare metal clusters or edge deployments, where the metadata service is unavailable. --- cmd/manila-csi-plugin/main.go | 21 ++--- pkg/csi/manila/driver.go | 45 +++++++++- pkg/csi/manila/metadata_test.go | 135 +++++++++++++++++++++++++++++ tests/sanity/manila/sanity_test.go | 2 +- 4 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 pkg/csi/manila/metadata_test.go diff --git a/cmd/manila-csi-plugin/main.go b/cmd/manila-csi-plugin/main.go index f1505b3f98..7557424f53 100644 --- a/cmd/manila-csi-plugin/main.go +++ b/cmd/manila-csi-plugin/main.go @@ -88,6 +88,8 @@ func main() { ManilaClientBuilder: manilaClientBuilder, CSIClientBuilder: csiClientBuilder, ClusterID: clusterID, + NodeID: nodeID, + NodeAZ: nodeAZ, PVCLister: csi.GetPVCLister(), } @@ -104,10 +106,12 @@ func main() { } if provideNodeService { - // Initialize metadata - metadata := metadata.GetMetadataProvider("") + var md metadata.IMetadata + if nodeID == "" || (withTopology && nodeAZ == "") { + md = metadata.GetMetadataProvider("") + } - err = d.SetupNodeService(metadata) + err = d.SetupNodeService(nodeID, nodeAZ, md) if err != nil { klog.Fatalf("Driver node service initialization failed: %v", err) } @@ -126,15 +130,8 @@ func main() { cmd.PersistentFlags().StringVar(&driverName, "drivername", "manila.csi.openstack.org", "name of the driver") - 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(&nodeID, "nodeid", "", "this node's ID. When set, the metadata service is not used to retrieve the node ID.") + cmd.PersistentFlags().StringVar(&nodeAZ, "nodeaz", "", "this node's availability zone. When set, the metadata service is not used to retrieve the availability zone.") cmd.PersistentFlags().StringVar(&runtimeConfigFile, "runtime-config-file", "", "path to the runtime configuration file") diff --git a/pkg/csi/manila/driver.go b/pkg/csi/manila/driver.go index 30d1b1f5d3..04603106ad 100644 --- a/pkg/csi/manila/driver.go +++ b/pkg/csi/manila/driver.go @@ -72,12 +72,43 @@ type DriverOpts struct { ServerCSIEndpoint string FwdCSIEndpoint string + NodeID string + NodeAZ string + ManilaClientBuilder manilaclient.Builder CSIClientBuilder csiclient.Builder PVCLister v1.PersistentVolumeClaimLister } +type staticMetadata struct { + nodeID string + nodeAZ string +} + +func (m *staticMetadata) GetInstanceID() (string, error) { return m.nodeID, nil } +func (m *staticMetadata) GetAvailabilityZone() (string, error) { return m.nodeAZ, nil } + +type overrideMetadata struct { + nodeID string + nodeAZ string + fallback metadata.IMetadata +} + +func (m *overrideMetadata) GetInstanceID() (string, error) { + if m.nodeID != "" { + return m.nodeID, nil + } + return m.fallback.GetInstanceID() +} + +func (m *overrideMetadata) GetAvailabilityZone() (string, error) { + if m.nodeAZ != "" { + return m.nodeAZ, nil + } + return m.fallback.GetAvailabilityZone() +} + type nonBlockingGRPCServer struct { wg sync.WaitGroup server *grpc.Server @@ -174,9 +205,19 @@ func (d *Driver) SetupControllerService() error { return nil } -func (d *Driver) SetupNodeService(metadata metadata.IMetadata) error { +func (d *Driver) SetupNodeService(nodeID, nodeAZ string, md metadata.IMetadata) error { klog.Info("Providing node service") + var effectiveMD metadata.IMetadata + switch { + case nodeID != "" && nodeAZ != "": + effectiveMD = &staticMetadata{nodeID: nodeID, nodeAZ: nodeAZ} + case nodeID != "" || nodeAZ != "": + effectiveMD = &overrideMetadata{nodeID: nodeID, nodeAZ: nodeAZ, fallback: md} + default: + effectiveMD = md + } + var supportsNodeStage bool nodeCapsMap, err := d.initProxiedDriver() @@ -196,7 +237,7 @@ func (d *Driver) SetupNodeService(metadata metadata.IMetadata) error { d.ns = &nodeServer{ d: d, - metadata: metadata, + metadata: effectiveMD, supportsNodeStage: supportsNodeStage, nodeStageCache: make(map[volumeID]stageCacheEntry), } diff --git a/pkg/csi/manila/metadata_test.go b/pkg/csi/manila/metadata_test.go new file mode 100644 index 0000000000..90d066bdb4 --- /dev/null +++ b/pkg/csi/manila/metadata_test.go @@ -0,0 +1,135 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package manila + +import ( + "errors" + "testing" +) + +type fakeMetadataProvider struct { + instanceID string + availabilityZone string + err error +} + +func (f *fakeMetadataProvider) GetInstanceID() (string, error) { + if f.err != nil { + return "", f.err + } + return f.instanceID, nil +} + +func (f *fakeMetadataProvider) GetAvailabilityZone() (string, error) { + if f.err != nil { + return "", f.err + } + return f.availabilityZone, nil +} + +func TestStaticMetadata(t *testing.T) { + tests := []struct { + name string + nodeID string + nodeAZ string + wantID string + wantAZ string + }{ + {"both set", "my-node", "az1", "my-node", "az1"}, + {"empty values", "", "", "", ""}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + m := &staticMetadata{nodeID: tc.nodeID, nodeAZ: tc.nodeAZ} + + id, err := m.GetInstanceID() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if id != tc.wantID { + t.Errorf("expected node ID %q, got %q", tc.wantID, id) + } + + az, err := m.GetAvailabilityZone() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if az != tc.wantAZ { + t.Errorf("expected AZ %q, got %q", tc.wantAZ, az) + } + }) + } +} + +func TestOverrideMetadata(t *testing.T) { + tests := []struct { + name string + nodeID string + nodeAZ string + fallback *fakeMetadataProvider + wantID string + wantAZ string + }{ + { + "nodeID set, AZ from fallback", + "flag-id", "", + &fakeMetadataProvider{instanceID: "meta-id", availabilityZone: "meta-az"}, + "flag-id", "meta-az", + }, + { + "nodeAZ set, ID from fallback", + "", "flag-az", + &fakeMetadataProvider{instanceID: "meta-id", availabilityZone: "meta-az"}, + "meta-id", "flag-az", + }, + { + "both set, fallback not called", + "flag-id", "flag-az", + &fakeMetadataProvider{err: errors.New("should not be called")}, + "flag-id", "flag-az", + }, + { + "neither set, both from fallback", + "", "", + &fakeMetadataProvider{instanceID: "meta-id", availabilityZone: "meta-az"}, + "meta-id", "meta-az", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + m := &overrideMetadata{nodeID: tc.nodeID, nodeAZ: tc.nodeAZ, fallback: tc.fallback} + + id, err := m.GetInstanceID() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if id != tc.wantID { + t.Errorf("expected node ID %q, got %q", tc.wantID, id) + } + + az, err := m.GetAvailabilityZone() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if az != tc.wantAZ { + t.Errorf("expected AZ %q, got %q", tc.wantAZ, az) + } + }) + } +} diff --git a/tests/sanity/manila/sanity_test.go b/tests/sanity/manila/sanity_test.go index 0219e2b894..8218ded280 100644 --- a/tests/sanity/manila/sanity_test.go +++ b/tests/sanity/manila/sanity_test.go @@ -54,7 +54,7 @@ func TestDriver(t *testing.T) { fakemeta := &fakemetadata{} - err = d.SetupNodeService(fakemeta) + err = d.SetupNodeService("", "", fakemeta) if err != nil { t.Fatalf("Failed to initialize CSI Manila node service: %v", err) }