Skip to content

Expose max concurrent connection config option [full ci] #8678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ whitespace:
@infra/scripts/whitespace-check.sh

# exit 1 if golint complains about anything other than comments
golintf = $(GOLINT) $(1) | sh -c "! grep -v 'lib/apiservers/portlayer/restapi/operations'" | sh -c "! grep -v 'lib/config/dynamic/admiral/client'" | sh -c "! grep -v 'should have comment'" | sh -c "! grep -v 'comment on exported'" | sh -c "! grep -v 'by other packages, and that stutters'" | sh -c "! grep -v 'error strings should not be capitalized'"
golintf = $(GOLINT) $(1) | sh -c "! grep -v 'lib/apiservers/portlayer/restapi/operations'" | sh -c "! grep -v 'lib/apiservers/service/restapi/operations/not_yet_implemented'" | sh -c "! grep -v 'lib/config/dynamic/admiral/client'" | sh -c "! grep -v 'should have comment'" | sh -c "! grep -v 'comment on exported'" | sh -c "! grep -v 'by other packages, and that stutters'" | sh -c "! grep -v 'error strings should not be capitalized'"

golint: $(GOLINT)
@echo checking go lint...
Expand Down
11 changes: 11 additions & 0 deletions cmd/vic-machine/common/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ type Target struct {
Password *string
CloneTicket string
Thumbprint string `cmd:"thumbprint"`

MaxConcurrentConnections *int `cmd:"max-concurrent-connections"`
}

func NewTarget() *Target {
Expand Down Expand Up @@ -67,11 +69,20 @@ 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,
},
}
}

// 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)
}
Expand Down
6 changes: 5 additions & 1 deletion cmd/vic-machine/common/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestFlags(t *testing.T) {
target := NewTarget()
flags := target.TargetFlags()

if len(flags) != 4 {
if len(flags) != 5 {
t.Errorf("Wrong flag numbers")
}
}
Expand All @@ -50,6 +50,10 @@ func TestProcess(t *testing.T) {
User string
Password *string

// MaxConcurrentConnections is not tested here as there's no explicit validation at this stage
// Validation of input type is via flags package, and validation of values is via validator
/// mcc *int

err error
result *url.URL
}{
Expand Down
6 changes: 6 additions & 0 deletions cmd/vic-machine/configure/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"net"
"os"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions cmd/vic-machine/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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("")

Expand Down
2 changes: 1 addition & 1 deletion cmd/vic-machine/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestParseGatewaySpec(t *testing.T) {
func TestFlags(t *testing.T) {
c := NewCreate()
flags := c.Flags()
numberOfFlags := 65
numberOfFlags := 66
assert.Equal(t, numberOfFlags, len(flags), "Missing flags during Create.")
}

Expand Down
6 changes: 3 additions & 3 deletions infra/scripts/bash-helpers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fi
# 1: release numnber, e.g. 1.4.1 or 1.4.1-rc2, or build id, e.g. 19653
vic-set-version () {
version="$1"

if [ "$1" == "" ]; then
unset VIC_VERSION
return
Expand Down Expand Up @@ -63,7 +63,7 @@ init-profile () {

unset-vic () {
unset TARGET_URL MAPPED_NETWORKS NETWORKS IMAGE_STORE DATASTORE COMPUTE VOLUME_STORES IPADDR TLS THUMBPRINT OPS_CREDS VIC_NAME PRESERVE_VOLUMES
unset GOVC_URL GOVC_INSECURE GOVC_DATACENTER GOVC_USERNAME GOVC_PASSWORD GOVC_DATASTORE GOVC_CERTIFICATE
unset GOVC_URL GOVC_INSECURE GOVC_DATACENTER GOVC_USERNAME GOVC_PASSWORD GOVC_DATASTORE GOVC_CERTIFICATE

unset vsphere datacenter user password opsuser opspass opsgrant timeout compute datastore dns publicNet publicIP publicGW bridgeNet bridgeRange
unset clientNet clientIP clientGW managementNet managementIP managementGW tls volumestores preserveVolumestores containernet
Expand All @@ -79,7 +79,7 @@ vic-create () {
base=$(pwd)
(
cd "$(vic-path)"/bin || return
"$(vic-path)/bin/${VIC_VERSION}/vic-machine-$OS" create --target="$TARGET_URL" "${OPS_CREDS[@]}" --image-store="$IMAGE_STORE" --compute-resource="$COMPUTE" "${TLS[@]}" ${TLS_OPTS} --name="${VIC_NAME:-${USER}test}" "${MAPPED_NETWORKS[@]}" "${VOLUME_STORES[@]}" "${NETWORKS[@]}" ${IPADDR} ${TIMEOUT} --thumbprint="$THUMBPRINT" --ai="${VIC_VERSION}/appliance.iso" --bi="${VIC_VERSION}/bootstrap.iso" "$@"
"$(vic-path)/bin/${VIC_VERSION}/vic-machine-$OS" create --target="$TARGET_URL" "${OPS_CREDS[@]}" --image-store="$IMAGE_STORE" --compute-resource="$COMPUTE" "${TLS[@]}" ${TLS_OPTS} --name="${VIC_NAME:-${USER}test}" "${MAPPED_NETWORKS[@]}" "${VOLUME_STORES[@]}" "${NETWORKS[@]}" ${IPADDR} --timeout=${TIMEOUT} --thumbprint="$THUMBPRINT" --ai="${VIC_VERSION}/appliance.iso" --bi="${VIC_VERSION}/bootstrap.iso" "$@"
)

vic-select
Expand Down
2 changes: 1 addition & 1 deletion infra/scripts/focused-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#

if [ -z "$1" ]; then
export REMOTE=$(git remote -v | grep "github.com.vmware/vic.git (fetch)" | awk '{print$1;exit}')/master
export REMOTE=$(git remote -v | grep "github.com.vmware/vic\(\.git\)* (fetch)" | awk '{print$1;exit}')/master
echo "Using ${REMOTE} as default remote"
else
echo "Using ${REMOTE} as specified remote"
Expand Down
37 changes: 36 additions & 1 deletion lib/config/executor/container_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
113 changes: 113 additions & 0 deletions lib/config/executor/container_vm_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
25 changes: 13 additions & 12 deletions lib/config/virtual_container_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
9 changes: 9 additions & 0 deletions lib/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions lib/install/data/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ type InstallerData struct {
ResourcePoolPath string
ApplianceInventoryPath string

MaxConcurrentConnections *int

Datacenter types.ManagedObjectReference
Cluster types.ManagedObjectReference

Expand Down Expand Up @@ -377,5 +379,7 @@ func (d *Data) CopyNonEmpty(src *Data) error {

d.ContainerConfig.ContainerNameConvention = src.ContainerConfig.ContainerNameConvention

d.MaxConcurrentConnections = src.MaxConcurrentConnections

return nil
}
5 changes: 5 additions & 0 deletions lib/install/management/appliance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions lib/install/validate/config_to_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"net/url"
"path"
"strconv"
"strings"

"github.com/docker/go-units"
Expand Down Expand Up @@ -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 {
Expand Down
Loading