From ef8555b7423740c2c38ed679a2bd771720638ce4 Mon Sep 17 00:00:00 2001 From: Divyen Patel Date: Fri, 6 Feb 2026 16:07:57 -0800 Subject: [PATCH] Fix Datacenter discovery failure after rename when MoRefs are used When a Datacenter is renamed in vCenter, the CSI driver's syncer container can fail to discover or register nodes with errors like "datacenter '/path' not found", even if the configuration uses a MoRef. This happens because the core discovery logic was strictly path-based and would resolve MoRefs to paths, which then became stale after a rename. This commit updates the core VirtualCenter discovery logic to be explicitly MoRef-aware. It adds support for both the full MoRef format (e.g., Datacenter:datacenter-3) and the raw MoID format used in Supervisor clusters (e.g., datacenter-40). By anchoring the lookup to the immutable MoID, the driver remains resilient to Datacenter renames. Strict regex matching is used to distinguish between MoRefs and inventory paths to ensure that paths starting with "datacenter-" are still handled correctly by the path-based finder. --- pkg/common/cns-lib/vsphere/virtualcenter.go | 37 ++++- .../cns-lib/vsphere/virtualcenter_test.go | 155 ++++++++++++++++++ 2 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 pkg/common/cns-lib/vsphere/virtualcenter_test.go diff --git a/pkg/common/cns-lib/vsphere/virtualcenter.go b/pkg/common/cns-lib/vsphere/virtualcenter.go index 32ecb13d40..a87470714c 100644 --- a/pkg/common/cns-lib/vsphere/virtualcenter.go +++ b/pkg/common/cns-lib/vsphere/virtualcenter.go @@ -25,7 +25,9 @@ import ( "net" neturl "net/url" "reflect" + "regexp" "strconv" + "strings" "sync" "time" @@ -443,17 +445,38 @@ func (vc *VirtualCenter) ListDatacenters(ctx context.Context) ( return dcs, nil } -// getDatacenters returns Datacenter instances given their paths. -func (vc *VirtualCenter) getDatacenters(ctx context.Context, dcPaths []string) ( +// getDatacenters returns Datacenter instances given their paths or morefs. +func (vc *VirtualCenter) getDatacenters(ctx context.Context, dcPathsOrMorefs []string) ( []*Datacenter, error) { log := logger.GetLogger(ctx) finder := find.NewFinder(vc.Client.Client, false) var dcs []*Datacenter - for _, dcPath := range dcPaths { - dcObj, err := finder.Datacenter(ctx, dcPath) - if err != nil { - log.Errorf("failed to fetch datacenter given dcPath %s with err: %v", dcPath, err) - return nil, err + dcMoIDRegex := regexp.MustCompile(`^datacenter-\d+$`) + dcMoRefRegex := regexp.MustCompile(`^Datacenter:datacenter-\d+$`) + for _, dcPathOrMoref := range dcPathsOrMorefs { + var dcObj *object.Datacenter + var err error + // Handle both "Datacenter:datacenter-N" and raw "datacenter-N" formats + if dcMoRefRegex.MatchString(dcPathOrMoref) || dcMoIDRegex.MatchString(dcPathOrMoref) { + log.Infof("Fetching datacenter given dcMoref %s", dcPathOrMoref) + moID := strings.TrimPrefix(dcPathOrMoref, "Datacenter:") + dcObj = object.NewDatacenter(vc.Client.Client, types.ManagedObjectReference{ + Type: "Datacenter", + Value: moID, + }) + // Verify if the datacenter exists. + var dc mo.Datacenter + err = dcObj.Properties(ctx, dcObj.Reference(), []string{"name"}, &dc) + if err != nil { + log.Errorf("failed to fetch datacenter given dcMoref %s with err: %v", dcPathOrMoref, err) + return nil, err + } + } else { + dcObj, err = finder.Datacenter(ctx, dcPathOrMoref) + if err != nil { + log.Errorf("failed to fetch datacenter given dcPath %s with err: %v", dcPathOrMoref, err) + return nil, err + } } dc := &Datacenter{Datacenter: dcObj, VirtualCenterHost: vc.Config.Host} dcs = append(dcs, dc) diff --git a/pkg/common/cns-lib/vsphere/virtualcenter_test.go b/pkg/common/cns-lib/vsphere/virtualcenter_test.go new file mode 100644 index 0000000000..b1229b055d --- /dev/null +++ b/pkg/common/cns-lib/vsphere/virtualcenter_test.go @@ -0,0 +1,155 @@ +/* +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 vsphere + +import ( + "context" + "strings" + "sync" + "testing" + + "github.com/vmware/govmomi" + "github.com/vmware/govmomi/find" + "github.com/vmware/govmomi/simulator" +) + +func TestGetDatacentersWithMoRef(t *testing.T) { + ctx := context.Background() + + // 1. Setup vcsim + model := simulator.VPX() + model.Datacenter = 1 + err := model.Create() + if err != nil { + t.Fatal(err) + } + defer model.Remove() + + s := model.Service.NewServer() + defer s.Close() + + client, err := govmomi.NewClient(ctx, s.URL, true) + if err != nil { + t.Fatal(err) + } + + // 2. Create VirtualCenter instance + vc := &VirtualCenter{ + Config: &VirtualCenterConfig{ + Host: s.URL.Host, + }, + Client: client, + ClientMutex: &sync.Mutex{}, + } + + // Get the initial datacenter to find its MoRef and Path + finder := find.NewFinder(client.Client, false) + dcs, err := finder.DatacenterList(ctx, "*") + if err != nil || len(dcs) == 0 { + t.Fatalf("failed to list datacenters: %v", err) + } + dcObj := dcs[0] + dcMoRef := dcObj.Reference().String() // e.g. "Datacenter:datacenter-2" + dcMoID := dcObj.Reference().Value // e.g. "datacenter-2" + dcPath := dcObj.InventoryPath // e.g. "/DC0" + + t.Logf("Initial Datacenter: Path=%s, MoRef=%s, MoID=%s", dcPath, dcMoRef, dcMoID) + + // 3. Test with Path (should succeed initially) + vc.Config.DatacenterPaths = []string{dcPath} + resolvedDCs, err := vc.getDatacenters(ctx, vc.Config.DatacenterPaths) + if err != nil { + t.Errorf("getDatacenters with path failed: %v", err) + } + if len(resolvedDCs) != 1 || resolvedDCs[0].InventoryPath != dcPath { + t.Errorf("Unexpected resolved datacenter with path: %+v", resolvedDCs) + } + + // 4. Rename the Datacenter + newName := "RenamedDC" + task, err := dcObj.Rename(ctx, newName) + if err != nil { + t.Fatalf("failed to rename datacenter: %v", err) + } + err = task.Wait(ctx) + if err != nil { + t.Fatalf("failed to wait for rename task: %v", err) + } + newPath := strings.Replace(dcPath, "DC0", newName, 1) + t.Logf("Renamed Datacenter to %s, New Path=%s", newName, newPath) + + // 5. Test with OLD Path (should fail now) + vc.Config.DatacenterPaths = []string{dcPath} + _, err = vc.getDatacenters(ctx, vc.Config.DatacenterPaths) + if err == nil { + t.Error("getDatacenters with old path should have failed but succeeded") + } else { + t.Logf("Expected failure with old path: %v", err) + } + + // 6. Test with MoRef (should succeed despite rename) + vc.Config.DatacenterPaths = []string{dcMoRef} + resolvedDCs, err = vc.getDatacenters(ctx, vc.Config.DatacenterPaths) + if err != nil { + t.Errorf("getDatacenters with MoRef failed after rename: %v", err) + } else if len(resolvedDCs) != 1 { + t.Errorf("Expected 1 datacenter with MoRef, got %d", len(resolvedDCs)) + } else { + // Verify the name is correct + name, err := resolvedDCs[0].ObjectName(ctx) + if err != nil { + t.Errorf("failed to get name for resolved DC: %v", err) + } else { + t.Logf("Resolved DC name via MoRef: %s", name) + if name != newName { + t.Errorf("Expected name to be %s, got %s", newName, name) + } + } + } + + // 7. Test with raw MoID (Supervisor format) + vc.Config.DatacenterPaths = []string{dcMoID} + resolvedDCs, err = vc.getDatacenters(ctx, vc.Config.DatacenterPaths) + if err != nil { + t.Errorf("getDatacenters with raw MoID failed after rename: %v", err) + } else if len(resolvedDCs) != 1 { + t.Errorf("Expected 1 datacenter with raw MoID, got %d", len(resolvedDCs)) + } else { + name, err := resolvedDCs[0].ObjectName(ctx) + if err != nil { + t.Errorf("failed to get name for resolved DC: %v", err) + } else { + t.Logf("Resolved DC name via raw MoID: %s", name) + if name != newName { + t.Errorf("Expected name to be %s, got %s", newName, name) + } + } + } + + // 8. Test with a path that starts with "datacenter-" but isn't a MoID + fakePath := "datacenter-not-a-real-id/something" + vc.Config.DatacenterPaths = []string{fakePath} + _, err = vc.getDatacenters(ctx, vc.Config.DatacenterPaths) + if err == nil { + t.Error("getDatacenters with fake path starting with 'datacenter-' should have failed") + } else { + t.Logf("Expected failure with fake path: %v", err) + if !strings.Contains(err.Error(), "not found") { + t.Errorf("Expected 'not found' error for fake path, got: %v", err) + } + } +}