From 5cc44b300865edc0a9ad94d4ce5a46a68621cf7d Mon Sep 17 00:00:00 2001 From: adohe Date: Mon, 28 Aug 2023 11:29:50 +0800 Subject: [PATCH] feat: update workload generator support probe&lifecycle --- go.mod | 1 + go.sum | 1 + .../generator/workload/workload_generator.go | 220 +++++++++++++++++- .../workload/workload_generator_test.go | 131 +++++++++++ .../appconfiguration/workload/common.go | 26 +-- pkg/util/net/parse.go | 54 +++++ pkg/util/net/parse_test.go | 141 +++++++++++ 7 files changed, 544 insertions(+), 30 deletions(-) create mode 100644 pkg/util/net/parse.go create mode 100644 pkg/util/net/parse_test.go diff --git a/go.mod b/go.mod index 78f3fc16..c15802c7 100644 --- a/go.mod +++ b/go.mod @@ -49,6 +49,7 @@ require ( github.com/variantdev/vals v0.21.0 github.com/zclconf/go-cty v1.12.1 go.uber.org/zap v1.24.0 + golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561 gopkg.in/natefinch/lumberjack.v2 v2.0.0 gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 574df44a..88c65db2 100644 --- a/go.sum +++ b/go.sum @@ -908,6 +908,7 @@ golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561 h1:MDc5xs78ZrZr3HMQugiXOAkSZtfTpbJLDr/lwfgO53E= +golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/pkg/generator/appconfiguration/generator/workload/workload_generator.go b/pkg/generator/appconfiguration/generator/workload/workload_generator.go index 0e6636ad..bc58c127 100644 --- a/pkg/generator/appconfiguration/generator/workload/workload_generator.go +++ b/pkg/generator/appconfiguration/generator/workload/workload_generator.go @@ -2,14 +2,20 @@ package workload import ( "fmt" + "net/url" + "strings" - corev1 "k8s.io/api/core/v1" + "golang.org/x/exp/maps" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/intstr" "kusionstack.io/kusion/pkg/generator/appconfiguration" "kusionstack.io/kusion/pkg/models" "kusionstack.io/kusion/pkg/models/appconfiguration/workload" "kusionstack.io/kusion/pkg/models/appconfiguration/workload/container" "kusionstack.io/kusion/pkg/projectstack" + "kusionstack.io/kusion/pkg/util/net" ) type workloadGenerator struct { @@ -71,29 +77,229 @@ func (g *workloadGenerator) Generate(spec *models.Spec) error { return nil } -func toOrderedContainers(appContainers map[string]container.Container) ([]corev1.Container, error) { +func toOrderedContainers(appContainers map[string]container.Container) ([]v1.Container, error) { // Create a slice of containers based on the app's // containers. - var containers []corev1.Container + var containers []v1.Container if err := appconfiguration.ForeachOrdered(appContainers, func(containerName string, c container.Container) error { // Create a slice of env vars based on the container's env vars. - var envs []corev1.EnvVar + var envs []v1.EnvVar for k, v := range c.Env { envs = append(envs, *MagicEnvVar(k, v)) } + resourceRequirements, err := handleResourceRequirementsV1(c.Resources) + if err != nil { + return err + } - // Create a container object and append it to the containers slice. - containers = append(containers, corev1.Container{ + ctn := v1.Container{ Name: containerName, Image: c.Image, Command: c.Command, Args: c.Args, WorkingDir: c.WorkingDir, Env: envs, - }) + Resources: resourceRequirements, + } + err = updateContainer(&c, &ctn) + if err != nil { + return err + } + // Create a container object and append it to the containers slice. + containers = append(containers, ctn) return nil }); err != nil { return nil, err } return containers, nil } + +// updateContainer updates v1.Container with passed parameters. +func updateContainer(in *container.Container, out *v1.Container) error { + if in.ReadinessProbe != nil { + readinessProbe, err := convertKusionProbeToV1Probe(in.ReadinessProbe) + if err != nil { + return err + } + out.ReadinessProbe = readinessProbe + } + + if in.LivenessProbe != nil { + livenessProbe, err := convertKusionProbeToV1Probe(in.LivenessProbe) + if err != nil { + return err + } + out.LivenessProbe = livenessProbe + } + + if in.StartupProbe != nil { + startupProbe, err := convertKusionProbeToV1Probe(in.StartupProbe) + if err != nil { + return err + } + out.StartupProbe = startupProbe + } + + if in.Lifecycle != nil { + lifecycle, err := convertKusionLifecycleToV1Lifecycle(in.Lifecycle) + if err != nil { + return err + } + out.Lifecycle = lifecycle + } + + return nil +} + +// handleResourceRequirementsV1 parses the resources parameter if specified and +// returns ResourceRequirements. +func handleResourceRequirementsV1(resources map[string]string) (v1.ResourceRequirements, error) { + result := v1.ResourceRequirements{} + if resources == nil { + return result, nil + } + for key, value := range resources { + resourceName := v1.ResourceName(key) + requests, limits, err := populateResourceLists(resourceName, value) + if err != nil { + return result, err + } + if requests != nil && result.Requests == nil { + result.Requests = make(v1.ResourceList) + } + maps.Copy(result.Requests, requests) + if limits != nil && result.Limits == nil { + result.Limits = make(v1.ResourceList) + } + maps.Copy(result.Limits, limits) + } + return result, nil +} + +// populateResourceLists takes strings of form =[-] and +// returns request&limit ResourceList. +func populateResourceLists(name v1.ResourceName, spec string) (v1.ResourceList, v1.ResourceList, error) { + requests := v1.ResourceList{} + limits := v1.ResourceList{} + + parts := strings.Split(spec, "-") + if len(parts) == 1 { + resourceQuantity, err := resource.ParseQuantity(parts[0]) + if err != nil { + return nil, nil, err + } + limits[name] = resourceQuantity + } else if len(parts) == 2 { + resourceQuantity, err := resource.ParseQuantity(parts[0]) + if err != nil { + return nil, nil, err + } + requests[name] = resourceQuantity + resourceQuantity, err = resource.ParseQuantity(parts[1]) + if err != nil { + return nil, nil, err + } + limits[name] = resourceQuantity + } + + return requests, limits, nil +} + +// convertKusionProbeToV1Probe converts Kusion Probe to Kubernetes Probe types. +func convertKusionProbeToV1Probe(p *container.Probe) (*v1.Probe, error) { + result := &v1.Probe{ + InitialDelaySeconds: p.InitialDelaySeconds, + TimeoutSeconds: p.TimeoutSeconds, + PeriodSeconds: p.PeriodSeconds, + SuccessThreshold: p.SuccessThreshold, + FailureThreshold: p.FailureThreshold, + } + probeHandler := p.ProbeHandler + switch probeHandler.Type { + case "Http": + action, err := httpGetAction(probeHandler.HTTPGetAction.URL, probeHandler.Headers) + if err != nil { + return nil, err + } + result.HTTPGet = action + case "Exec": + result.Exec = &v1.ExecAction{Command: probeHandler.Command} + case "Tcp": + action, err := tcpSocketAction(probeHandler.TCPSocketAction.URL) + if err != nil { + return nil, err + } + result.TCPSocket = action + } + return result, nil +} + +// convertKusionLifecycleToV1Lifecycle converts Kusion Lifecycle to Kubernetes Lifecycle types. +func convertKusionLifecycleToV1Lifecycle(l *container.Lifecycle) (*v1.Lifecycle, error) { + result := &v1.Lifecycle{} + if l.PreStop != nil { + preStop, err := lifecycleHandler(l.PreStop) + if err != nil { + return nil, err + } + result.PreStop = preStop + } + if l.PostStart != nil { + postStart, err := lifecycleHandler(l.PostStart) + if err != nil { + return nil, err + } + result.PostStart = postStart + } + return result, nil +} + +func lifecycleHandler(in *container.LifecycleHandler) (*v1.LifecycleHandler, error) { + result := &v1.LifecycleHandler{} + switch in.Type { + case "Http": + action, err := httpGetAction(in.HTTPGetAction.URL, in.Headers) + if err != nil { + return nil, err + } + result.HTTPGet = action + case "Exec": + result.Exec = &v1.ExecAction{Command: in.Command} + } + return result, nil +} + +func httpGetAction(urlstr string, headers map[string]string) (*v1.HTTPGetAction, error) { + u, err := url.Parse(urlstr) + if err != nil { + return nil, err + } + + httpHeaders := make([]v1.HTTPHeader, 0, len(headers)) + for k, v := range headers { + httpHeaders = append(httpHeaders, v1.HTTPHeader{ + Name: k, + Value: v, + }) + } + + return &v1.HTTPGetAction{ + Path: u.Path, + Port: intstr.FromString(u.Port()), + Host: u.Hostname(), + Scheme: v1.URIScheme(strings.ToUpper(u.Scheme)), + HTTPHeaders: httpHeaders, + }, nil +} + +func tcpSocketAction(urlstr string) (*v1.TCPSocketAction, error) { + host, port, err := net.ParseHostPort(urlstr) + if err != nil { + return nil, err + } + + return &v1.TCPSocketAction{ + Port: intstr.FromString(port), + Host: host, + }, nil +} diff --git a/pkg/generator/appconfiguration/generator/workload/workload_generator_test.go b/pkg/generator/appconfiguration/generator/workload/workload_generator_test.go index 73789d9f..2566a205 100644 --- a/pkg/generator/appconfiguration/generator/workload/workload_generator_test.go +++ b/pkg/generator/appconfiguration/generator/workload/workload_generator_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "kusionstack.io/kusion/pkg/generator/appconfiguration" "kusionstack.io/kusion/pkg/models" "kusionstack.io/kusion/pkg/models/appconfiguration/workload" @@ -147,4 +148,134 @@ func TestToOrderedContainers(t *testing.T) { assert.Equal(t, "key", actualContainers[1].Env[0].Name, "Env var name mismatch") assert.Equal(t, "value", actualContainers[1].Env[0].Value, "Env var value mismatch") }) + t.Run("toOrderedContainers should convert app containers with probe to ordered containers", func(t *testing.T) { + appContainers := map[string]container.Container{ + "nginx": { + Image: "nginx:v1", + Resources: map[string]string{ + "cpu": "2-4", + "memory": "4Gi-8Gi", + }, + LivenessProbe: &container.Probe{ + ProbeHandler: &container.ProbeHandler{ + TypeWrapper: container.TypeWrapper{ + Type: "Exec", + }, + ExecAction: &container.ExecAction{ + Command: []string{"/bin/sh", "-c", "echo live"}, + }, + }, + }, + ReadinessProbe: &container.Probe{ + ProbeHandler: &container.ProbeHandler{ + TypeWrapper: container.TypeWrapper{ + Type: "Http", + }, + HTTPGetAction: &container.HTTPGetAction{ + URL: "http://localhost:8080/readiness", + Headers: map[string]string{ + "header": "value", + }, + }, + }, + InitialDelaySeconds: 10, + }, + StartupProbe: &container.Probe{ + ProbeHandler: &container.ProbeHandler{ + TypeWrapper: container.TypeWrapper{ + Type: "Tcp", + }, + TCPSocketAction: &container.TCPSocketAction{ + URL: "10.0.0.1:8888", + }, + }, + }, + }, + } + + actualContainers, err := toOrderedContainers(appContainers) + + assert.NoError(t, err, "Error should be nil") + assert.Len(t, actualContainers, 1, "Number of containers mismatch") + assert.Equal(t, "nginx", actualContainers[0].Name, "Container name mismatch") + assert.Equal(t, "nginx:v1", actualContainers[0].Image, "Container image mismatch") + assert.Len(t, actualContainers[0].Resources.Requests, 2, "Number of resource requests mismatch") + + // Assert request resources + rQuantity := actualContainers[0].Resources.Requests["cpu"] + assert.Equal(t, "2", (&rQuantity).String(), "CPU request mismatch") + rQuantity = actualContainers[0].Resources.Requests["memory"] + assert.Equal(t, "4Gi", (&rQuantity).String(), "CPU request mismatch") + + // Assert limit resources + rQuantity = actualContainers[0].Resources.Limits["cpu"] + assert.Equal(t, "4", (&rQuantity).String(), "CPU request mismatch") + rQuantity = actualContainers[0].Resources.Limits["memory"] + assert.Equal(t, "8Gi", (&rQuantity).String(), "CPU request mismatch") + + assert.NotNil(t, actualContainers[0].ReadinessProbe, "ReadinessProbe should not be nil") + assert.NotNil(t, actualContainers[0].ReadinessProbe.HTTPGet, "ReadinessProbe.HTTPGet action should not be nil") + assert.Equal(t, "HTTP", string(actualContainers[0].ReadinessProbe.HTTPGet.Scheme), "HTTPGet.Scheme mismatch") + assert.Equal(t, "/readiness", actualContainers[0].ReadinessProbe.HTTPGet.Path, "HTTPGet.Path mismatch") + assert.Equal(t, "8080", actualContainers[0].ReadinessProbe.HTTPGet.Port.String(), "HTTPGet.Port mismatch") + assert.Equal(t, "localhost", actualContainers[0].ReadinessProbe.HTTPGet.Host, "HTTPGet.Host mismatch") + assert.Equal(t, 1, len(actualContainers[0].ReadinessProbe.HTTPGet.HTTPHeaders), "HTTPGet.HTTPHeaders length mismatch") + + assert.NotNil(t, actualContainers[0].LivenessProbe, "LivenessProbe should not be nil") + assert.NotNil(t, actualContainers[0].LivenessProbe.Exec, "LivenessProbe.Exec action should not be nil") + assert.Len(t, actualContainers[0].LivenessProbe.Exec.Command, 3, "LivenessProbe.Exec commands length mismatch") + assert.Equal(t, []string{"/bin/sh", "-c", "echo live"}, actualContainers[0].LivenessProbe.Exec.Command, "LivenessProbe.Exec commands mismatch") + + assert.NotNil(t, actualContainers[0].StartupProbe, "StartupProbe should not be nil") + assert.NotNil(t, actualContainers[0].StartupProbe.TCPSocket, "StartupProbe.TCPSocket action should not be nil") + assert.Equal(t, "10.0.0.1", actualContainers[0].StartupProbe.TCPSocket.Host, "TCPSocket.Host mismatch") + assert.Equal(t, "8888", actualContainers[0].StartupProbe.TCPSocket.Port.String(), "TCPSocket.Port mismatch") + }) + t.Run("toOrderedContainers should convert app containers with lifecycle to ordered containers", func(t *testing.T) { + appContainers := map[string]container.Container{ + "nginx": { + Image: "nginx:v1", + Lifecycle: &container.Lifecycle{ + PreStop: &container.LifecycleHandler{ + TypeWrapper: container.TypeWrapper{ + Type: "Exec", + }, + ExecAction: &container.ExecAction{ + Command: []string{"/bin/sh", "-c", "echo live"}, + }, + }, + PostStart: &container.LifecycleHandler{ + TypeWrapper: container.TypeWrapper{ + Type: "Http", + }, + HTTPGetAction: &container.HTTPGetAction{ + URL: "http://localhost:8080/readiness", + Headers: map[string]string{ + "header": "value", + }, + }, + }, + }, + }, + } + + actualContainers, err := toOrderedContainers(appContainers) + + assert.NoError(t, err, "Error should be nil") + assert.Len(t, actualContainers, 1, "Number of containers mismatch") + assert.Equal(t, "nginx", actualContainers[0].Name, "Container name mismatch") + assert.Equal(t, "nginx:v1", actualContainers[0].Image, "Container image mismatch") + + assert.NotNil(t, actualContainers[0].Lifecycle, "Lifecycle should not be nil") + assert.NotNil(t, actualContainers[0].Lifecycle.PreStop, "Lifecycle.PreStop should not be nil") + assert.NotNil(t, actualContainers[0].Lifecycle.PreStop.Exec, "PreStop.Exec action should not be nil") + assert.Len(t, actualContainers[0].Lifecycle.PreStop.Exec.Command, 3, "PreStop.Exec commands length mismatch") + assert.Equal(t, []string{"/bin/sh", "-c", "echo live"}, actualContainers[0].Lifecycle.PreStop.Exec.Command, "PreStop.Exec commands mismatch") + assert.NotNil(t, actualContainers[0].Lifecycle.PostStart, "Lifecycle.PostStart should not be nil") + assert.Equal(t, "HTTP", string(actualContainers[0].Lifecycle.PostStart.HTTPGet.Scheme), "PostStart.HTTPGet.Scheme mismatch") + assert.Equal(t, "/readiness", actualContainers[0].Lifecycle.PostStart.HTTPGet.Path, "PostStart.HTTPGet.Path mismatch") + assert.Equal(t, "8080", actualContainers[0].Lifecycle.PostStart.HTTPGet.Port.String(), "PostStart.HTTPGet.Port mismatch") + assert.Equal(t, "localhost", actualContainers[0].Lifecycle.PostStart.HTTPGet.Host, "PostStart.HTTPGet.Host mismatch") + assert.Equal(t, 1, len(actualContainers[0].Lifecycle.PostStart.HTTPGet.HTTPHeaders), "PostStart.HTTPGet.HTTPHeaders length mismatch") + }) } diff --git a/pkg/models/appconfiguration/workload/common.go b/pkg/models/appconfiguration/workload/common.go index b223b6e5..304413d2 100644 --- a/pkg/models/appconfiguration/workload/common.go +++ b/pkg/models/appconfiguration/workload/common.go @@ -3,13 +3,13 @@ package workload import "kusionstack.io/kusion/pkg/models/appconfiguration/workload/container" // Base defines set of attributes shared by different workload -// profile, e.g Service and Job. You can inherit this Schema to reuse +// profile, e.g. Service and Job. You can inherit this Schema to reuse // these common attributes. type Base struct { - // The templates of containers to be ran. + // The templates of containers to be run. Containers map[string]container.Container `yaml:"containers,omitempty" json:"containers,omitempty"` - // The number of containers that should be ran. + // The number of containers that should be run. // Default is 2 to meet high availability requirements. Replicas int `yaml:"replicas,omitempty" json:"replicas,omitempty"` @@ -21,24 +21,4 @@ type Base struct { // Dirs configures one or more volumes to be mounted to the // specified folder. Dirs map[string]string `json:"dirs,omitempty" yaml:"dirs,omitempty"` - // Files configures one or more files to be created in the container. - // files {str:FileSpec} - - // Liveness probe for this container. - // Liveness probe indicates if a running process is healthy. - // livenessProbe p.Probe`json:"livenessProbe,omitempty" yaml:"livenessProbe,omitempty"` - - // Readiness probe for this container. - // Readiness probe indicates whether an application is available - // to handle requests. - // readinessProbe p.Probe`json:"readinessProbe,omitempty" yaml:"readinessProbe,omitempty"` - - // Startup probe for this container. - // Startup probe indicates that the container has started for the - // first time. - // startupProbe p.Probe`json:"startupProbe,omitempty" yaml:"startupProbe,omitempty"` - - // Lifecycle configures actions which should be taken response to - // container lifecycle events. - // lifecycle lc.Lifecycle`json:"lifecycle,omitempty" yaml:"lifecycle,omitempty"` } diff --git a/pkg/util/net/parse.go b/pkg/util/net/parse.go new file mode 100644 index 00000000..39fe8bf4 --- /dev/null +++ b/pkg/util/net/parse.go @@ -0,0 +1,54 @@ +package net + +import ( + "errors" + "fmt" + "net" + + "k8s.io/apimachinery/pkg/util/validation" + netutils "k8s.io/utils/net" +) + +// ParseHostPort parses a network address of the form "host:port", "ipv4:port", "[ipv6]:port" into host and port; +// ":port" can be eventually omitted. +// If the string is not a valid representation of network address, ParseHostPort returns an error. +func ParseHostPort(hostport string) (string, string, error) { + var host, port string + var err error + + // try to split host and port + if host, port, err = net.SplitHostPort(hostport); err != nil { + // if SplitHostPort returns an error, the entire hostport is considered as host + host = hostport + } + + // if port is defined, parse and validate it + if port != "" { + if _, err := ParsePort(port); err != nil { + return "", "", fmt.Errorf("hostport %s: port %s must be a valid number between 1 and 65535, inclusive", hostport, port) + } + } + + // if host is a valid IP, returns it + if ip := netutils.ParseIPSloppy(host); ip != nil { + return host, port, nil + } + + // if host is a validated RFC-1123 subdomain, returns it + if errs := validation.IsDNS1123Subdomain(host); len(errs) == 0 { + return host, port, nil + } + + return "", "", fmt.Errorf("hostport %s: host '%s' must be a valid IP address or a valid RFC-1123 DNS subdomain", hostport, host) +} + +// ParsePort parses a string representing a TCP port. +// If the string is not a valid representation of a TCP port, ParsePort returns an error. +func ParsePort(port string) (int, error) { + portInt, err := netutils.ParsePort(port, true) + if err == nil && (1 <= portInt && portInt <= 65535) { + return portInt, nil + } + + return 0, errors.New("port must be a valid number between 1 and 65535, inclusive") +} diff --git a/pkg/util/net/parse_test.go b/pkg/util/net/parse_test.go new file mode 100644 index 00000000..2d20a22b --- /dev/null +++ b/pkg/util/net/parse_test.go @@ -0,0 +1,141 @@ +package net + +import ( + "testing" +) + +func TestParseHostPort(t *testing.T) { + tests := []struct { + name string + hostport string + expectedHost string + expectedPort string + expectedError bool + }{ + { + name: "valid host", + hostport: "kusionstack.io", + expectedHost: "kusionstack.io", + expectedPort: "", + }, + { + name: "valid host:port", + hostport: "kusionstack.io:1234", + expectedHost: "kusionstack.io", + expectedPort: "1234", + }, + { + name: "valid ip", + hostport: "1.2.3.4", + expectedHost: "1.2.3.4", + expectedPort: "", + }, + { + name: "valid ip:port", + hostport: "1.2.3.4:1234", + expectedHost: "1.2.3.4", + expectedPort: "1234", + }, + { + name: "invalid port(not a number)", + hostport: "kusionstack.io:aaa", + expectedError: true, + }, + { + name: "invalid port(>65535)", + hostport: "kusionstack.io:65536", + expectedError: true, + }, + { + name: "invalid port(<1)", + hostport: "kusionstack.io:-1", + expectedError: true, + }, + { + name: "invalid host", + hostport: "~~kusionstack.io", + expectedError: true, + }, + { + name: "invalid host:port", + hostport: "~~kusionstack.io:1234", + expectedError: true, + }, + { + name: "invalid ip", + hostport: "1..3.4", + expectedError: true, + }, + } + + for _, rt := range tests { + t.Run(rt.name, func(t *testing.T) { + actualHost, actualPort, actualError := ParseHostPort(rt.hostport) + + if (actualError != nil) && !rt.expectedError { + t.Errorf("%s unexpected failure: %v", rt.name, actualError) + return + } else if (actualError == nil) && rt.expectedError { + t.Errorf("%s passed when expected to fail", rt.name) + return + } + + if actualHost != rt.expectedHost { + t.Errorf("%s returned invalid host %s, expected %s", rt.name, actualHost, rt.expectedHost) + return + } + + if actualPort != rt.expectedPort { + t.Errorf("%s returned invalid port %s, expected %s", rt.name, actualPort, rt.expectedPort) + } + }) + } +} + +func TestParsePort(t *testing.T) { + tests := []struct { + name string + port string + expectedPort int + expectedError bool + }{ + { + name: "valid port", + port: "1234", + expectedPort: 1234, + }, + { + name: "invalid port (not a number)", + port: "a", + expectedError: true, + }, + { + name: "invalid port (<1)", + port: "-10", + expectedError: true, + }, + { + name: "invalid port (>65535)", + port: "65536", + expectedError: true, + }, + } + + for _, rt := range tests { + t.Run(rt.name, func(t *testing.T) { + actualPort, actualError := ParsePort(rt.port) + + if (actualError != nil) && !rt.expectedError { + t.Errorf("%s unexpected failure: %v", rt.name, actualError) + return + } else if (actualError == nil) && rt.expectedError { + t.Errorf("%s passed when expected to fail", rt.name) + return + } + + if actualPort != rt.expectedPort { + t.Errorf("%s returned invalid port %d, expected %d", rt.name, actualPort, rt.expectedPort) + } + }) + } +}