From 19cdb4f113853cbbc1a65b456d2561da168e3527 Mon Sep 17 00:00:00 2001 From: Denis Moiseev <1239415+lobziik@users.noreply.github.com> Date: Wed, 11 May 2022 13:23:05 +0200 Subject: [PATCH 1/3] Try to set up client timeout --- pkg/controller/vsphere/session/session.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/controller/vsphere/session/session.go b/pkg/controller/vsphere/session/session.go index 8a590b6292..1555bc53ac 100644 --- a/pkg/controller/vsphere/session/session.go +++ b/pkg/controller/vsphere/session/session.go @@ -22,6 +22,7 @@ import ( "fmt" "net/url" "sync" + "time" "github.com/vmware/govmomi/vapi/rest" "github.com/vmware/govmomi/vim25/mo" @@ -40,6 +41,7 @@ var sessionMU sync.Mutex const ( managedObjectTypeTask = "Task" + clientTimeout = 30 * time.Second ) // Session is a vSphere session with a configured Finder. @@ -85,6 +87,7 @@ func GetOrCreate( // See https://github.com/vmware/govmomi/blob/master/client.go#L91 soapURL.User = nil client, err := govmomi.NewClient(ctx, soapURL, insecure) + client.Timeout = clientTimeout if err != nil { return nil, fmt.Errorf("error setting up new vSphere SOAP client: %w", err) } From e23b27998ef549a36220d1a79f920e9c9aaf3496 Mon Sep 17 00:00:00 2001 From: Denis Moiseev <1239415+lobziik@users.noreply.github.com> Date: Wed, 11 May 2022 15:47:55 +0200 Subject: [PATCH 2/3] Client timeout tests, reduce timeout value to 15s --- pkg/controller/vsphere/session/session.go | 18 ++++-- .../vsphere/session/session_test.go | 64 +++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/pkg/controller/vsphere/session/session.go b/pkg/controller/vsphere/session/session.go index 1555bc53ac..e6820336d6 100644 --- a/pkg/controller/vsphere/session/session.go +++ b/pkg/controller/vsphere/session/session.go @@ -41,7 +41,7 @@ var sessionMU sync.Mutex const ( managedObjectTypeTask = "Task" - clientTimeout = 30 * time.Second + clientTimeout = 15 * time.Second ) // Session is a vSphere session with a configured Finder. @@ -54,6 +54,18 @@ type Session struct { password string } +func newClientWithTimeout(ctx context.Context, u *url.URL, insecure bool, timeout time.Duration) (*govmomi.Client, error) { + clientCreateCtx, clientCreateCtxCancel := context.WithTimeout(ctx, timeout) + defer clientCreateCtxCancel() + // It makes call to vcenter during new client creation, so pass context with timeout there. + client, err := govmomi.NewClient(clientCreateCtx, u, insecure) + if err != nil { + return nil, err + } + client.Timeout = timeout + return client, nil +} + // GetOrCreate gets a cached session or creates a new one if one does not // already exist. func GetOrCreate( @@ -86,12 +98,10 @@ func GetOrCreate( // Set user to nil there for prevent login during client creation. // See https://github.com/vmware/govmomi/blob/master/client.go#L91 soapURL.User = nil - client, err := govmomi.NewClient(ctx, soapURL, insecure) - client.Timeout = clientTimeout + client, err := newClientWithTimeout(ctx, soapURL, insecure, clientTimeout) if err != nil { return nil, fmt.Errorf("error setting up new vSphere SOAP client: %w", err) } - // Set up user agent before login for being able to track mapi component in vcenter sessions list client.UserAgent = "machineAPIvSphereProvider" if err := client.Login(ctx, url.UserPassword(username, password)); err != nil { diff --git a/pkg/controller/vsphere/session/session_test.go b/pkg/controller/vsphere/session/session_test.go index e48c3da833..56d61f3b97 100644 --- a/pkg/controller/vsphere/session/session_test.go +++ b/pkg/controller/vsphere/session/session_test.go @@ -17,9 +17,12 @@ limitations under the License. package session import ( + . "github.com/onsi/gomega" + "context" "crypto/tls" "testing" + "time" "github.com/vmware/govmomi/object" "github.com/vmware/govmomi/simulator" @@ -244,3 +247,64 @@ func TestGetTask(t *testing.T) { }) } } + +func TestClientTimeout(t *testing.T) { + + t.Run("Global delay which not exceeds the timeout", func(t *testing.T) { + g := NewGomegaWithT(t) + model, session, server := initSimulator(t) + defer model.Remove() + defer server.Close() + + model.DelayConfig = simulator.DelayConfig{ + Delay: int(time.Second.Milliseconds()), + } + + simulatorVM := simulator.Map.Any("VirtualMachine").(*simulator.VirtualMachine) + + _, err := session.FindVM(context.TODO(), simulatorVM.Config.Uuid, simulatorVM.Config.Name) + g.Expect(err).ShouldNot(HaveOccurred()) + }) + + t.Run("laggy method", func(t *testing.T) { + g := NewGomegaWithT(t) + model, session, server := initSimulator(t) + defer model.Remove() + defer server.Close() + + model.DelayConfig = simulator.DelayConfig{ + MethodDelay: map[string]int{ + "RetrieveProperties": int(18 * time.Second.Milliseconds()), + }, + } + simulatorVM := simulator.Map.Any("VirtualMachine").(*simulator.VirtualMachine) + + _, err := session.findVMByName(context.TODO(), simulatorVM.Config.Name) + g.Expect(err.Error()).Should(ContainSubstring("unable to find template by name")) + g.Expect(err.Error()).Should(ContainSubstring("context deadline exceeded (Client.Timeout exceeded while awaiting headers)")) + }) + + t.Run("Globally laggy vcenter", func(t *testing.T) { + g := NewGomegaWithT(t) + + model := simulator.VPX() + model.Host = 0 + err := model.Create() + g.Expect(err).NotTo(HaveOccurred()) + model.Service.TLS = new(tls.Config) + model.DelayConfig = simulator.DelayConfig{ + Delay: int(18 * time.Second.Milliseconds()), + } + + server := model.Service.NewServer() + pass, _ := server.URL.User.Password() + + _, err = GetOrCreate( + context.TODO(), + server.URL.Host, "", + server.URL.User.Username(), pass, true) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("error setting up new vSphere SOAP client")) + g.Expect(err.Error()).Should(ContainSubstring("context deadline exceeded")) + }) +} From d258ddc5ec5ee9fc8082ffdc59a1a740b55d9e95 Mon Sep 17 00:00:00 2001 From: Brian Ward Date: Tue, 14 Jun 2022 20:53:06 -0400 Subject: [PATCH 3/3] change ListTags call to ListTagsForCategory --- pkg/controller/vsphere/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/vsphere/reconciler.go b/pkg/controller/vsphere/reconciler.go index 5fd0b5fbc2..f036db046f 100644 --- a/pkg/controller/vsphere/reconciler.go +++ b/pkg/controller/vsphere/reconciler.go @@ -1031,7 +1031,7 @@ func (vm *virtualMachine) checkAttachedTag(ctx context.Context, tagName string, } func (vm *virtualMachine) foundTag(ctx context.Context, tagName string, m *tags.Manager) (bool, error) { - tags, err := m.ListTags(ctx) + tags, err := m.ListTagsForCategory(ctx,"openshift-"+tagName) if err != nil { return false, err }