Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
wanlin31 committed Feb 4, 2022
1 parent fbe0297 commit e674fb6
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 45 deletions.
2 changes: 1 addition & 1 deletion api/v1/loadtest_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ type Server struct {
// +optional
Build *Build `json:"build,omitempty"`

// Run describes a list of run container, , which is the runtime of the server for
// Run describes a list of run containers, which is the runtime of the server for
// the actual test.
Run []corev1.Container `json:"run"`
}
Expand Down
7 changes: 4 additions & 3 deletions config/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,10 @@ const (
// example, "loadtest-role=server" indicates a server component.
RoleLabel = "loadtest-role"

// RunContainerListName holds the name of the list of containers that run
// simultaneously after the init containers.
RunContainerListName = "run"
// RunContainerName holds the name of the main container where the test is
// executed. The runtime for the test may contain multiple run containers.
// The main container is always the first container on the list.
RunContainerName = "run"

// ScenariosFileEnv specifies the name of an env variable that specifies the
// path to a JSON file with scenarios.
Expand Down
4 changes: 2 additions & 2 deletions config/crd/bases/e2etest.grpc.io_loadtests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2910,8 +2910,8 @@ spec:
will choose a pool based on defaults.
type: string
run:
description: Run describes a list of run container, , which
is the runtime of the server for the actual test.
description: Run describes a list of run containers, which is
the runtime of the server for the actual test.
items:
description: A single application container that you want
to run within a pod.
Expand Down
37 changes: 13 additions & 24 deletions config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,20 @@ func (d *Defaults) setBuildOrDefault(im *imageMap, language string, build *grpcv

// setRunOrDefault sets the default runtime image if it is unset. It returns an
// error if there is no default runtime image for the provided language.
func (d *Defaults) setRunOrDefault(im *imageMap, language string, run *corev1.Container) error {
if run != nil && run.Image == "" {
func (d *Defaults) setRunOrDefault(im *imageMap, language string, run []corev1.Container) error {

if len(run) == 0 {
run = []corev1.Container{{Name: RunContainerName}}
}

if run[0].Image == "" {
runImage, err := im.runImage(language)
if err != nil {
return errors.Wrap(err, "could not infer default run image")
}

run.Image = runImage

run.Env = append(run.Env, corev1.EnvVar{
run[0].Image = runImage
run[0].Env = append(run[0].Env, corev1.EnvVar{
Name: KillAfterEnv,
Value: fmt.Sprintf("%f", d.KillAfter),
})
Expand All @@ -182,12 +186,9 @@ func (d *Defaults) setDriverDefaults(im *imageMap, testSpec *grpcv1.LoadTestSpec
}

if len(driver.Run) == 0 {
driver.Run = []corev1.Container{{Name: WorkerContainerName}}
}

if driver.Run[0].Image == "" {
driver.Run[0].Image = d.DriverImage
driver.Run = []corev1.Container{{Name: RunContainerName}}
}
driver.Run[0].Image = d.DriverImage

driver.Name = unwrapStrOrUUID(driver.Name)
d.setCloneOrDefault(driver.Clone)
Expand All @@ -196,10 +197,6 @@ func (d *Defaults) setDriverDefaults(im *imageMap, testSpec *grpcv1.LoadTestSpec
return errors.Wrap(err, "failed to set defaults on instructions to build the driver")
}

if err := d.setRunOrDefault(im, driver.Language, &driver.Run[0]); err != nil {
return errors.Wrap(err, "failed to set defaults on instructions to run the driver")
}

return nil
}

Expand All @@ -217,11 +214,7 @@ func (d *Defaults) setClientDefaults(im *imageMap, client *grpcv1.Client) error
return errors.Wrap(err, "failed to set defaults on instructions to build the client")
}

if len(client.Run) == 0 {
client.Run = []corev1.Container{{Name: WorkerContainerName}}
}

if err := d.setRunOrDefault(im, client.Language, &client.Run[0]); err != nil {
if err := d.setRunOrDefault(im, client.Language, client.Run); err != nil {
return errors.Wrap(err, "failed to set defaults on instructions to run the client")
}

Expand All @@ -242,11 +235,7 @@ func (d *Defaults) setServerDefaults(im *imageMap, server *grpcv1.Server) error
return errors.Wrap(err, "failed to set defaults on instructions to build the server")
}

if len(server.Run) == 0 {
server.Run = []corev1.Container{{Name: WorkerContainerName}}
}

if err := d.setRunOrDefault(im, server.Language, &server.Run[0]); err != nil {
if err := d.setRunOrDefault(im, server.Language, server.Run); err != nil {
return errors.Wrap(err, "failed to set defaults on instructions to run the server")
}

Expand Down
4 changes: 2 additions & 2 deletions config/samples/go_example_loadtest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
language: go
name: '0'
run:
- name: "worker"
- name: "run"
args:
- -c
- |
Expand Down Expand Up @@ -118,7 +118,7 @@ spec:
language: go
name: '0'
run:
- name: "worker"
- name: "run"
args:
- -c
- |
Expand Down
4 changes: 2 additions & 2 deletions containers/init/ready/ready_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Describe("WaitForReadyPods", func() {
slowDuration = 100 * time.Millisecond * timeMultiplier

driverPod = newTestPod("driver")
driverRunContainer := kubehelpers.ContainerForName(config.RunContainerListName, driverPod.Spec.Containers)
driverRunContainer := kubehelpers.ContainerForName(config.RunContainerName, driverPod.Spec.Containers)
driverRunContainer.Ports = nil

serverPod = newTestPod("server")
Expand Down Expand Up @@ -203,7 +203,7 @@ var _ = Describe("WaitForReadyPods", func() {
var customPort int32 = 9542
client2Pod := newTestPod("client")
client2Pod.Name = "client-2"
client2PodContainer := kubehelpers.ContainerForName(config.RunContainerListName, client2Pod.Spec.Containers)
client2PodContainer := kubehelpers.ContainerForName(config.RunContainerName, client2Pod.Spec.Containers)
client2PodContainer.Ports[0].ContainerPort = customPort

podListerMock := &PodListerMock{
Expand Down
10 changes: 4 additions & 6 deletions podbuilder/podbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,6 @@ func (pb *PodBuilder) PodForClient(client *grpcv1.Client) (*corev1.Pod, error) {
pb.pool = safeStrUnwrap(client.Pool)
pb.clone = client.Clone
pb.build = client.Build
// Pods are build after setting the default, so at least one container
// in the client.Run has been set.
pb.run[0] = client.Run[0]

pod := pb.newPod()
Expand All @@ -150,7 +148,7 @@ func (pb *PodBuilder) PodForClient(client *grpcv1.Client) (*corev1.Pod, error) {
}
pod.Spec.NodeSelector = nodeSelector

runContainer := kubehelpers.ContainerForName(config.RunContainerListName, pod.Spec.Containers)
runContainer := kubehelpers.ContainerForName(config.RunContainerName, pod.Spec.Containers)

pod.Spec.Containers[0].Env = append(pod.Spec.Containers[0].Env, corev1.EnvVar{
Name: config.DriverPortEnv,
Expand Down Expand Up @@ -188,7 +186,7 @@ func (pb *PodBuilder) PodForDriver(driver *grpcv1.Driver) (*corev1.Pod, error) {
}
pod.Spec.NodeSelector = nodeSelector

runContainer := kubehelpers.ContainerForName(config.RunContainerListName, pod.Spec.Containers)
runContainer := kubehelpers.ContainerForName(config.RunContainerName, pod.Spec.Containers)
addReadyInitContainer(pb.defaults, pb.test, &pod.Spec, runContainer)

pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{
Expand Down Expand Up @@ -254,7 +252,7 @@ func (pb *PodBuilder) PodForServer(server *grpcv1.Server) (*corev1.Pod, error) {
}
pod.Spec.NodeSelector = nodeSelector

runContainer := kubehelpers.ContainerForName(config.RunContainerListName, pod.Spec.Containers)
runContainer := kubehelpers.ContainerForName(config.RunContainerName, pod.Spec.Containers)

pod.Spec.Containers[0].Env = append(pod.Spec.Containers[0].Env, corev1.EnvVar{
Name: config.DriverPortEnv,
Expand Down Expand Up @@ -341,7 +339,7 @@ func (pb *PodBuilder) newPod() *corev1.Pod {
InitContainers: initContainers,
Containers: []corev1.Container{
{
Name: config.RunContainerListName,
Name: config.RunContainerName,
Image: safeStrUnwrap(&pb.run[0].Image),
Command: pb.run[0].Command,
Args: pb.run[0].Args,
Expand Down
10 changes: 5 additions & 5 deletions podbuilder/podbuilder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ var _ = Describe("PodBuilder", func() {
Expect(err).ToNot(HaveOccurred())
Expect(pod.Spec.Containers).ToNot(BeEmpty())

runContainer := kubehelpers.ContainerForName(config.RunContainerListName, pod.Spec.Containers)
runContainer := kubehelpers.ContainerForName(config.RunContainerName, pod.Spec.Containers)
Expect(runContainer.VolumeMounts).ToNot(BeEmpty())
Expect(runContainer.VolumeMounts).To(ContainElement(corev1.VolumeMount{
Name: config.WorkspaceVolumeName,
Expand All @@ -312,7 +312,7 @@ var _ = Describe("PodBuilder", func() {
Expect(err).ToNot(HaveOccurred())
Expect(pod.Spec.Containers).ToNot(BeEmpty())

runContainer := kubehelpers.ContainerForName(config.RunContainerListName, pod.Spec.Containers)
runContainer := kubehelpers.ContainerForName(config.RunContainerName, pod.Spec.Containers)
Expect(getNames(runContainer.Ports)).To(ContainElement("driver"))
Expect(getValue("driver", "ContainerPort", runContainer.Ports)).To(BeEquivalentTo(config.DriverPort))
})
Expand Down Expand Up @@ -541,7 +541,7 @@ var _ = Describe("PodBuilder", func() {
Expect(err).ToNot(HaveOccurred())
Expect(pod.Spec.Containers).ToNot(BeEmpty())

runContainer := kubehelpers.ContainerForName(config.RunContainerListName, pod.Spec.Containers)
runContainer := kubehelpers.ContainerForName(config.RunContainerName, pod.Spec.Containers)
Expect(runContainer.VolumeMounts).ToNot(BeEmpty())
Expect(runContainer.VolumeMounts).To(ContainElement(corev1.VolumeMount{
Name: config.WorkspaceVolumeName,
Expand All @@ -558,7 +558,7 @@ var _ = Describe("PodBuilder", func() {
Expect(err).ToNot(HaveOccurred())
Expect(pod.Spec.Containers).ToNot(BeEmpty())

runContainer := kubehelpers.ContainerForName(config.RunContainerListName, pod.Spec.Containers)
runContainer := kubehelpers.ContainerForName(config.RunContainerName, pod.Spec.Containers)
Expect(getNames(runContainer.Ports)).To(ContainElement("driver"))
Expect(getValue("driver", "ContainerPort", runContainer.Ports)).To(BeEquivalentTo(config.DriverPort))
})
Expand Down Expand Up @@ -787,7 +787,7 @@ var _ = Describe("PodBuilder", func() {
Expect(err).ToNot(HaveOccurred())
Expect(pod.Spec.Containers).ToNot(BeEmpty())

runContainer := kubehelpers.ContainerForName(config.RunContainerListName, pod.Spec.Containers)
runContainer := kubehelpers.ContainerForName(config.RunContainerName, pod.Spec.Containers)
Expect(runContainer.VolumeMounts).ToNot(BeEmpty())
Expect(runContainer.VolumeMounts).To(ContainElement(corev1.VolumeMount{
Name: config.WorkspaceVolumeName,
Expand Down

0 comments on commit e674fb6

Please sign in to comment.