vmware / govmomi

Go library for the VMware vSphere API
Apache License 2.0
2.3k stars 910 forks source link

[WIP] Remove the code to change the Namespace to vim25 in NewClient function. #3405

Closed lipingxue closed 5 months ago

lipingxue commented 5 months ago

Description

This change is to remove code to change the Namespace to vim25 in NewClient function. This is not needed and other client does not have this logic. Closes: #(issue-number)

Type of change

Please mark options that are relevant:

How Has This Been Tested?

Run the unit test for cns client, and it passed.

lipingx@lipingxNMD6R govmomi % pwd
/Users/lipingx/go/src/github.com/lipingxue/govmomi

lipingx@lipingxNMD6R cns %  go test -v
=== RUN   TestClient
    client_test.go:158: Creating volume using the spec: types.CnsVolumeCreateSpec{
            Name:       "pvc-901e87eb-c2bd-11e9-806f-005056a0c9a0",
            VolumeType: "BLOCK",
            Datastores: []types.ManagedObjectReference{
                {Type:"Datastore", Value:"datastore-48", ServerGUID:""},
            },
            Metadata: types.CnsVolumeMetadata{
                ContainerCluster: types.CnsContainerCluster{
                    ClusterType:         "KUBERNETES",
                    ClusterId:           "demo-cluster-id",
                    VSphereUser:         "Administrator@vsphere.local",
                    ClusterFlavor:       "VANILLA",
                    ClusterDistribution: "OpenShift",
                },
                EntityMetadata:        nil,
                ContainerClusterArray: nil,
            },
            BackingObjectDetails: &types.CnsBlockBackingDetails{
                CnsBackingObjectDetails: types.CnsBackingObjectDetails{
                    CapacityInMb: 5120,
                },
                BackingDiskId:                  "",
                BackingDiskUrlPath:             "",
                BackingDiskObjectId:            "",
                AggregatedSnapshotCapacityInMb: 0,
            },
            Profile:      nil,
            CreateSpec:   nil,
            VolumeSource: nil,
        }
    client_test.go:184: volumeCreateResult &{CnsVolumeOperationResult:{DynamicData:{} VolumeId:{DynamicData:{} Id:da92fb45-9308-43b7-84a4-2d5b44d2c0e0} Fault:<nil>} Name:pvc-901e87eb-c2bd-11e9-806f-005056a0c9a0 PlacementResults:[{Datastore:Datastore:datastore-48 PlacementFaults:[]}]}
    client_test.go:185: Volume created sucessfully. volumeId: da92fb45-9308-43b7-84a4-2d5b44d2c0e0

...

--- PASS: TestClient (23.62s)
PASS
ok      github.com/vmware/govmomi/cns   23.927s

Checklist:

divyenpatel commented 5 months ago

we set version for vim client as below.
https://github.com/vmware/govmomi/blob/main/cns/client_test.go#L104C2-L104C29

    c.UseServiceVersion("vsan")
    cnsClient, err := NewClient(ctx, c.Client)

Can we remove c.UseServiceVersion("vsan") and execute tests against vCenter?

lipingxue commented 5 months ago

This change is not needed. The issue has been addressed by this MR https://github.com/vmware/govmomi/pull/3406