Initial changes for distributed jmeter executor #4282
Conversation
| } | ||
| } | ||
|
|
||
| sl, err := slaves.NewClient(execution, r.Params, slaves.SlavesEnvOptions{RunPath: runPath, OverrideJvmArgs: "-Xmn256m -Xms512m -Xmx512m"}) |
There was a problem hiding this comment.
JVM args shouldn't be hardcoded, we should be able to override them
| return *result.WithErrors(errors.Errorf("Getting error while creating slaves nodes: %v", err)), nil | ||
| } | ||
|
|
||
| args = append(args, fmt.Sprintf("-R %v", GetSlavesIpString(slavesNameIpMap))) |
There was a problem hiding this comment.
Would be great to have dynamic arg as above -R <slaveAddresses>
| } | ||
| out = envManager.ObfuscateSecrets(out) | ||
|
|
||
| defer sl.DeleteSlaves(slavesNameIpMap) |
There was a problem hiding this comment.
I would move it ealier to CreateSlaves
| const ( | ||
| serverPort = 1099 | ||
| localport = 60001 | ||
| ) |
There was a problem hiding this comment.
should be able to redefine in config or parameters
|
|
||
| // Wait for the pod to become ready | ||
| conditionFunc := isPodReady(context.Background(), client.clientSet, p.Name, client.namespace) | ||
| timeout := 5 * time.Minute |
| conditionFunc := isPodReady(context.Background(), client.clientSet, p.Name, client.namespace) | ||
| timeout := 5 * time.Minute | ||
|
|
||
| err = wait.PollImmediate(time.Second, timeout, conditionFunc) |
There was a problem hiding this comment.
also should be const, interval or something
| func (client *Client) DeleteSlaves(slaveNameIpMap map[string]string) { | ||
| for slaveName := range slaveNameIpMap { | ||
| output.PrintLog(fmt.Sprintf("Deleting slave %v", slaveName)) | ||
| client.clientSet.CoreV1().Pods(client.namespace).Delete(context.Background(), slaveName, metav1.DeleteOptions{}) |
There was a problem hiding this comment.
I would record error, if we have it
| Value: envParams.CloudAPIURL, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Ideally, it should be in one place in api or we will need to sync them
| }, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Should be taken from template, we don't hard code spec
| return *result.WithErrors(errors.Errorf("Getting error while creating slaves nodes: %v", err)), nil | ||
| } | ||
|
|
||
| args = append(args, fmt.Sprintf("-R %v", GetSlavesIpString(slavesNameIpMap))) |
|
|
||
| const ( | ||
| serverPort = 1099 | ||
| localport = 60001 |
There was a problem hiding this comment.
again looks like a copy for above file
| Value: envParams.CloudAPIURL, | ||
| }, | ||
| } | ||
| } |
hiteshwani29
force-pushed
the
djmeter-support
branch
from
942a37f to
9ff550b
Compare
|
Unable to modify PR title. Initiating a new PR with an appropriate title following the correct naming conventions. |
Pull request description
Checklist (choose whats happened)
Breaking changes
Changes
Fixes