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 } diff --git a/pkg/controller/vsphere/session/session.go b/pkg/controller/vsphere/session/session.go index 8a590b6292..e6820336d6 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 = 15 * time.Second ) // Session is a vSphere session with a configured Finder. @@ -52,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( @@ -84,11 +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, 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")) + }) +}