Expose max concurrent connection config option

This adds an option to set the maximum number of concurrent connections
to vCenter that will be used by the port layer.
This does NOT effect the admin server, and admin server connections do
not count towards the maximum for the port layer.

This adds a --max-concurrent-connections (--mcc short form) option to
create and configure, allowing setting between 2 (because port layer
requires 2 minimum to initialize) and 255 (because it's a sane upper
limit, given default has been 32 since 2017, but with no reason beyond
that).

This has NOT made the necessary changes for this option to be available
via the VIC LCM API and vCenter UI extension.

Testing so far has been via manual use of CLI.

cmd/vic-machine/common/target.go

@@ -33,6 +33,8 @@ type Target struct {
 	Password    *string
 	CloneTicket string
 	Thumbprint  string `cmd:"thumbprint"`
+
+	MaxConcurrentConnections *int `cmd:"max-concurrent-connections"`
 }
 
 func NewTarget() *Target {
@@ -67,11 +69,30 @@ func (t *Target) TargetFlags() []cli.Flag {
 			Usage:       "ESX or vCenter host certificate thumbprint",
 			EnvVar:      "VIC_MACHINE_THUMBPRINT",
 		},
+		cli.GenericFlag{
+			Name:   "max-concurrent-connections, mcc",
+			Value:  flags.NewOptionalInt(&t.MaxConcurrentConnections),
+			Usage:  "Determines maximum number of connections (not sessions) to vCenter. Integer value, greater than 0",
+			EnvVar: "VIC_MACHINE_MAX_CONCURRENT_CONNECTIONS",
+			Hidden: true,
+		},
+		/*
+			cli.IntFlag{
+				Name:        "max-concurrent-connections, mcc",
+				Value:       constants.DefaultMaxConcurrentRequests,
+				Destination: &t.MaxConcurrentConnections,
+				Usage:       "Determines maximum number of connections (not sessions) to vCenter. Integer value, greater than 0",
+				EnvVar:      "VIC_MACHINE_MAX_CONCURRENT_CONNECTIONS",
+				Hidden:      true,
+			},
+		*/
 	}
 }
 
 // HasCredentials check that the credentials have been supplied by any of the permitted mechanisms
 func (t *Target) HasCredentials(op trace.Operation) error {
+	defer trace.End(trace.Begin("", op))
+
 	if t.URL == nil {
 		return cli.NewExitError("--target argument must be specified", 1)
 	}

cmd/vic-machine/configure/configure.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -214,6 +215,11 @@ func (c *Configure) copyChangedConf(o *config.VirtualContainerHostConfigSpec, n
 		updateSessionEnv(portlayerSession, config.PortLayerDCPath, v.Session().Datacenter.Name())
 	}
 
+	// if mcc is explicitly set, apply it
+	if c.Data.MaxConcurrentConnections != nil {
+		updateSessionEnv(portlayerSession, config.PortLayerMaxConcurrentConnections, strconv.Itoa(*c.Data.MaxConcurrentConnections))
+	}
+
 	if c.Debug.Debug != nil {
 		o.SetDebug(n.Diagnostics.DebugLevel)
 	}

cmd/vic-machine/create/create.go

@@ -719,6 +719,8 @@ func (c *Create) Run(clic *cli.Context) (err error) {
 
 	vConfig.Timeout = c.Data.Timeout
 
+	vConfig.MaxConcurrentConnections = c.MaxConcurrentConnections
+
 	// separate initial validation from dispatch of creation task
 	op.Info("")
 

lib/config/executor/container_vm.go

@@ -119,7 +119,8 @@ type ExitLog struct {
 // MountSpec details a mount that must be executed within the executor
 // A mount is a URI -> path mapping with a credential of some kind
 // In the case of a labeled disk:
-// 	label://<label name> => </mnt/path>
+//
+//	label://<label name> => </mnt/path>
 type MountSpec struct {
 	// A URI->path mapping, e.g.
 	// May contain credentials
@@ -283,6 +284,40 @@ type SessionConfig struct {
 	Detail `vic:"0.1" scope:"read-write" key:"detail"`
 }
 
+// GetEnv returns
+// * the value of the specified environment variable if present
+// * nil if not present
+func (sc *SessionConfig) GetEnv(name string) *string {
+	if sc == nil {
+		return nil
+	}
+
+	for _, env := range sc.Cmd.Env {
+		if strings.HasPrefix(env, name+"=") {
+			val := strings.TrimPrefix(env, name+"=")
+			return &val
+		}
+	}
+
+	return nil
+}
+
+// SetEnv sets the value of the specified environment variable, returning:
+// * the old value if present
+// * nil if not present
+func (sc *SessionConfig) SetEnv(name, value string) *string {
+	for i, env := range sc.Cmd.Env {
+		if strings.HasPrefix(env, name+"=") {
+			sc.Cmd.Env[i] = name + "=" + value
+			val := strings.TrimPrefix(env, name+"=")
+			return &val
+		}
+	}
+
+	sc.Cmd.Env = append(sc.Cmd.Env, name+"="+value)
+	return nil
+}
+
 type Detail struct {
 
 	// creation, started & stopped timestamps

lib/config/executor/container_vm_test.go

@@ -0,0 +1,113 @@
+// Copyright 2016-2018 VMware, Inc. All Rights Reserved.
+//
+// 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 executor
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetEnv(t *testing.T) {
+	sess := SessionConfig{
+		Cmd: Cmd{
+			Env: []string{
+				"hello=world",
+				"goodbye=",
+			},
+		},
+	}
+
+	assert.Equal(t, "world", *sess.GetEnv("hello"), "Get on set variable should return set value")
+	assert.Equal(t, "", *sess.GetEnv("goodbye"), "Get on set variable with empty value should return empty string")
+	assert.Nil(t, sess.GetEnv("nope"), "Get on an unset variable should return nil")
+}
+
+func TestSetEnvUpdateValue(t *testing.T) {
+	sess := SessionConfig{
+		Cmd: Cmd{
+			Env: []string{
+				"hello=world",
+				"goodbye=",
+			},
+		},
+	}
+
+	require.Equal(t, "world", *sess.GetEnv("hello"), "Get on set variable should return set value")
+	require.Equal(t, "", *sess.GetEnv("goodbye"), "Get on set variable with empty value should return empty string")
+
+	newVal := "sapients"
+	old := *sess.SetEnv("hello", newVal)
+
+	assert.Equal(t, "world", old, "Expected old value to be return on update")
+	assert.Equal(t, newVal, *sess.GetEnv("hello"), "Expected new value to be updated value")
+
+	assert.Equal(t, "", *sess.GetEnv("goodbye"), "Expected unmodified value not to have changed")
+}
+
+func TestSetEnvEmptyValue(t *testing.T) {
+	sess := SessionConfig{
+		Cmd: Cmd{
+			Env: []string{
+				"hello=world",
+				"goodbye=",
+			},
+		},
+	}
+
+	require.Equal(t, "world", *sess.GetEnv("hello"), "Get on set variable should return set value")
+	require.Equal(t, "", *sess.GetEnv("goodbye"), "Get on set variable with empty value should return empty string")
+
+	newVal := "sapients"
+	old := *sess.SetEnv("goodbye", newVal)
+
+	assert.Equal(t, "", old, "Expected old value to be return on update")
+	assert.Equal(t, newVal, *sess.GetEnv("goodbye"), "Expected new value to be updated value")
+
+	assert.Equal(t, "world", *sess.GetEnv("hello"), "Expected unmodified value not to have changed")
+}
+
+func TestSetEnvNewEnv(t *testing.T) {
+	sess := SessionConfig{
+		Cmd: Cmd{
+			Env: []string{
+				"hello=world",
+				"goodbye=",
+			},
+		},
+	}
+
+	require.Equal(t, "world", *sess.GetEnv("hello"), "Get on set variable should return set value")
+	require.Equal(t, "", *sess.GetEnv("goodbye"), "Get on set variable with empty value should return empty string")
+
+	newKey := "solo"
+	require.Nil(t, sess.GetEnv(newKey), "Expected nil return for unset value")
+
+	// checking we can set a new env with value of empty string
+	newVal := ""
+	_ = sess.SetEnv(newKey, newVal)
+	assert.Equal(t, newVal, *sess.GetEnv(newKey), "Expected new value to be updated value")
+
+	// checking we can set a new env with value specified
+	newKey = "absence"
+	newVal = "fonder"
+
+	_ = sess.SetEnv(newKey, newVal)
+	assert.Equal(t, newVal, *sess.GetEnv(newKey), "Expected new value to be updated value")
+
+	assert.Equal(t, "world", *sess.GetEnv("hello"), "Expected unmodified value not to have changed")
+	assert.Equal(t, "", *sess.GetEnv("goodbye"), "Expected unmodified value not to have changed")
+}

lib/config/virtual_container_host.go

@@ -54,18 +54,19 @@ const (
 	PortLayerService = "port-layer"
 	VicAdminService  = "vicadmin"
 
-	GeneralHTTPProxy   = "HTTP_PROXY"
-	GeneralHTTPSProxy  = "HTTPS_PROXY"
-	GeneralNoProxy     = "NO_PROXY"
-	VICAdminHTTPProxy  = "VICADMIN_HTTP_PROXY"
-	VICAdminHTTPSProxy = "VICADMIN_HTTPS_PROXY"
-	VICAdminNoProxy    = "NO_PROXY"
-	VICAdminCSPath     = "--cluster"
-	VICAdminPoolPath   = "--pool"
-	VICAdminDCPath     = "--dc"
-	PortLayerCSPath    = "CS_PATH"
-	PortLayerPoolPath  = "POOL_PATH"
-	PortLayerDCPath    = "DC_PATH"
+	GeneralHTTPProxy                  = "HTTP_PROXY"
+	GeneralHTTPSProxy                 = "HTTPS_PROXY"
+	GeneralNoProxy                    = "NO_PROXY"
+	VICAdminHTTPProxy                 = "VICADMIN_HTTP_PROXY"
+	VICAdminHTTPSProxy                = "VICADMIN_HTTPS_PROXY"
+	VICAdminNoProxy                   = "NO_PROXY"
+	VICAdminCSPath                    = "--cluster"
+	VICAdminPoolPath                  = "--pool"
+	VICAdminDCPath                    = "--dc"
+	PortLayerCSPath                   = "CS_PATH"
+	PortLayerPoolPath                 = "POOL_PATH"
+	PortLayerDCPath                   = "DC_PATH"
+	PortLayerMaxConcurrentConnections = "VIC_MAX_IN_FLIGHT"
 
 	AddPerms = "ADD"
 )

lib/constants/constants.go

@@ -84,6 +84,15 @@ const (
 	TaskCreatedState = "created"
 	TaskFailedState  = "failed"
 	TaskUnknownState = "unknown"
+
+	// DefaultMaxConcurrentRequests limits the maximum number of SOAP operations in flight
+	// This has the effect of modifying the MaxIdleConnsPerHost field of the soap transport
+	DefaultMaxConcurrentRequests = 32
+	// MaximumMaxConcurrentRequests limits the upper bound that can be specified for concurrent connections
+	MaximumMaxConcurrentRequests = 255
+	// MinMaxConcurrentRequests is necessary because testing shows the port layer doesn't fully initialize if
+	// lower than this
+	MinMaxConcurrentRequests = 2
 )
 
 func DefaultAltVCHGuestName() string {

lib/install/data/data.go

@@ -127,6 +127,8 @@ type InstallerData struct {
 	ResourcePoolPath       string
 	ApplianceInventoryPath string
 
+	MaxConcurrentConnections *int
+
 	Datacenter types.ManagedObjectReference
 	Cluster    types.ManagedObjectReference
 
@@ -377,5 +379,7 @@ func (d *Data) CopyNonEmpty(src *Data) error {
 
 	d.ContainerConfig.ContainerNameConvention = src.ContainerConfig.ContainerNameConvention
 
+	d.MaxConcurrentConnections = src.MaxConcurrentConnections
+
 	return nil
 }

lib/install/management/appliance.go

@@ -693,6 +693,11 @@ func (d *Dispatcher) createAppliance(conf *config.VirtualContainerHostConfigSpec
 		Active:  true,
 	}
 
+	if settings.MaxConcurrentConnections != nil {
+		d.op.Debugf("Setting max concurrent connections in portlayer environment to %d", *settings.MaxConcurrentConnections)
+		cfg.Cmd.Env = append(cfg.Cmd.Env, fmt.Sprintf("%s=%s", config.PortLayerMaxConcurrentConnections, strconv.Itoa(*settings.MaxConcurrentConnections)))
+	}
+
 	conf.AddComponent(config.PortLayerService, cfg)
 
 	// fix up those parts of the config that depend on the final applianceVM folder name

lib/install/validate/config_to_data.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"net/url"
 	"path"
+	"strconv"
 	"strings"
 
 	"github.com/docker/go-units"
@@ -162,6 +163,16 @@ func NewDataFromConfig(ctx context.Context, finder Finder, conf *config.VirtualC
 	d.OpsCredentials.OpsPassword = &conf.Connection.Token
 	d.Thumbprint = conf.Connection.TargetThumbprint
 
+	maxConcurrent := conf.ExecutorConfig.Sessions[config.PortLayerService].GetEnv(config.PortLayerMaxConcurrentConnections)
+	if maxConcurrent != nil {
+		var max int
+		max, err = strconv.Atoi(*maxConcurrent)
+		if err != nil {
+			return
+		}
+		d.MaxConcurrentConnections = &max
+	}
+
 	d.AsymmetricRouting = conf.AsymmetricRouting
 
 	if err = setBridgeNetwork(op, finder, d, conf); err != nil {

lib/install/validate/validator.go

@@ -490,22 +490,25 @@ func (v *Validator) sessionValid(op trace.Operation, errMsg string) bool {
 		v.NoteIssue(errors.New(errMsg))
 		return false
 	}
+
 	return true
 }
 
 func (v *Validator) target(op trace.Operation, input *data.Data, conf *config.VirtualContainerHostConfigSpec) {
 	defer trace.End(trace.Begin("", op))
 
-	// check if host is managed by VC
 	v.managedbyVC(op)
+	v.connectionManagement(op, input, conf)
 }
 
+// managedbyVC checks if host is managed by VC and is blocked from direct interaction
 func (v *Validator) managedbyVC(op trace.Operation) {
 	defer trace.End(trace.Begin("", op))
 
 	if v.isVC() {
 		return
 	}
+
 	host, err := v.session.Finder.DefaultHostSystem(op)
 	if err != nil {
 		v.NoteIssue(fmt.Errorf("Failed to get host system: %s", err))
@@ -522,6 +525,26 @@ func (v *Validator) managedbyVC(op trace.Operation) {
 	if ip := mh.Summary.ManagementServerIp; ip != "" {
 		v.NoteIssue(fmt.Errorf("Target is managed by vCenter server %q, please change --target to vCenter server address or select a standalone ESXi", ip))
 	}
+
+	return
+}
+
+func (v *Validator) connectionManagement(op trace.Operation, input *data.Data, conf *config.VirtualContainerHostConfigSpec) {
+	defer trace.End(trace.Begin("", op))
+
+	if input.MaxConcurrentConnections == nil {
+		return
+	}
+
+	// unsure if we need to set a maximum bound as well. Adding something sane, but above the
+	// default that's been in force since 2017
+	if *input.MaxConcurrentConnections < constants.MinMaxConcurrentRequests || *input.MaxConcurrentConnections > constants.MaximumMaxConcurrentRequests {
+		op.Debugf("Validating max connection failed")
+
+		v.NoteIssue(fmt.Errorf("Maximum concurrent connections must be between %d - %d inclusive", constants.MinMaxConcurrentRequests, constants.MaximumMaxConcurrentRequests))
+		return
+	}
+
 	return
 }