From 5f1cf11b86f4202664b269d4d61bf0f86e3249ce Mon Sep 17 00:00:00 2001 From: Parthvi Vala Date: Wed, 5 Apr 2023 21:06:25 +0530 Subject: [PATCH 1/8] Temp Change --- pkg/dev/podmandev/pod.go | 12 +++++-- pkg/dev/podmandev/podmandev.go | 58 ++++++++++++++++++---------------- pkg/dev/podmandev/reconcile.go | 1 + 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/pkg/dev/podmandev/pod.go b/pkg/dev/podmandev/pod.go index 787715d14b7..63b7eabd648 100644 --- a/pkg/dev/podmandev/pod.go +++ b/pkg/dev/podmandev/pod.go @@ -39,6 +39,7 @@ func createPodFromComponent( debugCommand string, withHelperContainer bool, randomPorts bool, + customForwardedPorts []api.ForwardedPort, usedPorts []int, ) (*corev1.Pod, []api.ForwardedPort, error) { podTemplate, err := generator.GetPodTemplateSpec(devfileObj, generator.PodTemplateParams{}) @@ -50,9 +51,14 @@ func createPodFromComponent( return nil, nil, fmt.Errorf("no valid components found in the devfile") } - fwPorts, err := getPortMapping(devfileObj, debug, randomPorts, usedPorts) - if err != nil { - return nil, nil, err + var fwPorts []api.ForwardedPort + if len(customForwardedPorts) != 0 { + + } else { + fwPorts, err = getPortMapping(devfileObj, debug, randomPorts, usedPorts) + if err != nil { + return nil, nil, err + } } utils.AddOdoProjectVolume(&containers) diff --git a/pkg/dev/podmandev/podmandev.go b/pkg/dev/podmandev/podmandev.go index d8c13856138..27469b220d2 100644 --- a/pkg/dev/podmandev/podmandev.go +++ b/pkg/dev/podmandev/podmandev.go @@ -101,25 +101,26 @@ func (o *DevClient) Start( watch.PrintInfoMessage(out, path, options.WatchFiles, promptMessage) watchParameters := watch.WatchParameters{ - DevfilePath: devfilePath, - Path: path, - ComponentName: componentName, - ApplicationName: appName, - InitialDevfileObj: *devfileObj, - DevfileWatchHandler: o.watchHandler, - FileIgnores: options.IgnorePaths, - Debug: options.Debug, - DevfileBuildCmd: options.BuildCommand, - DevfileRunCmd: options.RunCommand, - Variables: options.Variables, - RandomPorts: options.RandomPorts, - IgnoreLocalhost: options.IgnoreLocalhost, - ForwardLocalhost: options.ForwardLocalhost, - WatchFiles: options.WatchFiles, - WatchCluster: false, - Out: out, - ErrOut: errOut, - PromptMessage: promptMessage, + DevfilePath: devfilePath, + Path: path, + ComponentName: componentName, + ApplicationName: appName, + InitialDevfileObj: *devfileObj, + DevfileWatchHandler: o.watchHandler, + FileIgnores: options.IgnorePaths, + Debug: options.Debug, + DevfileBuildCmd: options.BuildCommand, + DevfileRunCmd: options.RunCommand, + Variables: options.Variables, + RandomPorts: options.RandomPorts, + IgnoreLocalhost: options.IgnoreLocalhost, + ForwardLocalhost: options.ForwardLocalhost, + CustomForwardedPorts: options.CustomForwardedPorts, + WatchFiles: options.WatchFiles, + WatchCluster: false, + Out: out, + ErrOut: errOut, + PromptMessage: promptMessage, } return o.watchClient.WatchAndPush(out, watchParameters, ctx, componentStatus) @@ -196,15 +197,16 @@ func (o *DevClient) watchHandler(ctx context.Context, pushParams adapters.PushPa printWarningsOnDevfileChanges(ctx, watchParams) startOptions := dev.StartOptions{ - IgnorePaths: watchParams.FileIgnores, - Debug: watchParams.Debug, - BuildCommand: watchParams.DevfileBuildCmd, - RunCommand: watchParams.DevfileRunCmd, - RandomPorts: watchParams.RandomPorts, - IgnoreLocalhost: watchParams.IgnoreLocalhost, - ForwardLocalhost: watchParams.ForwardLocalhost, - WatchFiles: watchParams.WatchFiles, - Variables: watchParams.Variables, + IgnorePaths: watchParams.FileIgnores, + Debug: watchParams.Debug, + BuildCommand: watchParams.DevfileBuildCmd, + RunCommand: watchParams.DevfileRunCmd, + RandomPorts: watchParams.RandomPorts, + IgnoreLocalhost: watchParams.IgnoreLocalhost, + ForwardLocalhost: watchParams.ForwardLocalhost, + CustomForwardedPorts: watchParams.CustomForwardedPorts, + WatchFiles: watchParams.WatchFiles, + Variables: watchParams.Variables, } return o.reconcile(ctx, watchParams.Out, watchParams.ErrOut, startOptions, componentStatus) } diff --git a/pkg/dev/podmandev/reconcile.go b/pkg/dev/podmandev/reconcile.go index 44a81f95cf7..73844f5fdd3 100644 --- a/pkg/dev/podmandev/reconcile.go +++ b/pkg/dev/podmandev/reconcile.go @@ -204,6 +204,7 @@ func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions) (*c options.DebugCommand, options.ForwardLocalhost, options.RandomPorts, + options.CustomForwardedPorts, o.usedPorts, ) if err != nil { From 5cad10131cb57f84c9eaa3908f6a654f166d550d Mon Sep 17 00:00:00 2001 From: Parthvi Vala Date: Mon, 10 Apr 2023 14:39:29 +0530 Subject: [PATCH 2/8] Custom port mapping works with podman Signed-off-by: Parthvi Vala --- pkg/dev/podmandev/pod.go | 106 +++++++++++++++++++++++++++++---------- 1 file changed, 79 insertions(+), 27 deletions(-) diff --git a/pkg/dev/podmandev/pod.go b/pkg/dev/podmandev/pod.go index 63b7eabd648..a6d1e1272d8 100644 --- a/pkg/dev/podmandev/pod.go +++ b/pkg/dev/podmandev/pod.go @@ -2,7 +2,7 @@ package podmandev import ( "fmt" - "math/rand" //#nosec + "math/rand" // #nosec "time" "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" @@ -51,14 +51,19 @@ func createPodFromComponent( return nil, nil, fmt.Errorf("no valid components found in the devfile") } + containerComponents, err := devfileObj.Data.GetComponents(common.DevfileOptions{ + ComponentOptions: common.ComponentOptions{ComponentType: v1alpha2.ContainerComponentType}, + }) + if err != nil { + return nil, nil, err + } + ceMapping := libdevfile.GetContainerEndpointMapping(containerComponents, debug) + var fwPorts []api.ForwardedPort if len(customForwardedPorts) != 0 { - + fwPorts = getCustomPortPairs(customForwardedPorts, ceMapping, usedPorts) } else { - fwPorts, err = getPortMapping(devfileObj, debug, randomPorts, usedPorts) - if err != nil { - return nil, nil, err - } + fwPorts = getPortMapping(ceMapping, debug, randomPorts, usedPorts) } utils.AddOdoProjectVolume(&containers) @@ -180,15 +185,7 @@ func getVolumeName(volume string, componentName string, appName string) string { return volume + "-" + componentName + "-" + appName } -func getPortMapping(devfileObj parser.DevfileObj, debug bool, randomPorts bool, usedPorts []int) ([]api.ForwardedPort, error) { - containerComponents, err := devfileObj.Data.GetComponents(common.DevfileOptions{ - ComponentOptions: common.ComponentOptions{ComponentType: v1alpha2.ContainerComponentType}, - }) - if err != nil { - return nil, err - } - ceMapping := libdevfile.GetContainerEndpointMapping(containerComponents, debug) - +func getPortMapping(ceMapping map[string][]v1alpha2.Endpoint, debug bool, randomPorts bool, usedPorts []int) []api.ForwardedPort { var existingContainerPorts []int for _, endpoints := range ceMapping { for _, ep := range endpoints { @@ -236,17 +233,11 @@ func getPortMapping(devfileObj parser.DevfileObj, debug bool, randomPorts bool, } } } else { - for { - freePort, err = util.NextFreePort(startPort, endPort, usedPorts) - if err != nil { - klog.Infof("%s", err) - continue epLoop - } - if !isPortUsedInContainer(freePort) { - break - } - startPort = freePort + 1 - time.Sleep(100 * time.Millisecond) + var err error + freePort, err = util.NextFreePort(startPort, endPort, usedPorts) + if err != nil { + klog.Infof("%s", err) + continue epLoop } startPort = freePort + 1 } @@ -264,7 +255,7 @@ func getPortMapping(devfileObj parser.DevfileObj, debug bool, randomPorts bool, result = append(result, fp) } } - return result, nil + return result } func addVolumeMountToContainer(containers []corev1.Container, devfileVolume storage.LocalStorage) error { @@ -287,3 +278,64 @@ func getUsedPorts(ports []api.ForwardedPort) []int { } return res } + +func getCustomPortPairs(definedPorts []api.ForwardedPort, ceMapping map[string][]v1alpha2.Endpoint, usedPorts []int) []api.ForwardedPort { + var result []api.ForwardedPort + customLocalPorts := make(map[int]struct{}) + + for _, dPort := range definedPorts { + customLocalPorts[dPort.LocalPort] = struct{}{} + } + // getCustomLocalPort analyzes the definedPorts i.e. custom port forwarding to see if a containerPort has a custom localPort, if a container name is provided, it also takes that into account. + getCustomLocalPort := func(containerPort int, container string) int { + for _, dp := range definedPorts { + if dp.ContainerPort == containerPort { + if dp.ContainerName != "" { + if dp.ContainerName == container { + return dp.LocalPort + } + } else { + return dp.LocalPort + } + } + } + return 0 + } + startPort := 20001 + endPort := startPort + 10000 + for name, ports := range ceMapping { + for _, p := range ports { + freePort := getCustomLocalPort(p.TargetPort, name) + if freePort == 0 { + for { + var err error + freePort, err = util.NextFreePort(startPort, endPort, usedPorts) + if err != nil { + klog.Infof("%s", err) + continue + } + if _, isPortUsed := customLocalPorts[freePort]; isPortUsed { + startPort = freePort + 1 + continue + } + break + } + startPort = freePort + 1 + } + fp := api.ForwardedPort{ + Platform: commonflags.PlatformPodman, + PortName: p.Name, + IsDebug: libdevfile.IsDebugPort(p.Name), + ContainerName: name, + LocalAddress: "127.0.0.1", + LocalPort: freePort, + ContainerPort: p.TargetPort, + Exposure: string(p.Exposure), + Protocol: string(p.Protocol), + } + result = append(result, fp) + } + } + + return result +} From defd7ce33e04cdbb534901c9a5fc3b844645415b Mon Sep 17 00:00:00 2001 From: Parthvi Vala Date: Mon, 10 Apr 2023 17:40:01 +0530 Subject: [PATCH 3/8] Add unit tests --- pkg/dev/podmandev/pod_test.go | 408 +++++++++++++++++++++++++++++++++- 1 file changed, 400 insertions(+), 8 deletions(-) diff --git a/pkg/dev/podmandev/pod_test.go b/pkg/dev/podmandev/pod_test.go index fa6d48a92cc..cb032ecd8ad 100644 --- a/pkg/dev/podmandev/pod_test.go +++ b/pkg/dev/podmandev/pod_test.go @@ -127,14 +127,15 @@ func buildBasePod(withPfContainer bool) *corev1.Pod { func Test_createPodFromComponent(t *testing.T) { type args struct { - devfileObj func() parser.DevfileObj - componentName string - appName string - debug bool - buildCommand string - runCommand string - debugCommand string - forwardLocalhost bool + devfileObj func() parser.DevfileObj + componentName string + appName string + debug bool + buildCommand string + runCommand string + debugCommand string + forwardLocalhost bool + customForwardedPorts []api.ForwardedPort } tests := []struct { name string @@ -944,6 +945,210 @@ func Test_createPodFromComponent(t *testing.T) { }, }, }, + + { + name: "basic component + application endpoint + debug endpoint - with debug / custom mapping for port forwarding with container name (customForwardedPorts)", + args: args{ + devfileObj: func() parser.DevfileObj { + data, _ := data.NewDevfileData(string(data.APISchemaVersion200)) + _ = data.AddCommands([]v1alpha2.Command{command}) + cmp := baseComponent.DeepCopy() + cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ + Name: "http", + TargetPort: 8080, + }) + cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ + Name: "debug", + TargetPort: 5858, + }) + _ = data.AddComponents([]v1alpha2.Component{*cmp}) + return parser.DevfileObj{ + Data: data, + } + }, + componentName: devfileName, + appName: appName, + debug: true, + customForwardedPorts: []api.ForwardedPort{ + { + LocalPort: 8080, + ContainerPort: 8080, + }, + }, + }, + wantPod: func(basePod *corev1.Pod) *corev1.Pod { + pod := basePod.DeepCopy() + pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ + Name: "http", + ContainerPort: 8080, + HostPort: 8080, + Protocol: corev1.ProtocolTCP, + }) + pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ + Name: "debug", + ContainerPort: 5858, + HostPort: 20001, + Protocol: corev1.ProtocolTCP, + }) + return pod + }, + wantFwPorts: []api.ForwardedPort{ + { + Platform: "podman", + ContainerName: "mycomponent", + PortName: "http", + LocalAddress: "127.0.0.1", + LocalPort: 8080, + ContainerPort: 8080, + IsDebug: false, + }, + { + Platform: "podman", + ContainerName: "mycomponent", + PortName: "debug", + LocalAddress: "127.0.0.1", + LocalPort: 20001, + ContainerPort: 5858, + IsDebug: true, + }, + }, + }, + { + name: "basic component + application endpoint + debug endpoint - with debug / custom mapping for port forwarding without container name (customForwardedPorts)", + args: args{ + devfileObj: func() parser.DevfileObj { + data, _ := data.NewDevfileData(string(data.APISchemaVersion200)) + _ = data.AddCommands([]v1alpha2.Command{command}) + cmp := baseComponent.DeepCopy() + cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ + Name: "http", + TargetPort: 8080, + }) + cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ + Name: "debug", + TargetPort: 5858, + }) + _ = data.AddComponents([]v1alpha2.Component{*cmp}) + return parser.DevfileObj{ + Data: data, + } + }, + componentName: devfileName, + appName: appName, + debug: true, + customForwardedPorts: []api.ForwardedPort{ + { + ContainerName: "mycomponent", + LocalPort: 8080, + ContainerPort: 8080, + }, + }, + }, + wantPod: func(basePod *corev1.Pod) *corev1.Pod { + pod := basePod.DeepCopy() + pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ + Name: "http", + ContainerPort: 8080, + HostPort: 8080, + Protocol: corev1.ProtocolTCP, + }) + pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ + Name: "debug", + ContainerPort: 5858, + HostPort: 20001, + Protocol: corev1.ProtocolTCP, + }) + return pod + }, + wantFwPorts: []api.ForwardedPort{ + { + Platform: "podman", + ContainerName: "mycomponent", + PortName: "http", + LocalAddress: "127.0.0.1", + LocalPort: 8080, + ContainerPort: 8080, + IsDebug: false, + }, + { + Platform: "podman", + ContainerName: "mycomponent", + PortName: "debug", + LocalAddress: "127.0.0.1", + LocalPort: 20001, + ContainerPort: 5858, + IsDebug: true, + }, + }, + }, + { + name: "basic component + application endpoint + debug endpoint - with debug / custom mapping for port forwarding with local port in ranged [20001-30001] ports (customForwardedPorts)", + args: args{ + devfileObj: func() parser.DevfileObj { + data, _ := data.NewDevfileData(string(data.APISchemaVersion200)) + _ = data.AddCommands([]v1alpha2.Command{command}) + cmp := baseComponent.DeepCopy() + cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ + Name: "http", + TargetPort: 8080, + }) + cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ + Name: "debug", + TargetPort: 5858, + }) + _ = data.AddComponents([]v1alpha2.Component{*cmp}) + return parser.DevfileObj{ + Data: data, + } + }, + componentName: devfileName, + appName: appName, + debug: true, + customForwardedPorts: []api.ForwardedPort{ + { + ContainerName: "mycomponent", + LocalPort: 20001, + ContainerPort: 8080, + }, + }, + }, + wantPod: func(basePod *corev1.Pod) *corev1.Pod { + pod := basePod.DeepCopy() + pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ + Name: "http", + ContainerPort: 8080, + HostPort: 20001, + Protocol: corev1.ProtocolTCP, + }) + pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ + Name: "debug", + ContainerPort: 5858, + HostPort: 20002, + Protocol: corev1.ProtocolTCP, + }) + return pod + }, + wantFwPorts: []api.ForwardedPort{ + { + Platform: "podman", + ContainerName: "mycomponent", + PortName: "http", + LocalAddress: "127.0.0.1", + LocalPort: 20001, + ContainerPort: 8080, + IsDebug: false, + }, + { + Platform: "podman", + ContainerName: "mycomponent", + PortName: "debug", + LocalAddress: "127.0.0.1", + LocalPort: 20002, + ContainerPort: 5858, + IsDebug: true, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -957,6 +1162,7 @@ func Test_createPodFromComponent(t *testing.T) { tt.args.debugCommand, tt.args.forwardLocalhost, false, + tt.args.customForwardedPorts, []int{20001, 20002, 20003, 20004, 20005}, ) if (err != nil) != tt.wantErr { @@ -974,3 +1180,189 @@ func Test_createPodFromComponent(t *testing.T) { }) } } + +func Test_getCustomPortPairs(t *testing.T) { + type args struct { + definedPorts []api.ForwardedPort + ceMapping map[string][]v1alpha2.Endpoint + usedPorts []int + } + tests := []struct { + name string + args args + want []api.ForwardedPort + }{ + // TODO: Add test cases. + { + name: "ports are provided with container name", + args: args{ + definedPorts: []api.ForwardedPort{ + {ContainerName: "runtime", LocalPort: 8080, ContainerPort: 8000}, + }, + ceMapping: map[string][]v1alpha2.Endpoint{ + "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, + "tools": {{TargetPort: 5000}}, + }, + usedPorts: nil, + }, + want: []api.ForwardedPort{ + { + Platform: "podman", + ContainerName: "runtime", + LocalAddress: "127.0.0.1", + LocalPort: 8080, + ContainerPort: 8000, + }, + { + Platform: "podman", + ContainerName: "runtime", + LocalPort: 20001, + LocalAddress: "127.0.0.1", + ContainerPort: 9000, + }, + { + Platform: "podman", + ContainerName: "tools", + LocalAddress: "127.0.0.1", + LocalPort: 20002, + ContainerPort: 5000, + }, + }, + }, + { + name: "ports are provided without container name", + args: args{ + definedPorts: []api.ForwardedPort{ + {LocalPort: 8080, ContainerPort: 8000}, + {LocalPort: 5000, ContainerPort: 5000}, + }, + ceMapping: map[string][]v1alpha2.Endpoint{ + "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, + "tools": {{TargetPort: 5000}}, + }, + usedPorts: nil, + }, + want: []api.ForwardedPort{ + { + Platform: "podman", + ContainerName: "runtime", + LocalAddress: "127.0.0.1", + LocalPort: 8080, + ContainerPort: 8000, + }, + { + Platform: "podman", + ContainerName: "runtime", + LocalPort: 20001, + LocalAddress: "127.0.0.1", + ContainerPort: 9000, + }, + { + Platform: "podman", + ContainerName: "tools", + LocalAddress: "127.0.0.1", + LocalPort: 5000, + ContainerPort: 5000, + }, + }, + }, + + { + name: "custom local ports is same as one of the random ranged [20001-30001] ports", + args: args{ + definedPorts: []api.ForwardedPort{ + {LocalPort: 20001, ContainerPort: 8000}, + {LocalPort: 20002, ContainerPort: 9000}, + {LocalPort: 5000, ContainerPort: 5000}, + }, + ceMapping: map[string][]v1alpha2.Endpoint{ + "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, + "tools": {{TargetPort: 5000}, {TargetPort: 8080}}, + }, + usedPorts: nil, + }, + want: []api.ForwardedPort{ + { + Platform: "podman", + ContainerName: "runtime", + LocalAddress: "127.0.0.1", + LocalPort: 20001, + ContainerPort: 8000, + }, + { + Platform: "podman", + ContainerName: "runtime", + LocalPort: 20002, + LocalAddress: "127.0.0.1", + ContainerPort: 9000, + }, + { + Platform: "podman", + ContainerName: "tools", + LocalAddress: "127.0.0.1", + LocalPort: 5000, + ContainerPort: 5000, + }, + { + Platform: "podman", + ContainerName: "tools", + LocalAddress: "127.0.0.1", + LocalPort: 20003, + ContainerPort: 8080, + }, + }, + }, + { + name: "should reuse a port if it is defined in usedPorts", + args: args{ + definedPorts: []api.ForwardedPort{ + {LocalPort: 20001, ContainerPort: 8000}, + {LocalPort: 5000, ContainerPort: 5000}, + }, + ceMapping: map[string][]v1alpha2.Endpoint{ + "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, + "tools": {{TargetPort: 5000}, {TargetPort: 8080}}, + }, + usedPorts: []int{20002, 20003}, + }, + want: []api.ForwardedPort{ + { + Platform: "podman", + ContainerName: "runtime", + LocalAddress: "127.0.0.1", + LocalPort: 20001, + ContainerPort: 8000, + }, + { + Platform: "podman", + ContainerName: "runtime", + LocalPort: 20002, + LocalAddress: "127.0.0.1", + ContainerPort: 9000, + }, + { + Platform: "podman", + ContainerName: "tools", + LocalAddress: "127.0.0.1", + LocalPort: 5000, + ContainerPort: 5000, + }, + { + Platform: "podman", + ContainerName: "tools", + LocalAddress: "127.0.0.1", + LocalPort: 20003, + ContainerPort: 8000, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := getCustomPortPairs(tt.args.definedPorts, tt.args.ceMapping, tt.args.usedPorts) + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Errorf("getCustomPortPairs() = %v", diff) + } + }) + } +} From f7495140e44fc5543135a23bd54a0d85ccb7b12f Mon Sep 17 00:00:00 2001 From: Parthvi Vala Date: Mon, 10 Apr 2023 17:46:21 +0530 Subject: [PATCH 4/8] Move custom port forward formation to the general getPortMapping function Signed-off-by: Parthvi Vala --- pkg/dev/podmandev/pod.go | 94 +++++++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 25 deletions(-) diff --git a/pkg/dev/podmandev/pod.go b/pkg/dev/podmandev/pod.go index a6d1e1272d8..880f85ae4ea 100644 --- a/pkg/dev/podmandev/pod.go +++ b/pkg/dev/podmandev/pod.go @@ -51,20 +51,15 @@ func createPodFromComponent( return nil, nil, fmt.Errorf("no valid components found in the devfile") } - containerComponents, err := devfileObj.Data.GetComponents(common.DevfileOptions{ - ComponentOptions: common.ComponentOptions{ComponentType: v1alpha2.ContainerComponentType}, - }) + var fwPorts []api.ForwardedPort + // if len(customForwardedPorts) != 0 { + // fwPorts = getCustomPortPairs(customForwardedPorts, ceMapping, usedPorts) + // } else { + fwPorts, err = getPortMapping(devfileObj, debug, randomPorts, usedPorts, customForwardedPorts) if err != nil { return nil, nil, err } - ceMapping := libdevfile.GetContainerEndpointMapping(containerComponents, debug) - - var fwPorts []api.ForwardedPort - if len(customForwardedPorts) != 0 { - fwPorts = getCustomPortPairs(customForwardedPorts, ceMapping, usedPorts) - } else { - fwPorts = getPortMapping(ceMapping, debug, randomPorts, usedPorts) - } + // } utils.AddOdoProjectVolume(&containers) utils.AddOdoMandatoryVolume(&containers) @@ -185,7 +180,15 @@ func getVolumeName(volume string, componentName string, appName string) string { return volume + "-" + componentName + "-" + appName } -func getPortMapping(ceMapping map[string][]v1alpha2.Endpoint, debug bool, randomPorts bool, usedPorts []int) []api.ForwardedPort { +func getPortMapping(devfileObj parser.DevfileObj, debug bool, randomPorts bool, usedPorts []int, definedPorts []api.ForwardedPort) ([]api.ForwardedPort, error) { + containerComponents, err := devfileObj.Data.GetComponents(common.DevfileOptions{ + ComponentOptions: common.ComponentOptions{ComponentType: v1alpha2.ContainerComponentType}, + }) + if err != nil { + return nil, err + } + ceMapping := libdevfile.GetContainerEndpointMapping(containerComponents, debug) + var existingContainerPorts []int for _, endpoints := range ceMapping { for _, ep := range endpoints { @@ -193,6 +196,12 @@ func getPortMapping(ceMapping map[string][]v1alpha2.Endpoint, debug bool, random } } + // this list makes sure that we ranged ports[20001-30001] do not coincide with a custom local port + customLocalPorts := make(map[int]struct{}) + for _, dPort := range definedPorts { + customLocalPorts[dPort.LocalPort] = struct{}{} + } + isPortUsedInContainer := func(p int) bool { for _, port := range existingContainerPorts { if p == port { @@ -202,6 +211,22 @@ func getPortMapping(ceMapping map[string][]v1alpha2.Endpoint, debug bool, random return false } + // getCustomLocalPort analyzes the definedPorts i.e. custom port forwarding to see if a containerPort has a custom localPort, if a container name is provided, it also takes that into account. + getCustomLocalPort := func(containerPort int, container string) int { + for _, dp := range definedPorts { + if dp.ContainerPort == containerPort { + if dp.ContainerName != "" { + if dp.ContainerName == container { + return dp.LocalPort + } + } else { + return dp.LocalPort + } + } + } + return 0 + } + var result []api.ForwardedPort startPort := 20001 endPort := startPort + 10000 @@ -218,14 +243,33 @@ func getPortMapping(ceMapping map[string][]v1alpha2.Endpoint, debug bool, random continue } var freePort int - if randomPorts { + if len(definedPorts) != 0 { + freePort = getCustomLocalPort(ep.TargetPort, containerName) + if freePort == 0 { + for { + var err error + freePort, err = util.NextFreePort(startPort, endPort, usedPorts) + if err != nil { + klog.Infof("%s", err) + continue + } + // ensure that freePort is not a custom local port + if _, isPortUsed := customLocalPorts[freePort]; isPortUsed { + startPort = freePort + 1 + continue + } + break + } + startPort = freePort + 1 + } + } else if randomPorts { if len(usedPortsCopy) != 0 { freePort = usedPortsCopy[0] usedPortsCopy = usedPortsCopy[1:] } else { - rand.Seed(time.Now().UnixNano()) //#nosec + rand.Seed(time.Now().UnixNano()) // #nosec for { - freePort = rand.Intn(endPort-startPort+1) + startPort //#nosec + freePort = rand.Intn(endPort-startPort+1) + startPort // #nosec if !isPortUsedInContainer(freePort) && util.IsPortFree(freePort) { break } @@ -255,7 +299,7 @@ func getPortMapping(ceMapping map[string][]v1alpha2.Endpoint, debug bool, random result = append(result, fp) } } - return result + return result, nil } func addVolumeMountToContainer(containers []corev1.Container, devfileVolume storage.LocalStorage) error { @@ -303,9 +347,9 @@ func getCustomPortPairs(definedPorts []api.ForwardedPort, ceMapping map[string][ } startPort := 20001 endPort := startPort + 10000 - for name, ports := range ceMapping { - for _, p := range ports { - freePort := getCustomLocalPort(p.TargetPort, name) + for containerName, endpoints := range ceMapping { + for _, ep := range endpoints { + freePort := getCustomLocalPort(ep.TargetPort, containerName) if freePort == 0 { for { var err error @@ -324,14 +368,14 @@ func getCustomPortPairs(definedPorts []api.ForwardedPort, ceMapping map[string][ } fp := api.ForwardedPort{ Platform: commonflags.PlatformPodman, - PortName: p.Name, - IsDebug: libdevfile.IsDebugPort(p.Name), - ContainerName: name, + PortName: ep.Name, + IsDebug: libdevfile.IsDebugPort(ep.Name), + ContainerName: containerName, LocalAddress: "127.0.0.1", LocalPort: freePort, - ContainerPort: p.TargetPort, - Exposure: string(p.Exposure), - Protocol: string(p.Protocol), + ContainerPort: ep.TargetPort, + Exposure: string(ep.Exposure), + Protocol: string(ep.Protocol), } result = append(result, fp) } From 070c210c8d9070e55ee659afad74a3b4a8fba0e5 Mon Sep 17 00:00:00 2001 From: Parthvi Vala Date: Mon, 10 Apr 2023 19:10:13 +0530 Subject: [PATCH 5/8] Integration test Signed-off-by: Parthvi Vala --- tests/integration/cmd_dev_test.go | 661 ++++++++++++------------------ 1 file changed, 256 insertions(+), 405 deletions(-) diff --git a/tests/integration/cmd_dev_test.go b/tests/integration/cmd_dev_test.go index aa0edf27401..06f370691ea 100644 --- a/tests/integration/cmd_dev_test.go +++ b/tests/integration/cmd_dev_test.go @@ -712,459 +712,310 @@ ComponentSettings: }) } - // TODO(pvala): Merge this into the test below once custom port mapping for port-forwarding has been implemented for podman. - Context("port-forwarding for the component with custom port mapping", func() { - When("a component is bootstrapped", func() { - BeforeEach(func() { - helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() - helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass() - }) - - It("should fail when using --random-ports and --port-forward together", func() { - errOut := helper.Cmd("odo", "dev", "--random-ports", "--port-forward=8000:3000").ShouldFail().Err() - Expect(errOut).To(ContainSubstring("--random-ports and --port-forward cannot be used together")) - }) - - }) - When("devfile has single endpoint", func() { - var ( - LocalPort = helper.GetRandomFreePort() - ) - const ( - ContainerPort = "3000" - ) - BeforeEach(func() { - helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() - helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass() - }) - - When("running odo dev", func() { - var devSession helper.DevSession - var ports map[string]string - BeforeEach(func() { - var err error - opts := []string{fmt.Sprintf("--port-forward=%s:%s", LocalPort, ContainerPort)} - devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ - CmdlineArgs: opts, - NoRandomPorts: true, + for _, manual := range []bool{true, false} { + for _, customPortForwarding := range []bool{true, false} { + for _, podman := range []bool{true, false} { + manual := manual + customPortForwarding := customPortForwarding + podman := podman + Context("port-forwarding for the component", helper.LabelPodmanIf(podman, func() { + var NoRandomPorts bool + if customPortForwarding { + NoRandomPorts = true + } + When("a component is bootstrapped", func() { + BeforeEach(func() { + helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) + helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass() + }) + if customPortForwarding { + It("should fail when using --random-ports and --port-forward together", func() { + args := []string{"dev", "--random-ports", "--port-forward=8000:3000"} + if podman { + args = append(args, "--platform", "podman") + } + errOut := helper.Cmd("odo", args...).ShouldFail().Err() + Expect(errOut).To(ContainSubstring("--random-ports and --port-forward cannot be used together")) + }) + } }) - Expect(err).ToNot(HaveOccurred()) - }) - - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() - }) + if !customPortForwarding { + When("devfile has no endpoint", func() { + BeforeEach(func() { + if !podman { + helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() + } + helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) + helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-no-endpoint.yaml")).ShouldPass() + }) - It("should expose the endpoint on localhost", func() { - url := fmt.Sprintf("http://%s", ports[ContainerPort]) - Expect(url).To(ContainSubstring(LocalPort)) - resp, err := http.Get(url) - Expect(err).ToNot(HaveOccurred()) - defer resp.Body.Close() + When("running odo dev", func() { + var devSession helper.DevSession + var ports map[string]string + BeforeEach(func() { + var err error + opts := []string{} + if manual { + opts = append(opts, "--no-watch") + } + devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ + CmdlineArgs: opts, + RunOnPodman: podman, + }) + Expect(err).ToNot(HaveOccurred()) + }) - body, _ := io.ReadAll(resp.Body) - helper.MatchAllInOutput(string(body), []string{"Hello from Node.js Starter Application!"}) - Expect(err).ToNot(HaveOccurred()) - }) + AfterEach(func() { + devSession.Stop() + devSession.WaitEnd() + }) - When("modifying memoryLimit for container in Devfile", func() { - var stdout string - var stderr string - BeforeEach(func() { - src := "memoryLimit: 1024Mi" - dst := "memoryLimit: 1023Mi" - helper.ReplaceString("devfile.yaml", src, dst) - var err error - var stdoutBytes []byte - var stderrBytes []byte - stdoutBytes, stderrBytes, ports, err = devSession.WaitSync() - Expect(err).Should(Succeed()) - stdout = string(stdoutBytes) - stderr = string(stderrBytes) - }) - - It("should react on the Devfile modification", func() { - By("not warning users that odo dev needs to be restarted", func() { - warning := "Please restart 'odo dev'" - Expect(stdout).ShouldNot(ContainSubstring(warning)) - Expect(stderr).ShouldNot(ContainSubstring(warning)) + It("should have no endpoint forwarded", func() { + Expect(ports).To(BeEmpty()) + }) + }) }) - By("updating the pod", func() { - podName := commonVar.CliRunner.GetRunningPodNameByComponent(cmpName, commonVar.Project) - bufferOutput := commonVar.CliRunner.Run("get", "pods", podName, "-o", "jsonpath='{.spec.containers[0].resources.requests.memory}'").Out.Contents() - output := string(bufferOutput) - Expect(output).To(ContainSubstring("1023Mi")) + } + When("devfile has single endpoint", func() { + var ( + LocalPort = helper.GetRandomFreePort() + ) + const ( + ContainerPort = "3000" + ) + BeforeEach(func() { + helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) + helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass() }) - By("exposing the endpoint", func() { - Eventually(func(g Gomega) { + When("running odo dev", func() { + var devSession helper.DevSession + var ports map[string]string + BeforeEach(func() { + var err error + opts := []string{} + if customPortForwarding { + opts = []string{fmt.Sprintf("--port-forward=%s:%s", LocalPort, ContainerPort)} + } + if manual { + opts = append(opts, "--no-watch") + } + devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ + CmdlineArgs: opts, + NoRandomPorts: NoRandomPorts, + RunOnPodman: podman, + }) + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + devSession.Stop() + devSession.WaitEnd() + }) + + It("should expose the endpoint on localhost", func() { url := fmt.Sprintf("http://%s", ports[ContainerPort]) - Expect(url).To(ContainSubstring(LocalPort)) + if customPortForwarding { + Expect(url).To(ContainSubstring(LocalPort)) + } resp, err := http.Get(url) - g.Expect(err).ToNot(HaveOccurred()) + Expect(err).ToNot(HaveOccurred()) defer resp.Body.Close() body, _ := io.ReadAll(resp.Body) - for _, i := range []string{"Hello from Node.js Starter Application!"} { - g.Expect(string(body)).To(ContainSubstring(i)) - } - g.Expect(err).ToNot(HaveOccurred()) - }).WithPolling(1 * time.Second).WithTimeout(20 * time.Second).Should(Succeed()) - }) - }) - }) - }) - }) - - When("devfile has multiple endpoints", func() { - var ( - LocalPort1 = helper.GetRandomFreePort() - LocalPort2 = helper.GetRandomFreePort() - LocalPort3 = helper.GetRandomFreePort() - ) - const ( - // ContainerPort are hard-coded from devfile-with-multiple-endpoints.yaml - // Note 1: Debug endpoints will not be exposed for this instance, so we do not add custom mapping for them. - // Note 2: We add custom mapping for all the endpoints so that none of them are assigned random ports from the 20001-30001 range; - // Note 2(contd.): this is to avoid a race condition where a test running in parallel is also assigned similar ranged port the one here, and we fail to access either of them. - ContainerPort1 = "3000" - ContainerPort2 = "4567" - ContainerPort3 = "7890" - ) - BeforeEach(func() { - helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() - helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project-with-multiple-endpoints"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-with-multiple-endpoints.yaml")).ShouldPass() - }) - - When("running odo dev", func() { - var devSession helper.DevSession - var ports map[string]string - BeforeEach(func() { - opts := []string{fmt.Sprintf("--port-forward=%s:%s", LocalPort1, ContainerPort1), fmt.Sprintf("--port-forward=%s:%s", LocalPort2, ContainerPort2), fmt.Sprintf("--port-forward=%s:%s", LocalPort3, ContainerPort3)} - var err error - devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ - CmdlineArgs: opts, - NoRandomPorts: true, - }) - Expect(err).ToNot(HaveOccurred()) - }) - - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() - }) - - It("should expose all endpoints on localhost regardless of exposure", func() { - By("not exposing debug endpoints", func() { - for _, p := range []int{5005, 5006} { - _, found := ports[strconv.Itoa(p)] - Expect(found).To(BeFalse(), fmt.Sprintf("debug port %d should not be forwarded", p)) - } - }) - - getServerResponse := func(containerPort, localPort string) (string, error) { - url := fmt.Sprintf("http://%s", ports[containerPort]) - Expect(url).To(ContainSubstring(localPort)) - resp, err := http.Get(url) - if err != nil { - return "", err - } - defer resp.Body.Close() - - body, _ := io.ReadAll(resp.Body) - return string(body), nil - } - containerPorts := []string{ContainerPort1, ContainerPort2, ContainerPort3} - localPorts := []string{LocalPort1, LocalPort2, LocalPort3} - - for i := range containerPorts { - containerPort := containerPorts[i] - localPort := localPorts[i] - By(fmt.Sprintf("exposing a port targeting container port %s", containerPort), func() { - r, err := getServerResponse(containerPort, localPort) - Expect(err).ShouldNot(HaveOccurred()) - helper.MatchAllInOutput(r, []string{"Hello from Node.js Starter Application!"}) - }) - } - - helper.ReplaceString("server.js", "Hello from Node.js", "H3110 from Node.js") - - var stdout, stderr []byte - var err error - stdout, stderr, _, err = devSession.WaitSync() - Expect(err).Should(Succeed()) - - By("not warning users that odo dev needs to be restarted because the Devfile has not changed", func() { - warning := "Please restart 'odo dev'" - Expect(stdout).ShouldNot(ContainSubstring(warning)) - Expect(stderr).ShouldNot(ContainSubstring(warning)) - }) - - for i := range containerPorts { - containerPort := containerPorts[i] - localPort := localPorts[i] - By(fmt.Sprintf("returning the right response when querying port forwarded for container port %s", containerPort), - func() { - Eventually(func(g Gomega) string { - r, err := getServerResponse(containerPort, localPort) - g.Expect(err).ShouldNot(HaveOccurred()) - return r - }, 180, 10).Should(Equal("H3110 from Node.js Starter Application!")) + helper.MatchAllInOutput(string(body), []string{"Hello from Node.js Starter Application!"}) + Expect(err).ToNot(HaveOccurred()) }) - } - }) - }) - }) - }) + When("modifying memoryLimit for container in Devfile", func() { + var stdout string + var stderr string + BeforeEach(func() { + src := "memoryLimit: 1024Mi" + dst := "memoryLimit: 1023Mi" + helper.ReplaceString("devfile.yaml", src, dst) + if manual { + if os.Getenv("SKIP_KEY_PRESS") == "true" { + Skip("This is a unix-terminal specific scenario, skipping") + } - for _, manual := range []bool{false, true} { - for _, podman := range []bool{false, true} { - manual := manual - podman := podman - Context("port-forwarding for the component", helper.LabelPodmanIf(podman, func() { - When("devfile has no endpoint", func() { - BeforeEach(func() { - if !podman { - helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() - } - helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-no-endpoint.yaml")).ShouldPass() - }) + devSession.PressKey('p') + } + var err error + var stdoutBytes []byte + var stderrBytes []byte + stdoutBytes, stderrBytes, ports, err = devSession.WaitSync() + Expect(err).Should(Succeed()) + stdout = string(stdoutBytes) + stderr = string(stderrBytes) + }) - When("running odo dev", func() { - var devSession helper.DevSession - var ports map[string]string - BeforeEach(func() { - var err error - opts := []string{} - if manual { - opts = append(opts, "--no-watch") - } - devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ - CmdlineArgs: opts, - RunOnPodman: podman, + It("should react on the Devfile modification", func() { + if podman { + By("warning users that odo dev needs to be restarted", func() { + Expect(stdout).To(ContainSubstring( + "Detected changes in the Devfile, but this is not supported yet on Podman. Please restart 'odo dev' for such changes to be applied.")) + }) + } else { + By("not warning users that odo dev needs to be restarted", func() { + warning := "Please restart 'odo dev'" + Expect(stdout).ShouldNot(ContainSubstring(warning)) + Expect(stderr).ShouldNot(ContainSubstring(warning)) + }) + By("updating the pod", func() { + podName := commonVar.CliRunner.GetRunningPodNameByComponent(cmpName, commonVar.Project) + bufferOutput := commonVar.CliRunner.Run("get", "pods", podName, "-o", "jsonpath='{.spec.containers[0].resources.requests.memory}'").Out.Contents() + output := string(bufferOutput) + Expect(output).To(ContainSubstring("1023Mi")) + }) + + By("exposing the endpoint", func() { + Eventually(func(g Gomega) { + url := fmt.Sprintf("http://%s", ports[ContainerPort]) + if customPortForwarding { + Expect(url).To(ContainSubstring(LocalPort)) + } + resp, err := http.Get(url) + g.Expect(err).ToNot(HaveOccurred()) + defer resp.Body.Close() + + body, _ := io.ReadAll(resp.Body) + for _, i := range []string{"Hello from Node.js Starter Application!"} { + g.Expect(string(body)).To(ContainSubstring(i)) + } + g.Expect(err).ToNot(HaveOccurred()) + }).WithPolling(1 * time.Second).WithTimeout(20 * time.Second).Should(Succeed()) + }) + } + }) }) - Expect(err).ToNot(HaveOccurred()) - }) - - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() - }) - - It("should have no endpoint forwarded", func() { - Expect(ports).To(BeEmpty()) }) }) - }) - When("devfile has single endpoint", func() { - BeforeEach(func() { - if !podman { - helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() - } - helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass() - }) - - When("running odo dev", func() { - var devSession helper.DevSession - var ports map[string]string + When("devfile has multiple endpoints", func() { + var ( + LocalPort1 = helper.GetRandomFreePort() + LocalPort2 = helper.GetRandomFreePort() + LocalPort3 = helper.GetRandomFreePort() + ) + const ( + // ContainerPort are hard-coded from devfile-with-multiple-endpoints.yaml + // Note 1: Debug endpoints will not be exposed for this instance, so we do not add custom mapping for them. + // Note 2: We add custom mapping for all the endpoints so that none of them are assigned random ports from the 20001-30001 range; + // Note 2(contd.): this is to avoid a race condition where a test running in parallel is also assigned similar ranged port the one here, and we fail to access either of them. + ContainerPort1 = "3000" + ContainerPort2 = "4567" + ContainerPort3 = "7890" + ) BeforeEach(func() { - var err error - opts := []string{} - if manual { - opts = append(opts, "--no-watch") - } - devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ - CmdlineArgs: opts, - RunOnPodman: podman, - }) - Expect(err).ToNot(HaveOccurred()) - }) - - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() - }) - - It("should expose the endpoint on localhost", func() { - url := fmt.Sprintf("http://%s", ports["3000"]) - resp, err := http.Get(url) - Expect(err).ToNot(HaveOccurred()) - defer resp.Body.Close() - - body, _ := io.ReadAll(resp.Body) - helper.MatchAllInOutput(string(body), []string{"Hello from Node.js Starter Application!"}) - Expect(err).ToNot(HaveOccurred()) + helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project-with-multiple-endpoints"), commonVar.Context) + helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-with-multiple-endpoints.yaml")).ShouldPass() }) - When("modifying memoryLimit for container in Devfile", func() { - var stdout string - var stderr string + When("running odo dev", func() { + var devSession helper.DevSession + var ports map[string]string BeforeEach(func() { - src := "memoryLimit: 1024Mi" - dst := "memoryLimit: 1023Mi" - helper.ReplaceString("devfile.yaml", src, dst) + opts := []string{} + if customPortForwarding { + opts = []string{fmt.Sprintf("--port-forward=%s:%s", LocalPort1, ContainerPort1), fmt.Sprintf("--port-forward=%s:%s", LocalPort2, ContainerPort2), fmt.Sprintf("--port-forward=%s:%s", LocalPort3, ContainerPort3)} + } if manual { - if os.Getenv("SKIP_KEY_PRESS") == "true" { - Skip("This is a unix-terminal specific scenario, skipping") - } - - devSession.PressKey('p') + opts = append(opts, "--no-watch") } var err error - var stdoutBytes []byte - var stderrBytes []byte - stdoutBytes, stderrBytes, ports, err = devSession.WaitSync() - Expect(err).Should(Succeed()) - stdout = string(stdoutBytes) - stderr = string(stderrBytes) - }) - - It("should react on the Devfile modification", func() { - if podman { - By("warning users that odo dev needs to be restarted", func() { - Expect(stdout).To(ContainSubstring( - "Detected changes in the Devfile, but this is not supported yet on Podman. Please restart 'odo dev' for such changes to be applied.")) - }) - } else { - By("not warning users that odo dev needs to be restarted", func() { - warning := "Please restart 'odo dev'" - Expect(stdout).ShouldNot(ContainSubstring(warning)) - Expect(stderr).ShouldNot(ContainSubstring(warning)) - }) - By("updating the pod", func() { - podName := commonVar.CliRunner.GetRunningPodNameByComponent(cmpName, commonVar.Project) - bufferOutput := commonVar.CliRunner.Run("get", "pods", podName, "-o", "jsonpath='{.spec.containers[0].resources.requests.memory}'").Out.Contents() - output := string(bufferOutput) - Expect(output).To(ContainSubstring("1023Mi")) - }) - } - - By("exposing the endpoint", func() { - Eventually(func(g Gomega) { - url := fmt.Sprintf("http://%s", ports["3000"]) - resp, err := http.Get(url) - g.Expect(err).ToNot(HaveOccurred()) - defer resp.Body.Close() - - body, _ := io.ReadAll(resp.Body) - for _, i := range []string{"Hello from Node.js Starter Application!"} { - g.Expect(string(body)).To(ContainSubstring(i)) - } - g.Expect(err).ToNot(HaveOccurred()) - }).WithPolling(1 * time.Second).WithTimeout(20 * time.Second).Should(Succeed()) + devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ + CmdlineArgs: opts, + NoRandomPorts: NoRandomPorts, + RunOnPodman: podman, }) + Expect(err).ToNot(HaveOccurred()) }) - }) - }) - }) - - When("devfile has multiple endpoints", func() { - BeforeEach(func() { - if !podman { - helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() - } - helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project-with-multiple-endpoints"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-with-multiple-endpoints.yaml")).ShouldPass() - }) - When("running odo dev", func() { - var devSession helper.DevSession - var ports map[string]string - BeforeEach(func() { - opts := []string{} - if manual { - opts = append(opts, "--no-watch") - } - var err error - devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ - CmdlineArgs: opts, - RunOnPodman: podman, + AfterEach(func() { + devSession.Stop() + devSession.WaitEnd() }) - Expect(err).ToNot(HaveOccurred()) - }) - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() - }) + It("should expose all endpoints on localhost regardless of exposure", func() { + By("not exposing debug endpoints", func() { + for _, p := range []int{5005, 5006} { + _, found := ports[strconv.Itoa(p)] + Expect(found).To(BeFalse(), fmt.Sprintf("debug port %d should not be forwarded", p)) + } + }) - It("should expose all endpoints on localhost regardless of exposure", func() { - By("not exposing debug endpoints", func() { - for _, p := range []int{5005, 5006} { - _, found := ports[strconv.Itoa(p)] - Expect(found).To(BeFalse(), fmt.Sprintf("debug port %d should not be forwarded", p)) - } - }) + getServerResponse := func(containerPort, localPort string) (string, error) { + url := fmt.Sprintf("http://%s", ports[containerPort]) + if customPortForwarding { + Expect(url).To(ContainSubstring(localPort)) + } + resp, err := http.Get(url) + if err != nil { + return "", err + } + defer resp.Body.Close() - getServerResponse := func(p int) (string, error) { - resp, err := http.Get(fmt.Sprintf("http://%s", ports[strconv.Itoa(p)])) - if err != nil { - return "", err + body, _ := io.ReadAll(resp.Body) + return string(body), nil + } + containerPorts := []string{ContainerPort1, ContainerPort2, ContainerPort3} + localPorts := []string{LocalPort1, LocalPort2, LocalPort3} + + for i := range containerPorts { + containerPort := containerPorts[i] + localPort := localPorts[i] + By(fmt.Sprintf("exposing a port targeting container port %s", containerPort), func() { + r, err := getServerResponse(containerPort, localPort) + Expect(err).ShouldNot(HaveOccurred()) + helper.MatchAllInOutput(r, []string{"Hello from Node.js Starter Application!"}) + }) } - defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) - return string(body), nil - } - containerPorts := []int{3000, 4567, 7890} - for _, p := range containerPorts { - By(fmt.Sprintf("exposing a port targeting container port %d", p), func() { - r, err := getServerResponse(p) - Expect(err).ShouldNot(HaveOccurred()) - helper.MatchAllInOutput(r, []string{"Hello from Node.js Starter Application!"}) - }) - } + helper.ReplaceString("server.js", "Hello from Node.js", "H3110 from Node.js") - helper.ReplaceString("server.js", "Hello from Node.js", "H3110 from Node.js") + if manual { + if os.Getenv("SKIP_KEY_PRESS") == "true" { + Skip("This is a unix-terminal specific scenario, skipping") + } - if manual { - if os.Getenv("SKIP_KEY_PRESS") == "true" { - Skip("This is a unix-terminal specific scenario, skipping") + devSession.PressKey('p') } - devSession.PressKey('p') - } + var stdout, stderr []byte + var err error + stdout, stderr, _, err = devSession.WaitSync() + Expect(err).Should(Succeed()) - var stdout, stderr []byte - var err error - stdout, stderr, _, err = devSession.WaitSync() - Expect(err).Should(Succeed()) + By("not warning users that odo dev needs to be restarted because the Devfile has not changed", func() { + warning := "Please restart 'odo dev'" + if podman { + warning = "Detected changes in the Devfile, but this is not supported yet on Podman. Please restart 'odo dev' for such changes to be applied." + } + Expect(stdout).ShouldNot(ContainSubstring(warning)) + Expect(stderr).ShouldNot(ContainSubstring(warning)) + }) - By("not warning users that odo dev needs to be restarted because the Devfile has not changed", func() { - warning := "Please restart 'odo dev'" - if podman { - warning = "Detected changes in the Devfile, but this is not supported yet on Podman. Please restart 'odo dev' for such changes to be applied." + for i := range containerPorts { + containerPort := containerPorts[i] + localPort := localPorts[i] + By(fmt.Sprintf("returning the right response when querying port forwarded for container port %s", containerPort), + func() { + Eventually(func(g Gomega) string { + r, err := getServerResponse(containerPort, localPort) + g.Expect(err).ShouldNot(HaveOccurred()) + return r + }, 180, 10).Should(Equal("H3110 from Node.js Starter Application!")) + }) } - Expect(stdout).ShouldNot(ContainSubstring(warning)) - Expect(stderr).ShouldNot(ContainSubstring(warning)) }) - - for _, p := range containerPorts { - By(fmt.Sprintf("returning the right response when querying port forwarded for container port %d", p), - func() { - Eventually(func(g Gomega) string { - r, err := getServerResponse(p) - g.Expect(err).ShouldNot(HaveOccurred()) - return r - }, 180, 10).Should(Equal("H3110 from Node.js Starter Application!")) - }) - } }) - }) - }) - })...) + }) + })) + } } } - for _, devfileHandlerCtx := range []struct { name string sourceHandler func(path string, originalCmpName string) From b44b27407ca05dfcba18c1824f64802aa5e9abe9 Mon Sep 17 00:00:00 2001 From: Parthvi Vala Date: Tue, 11 Apr 2023 18:36:56 +0530 Subject: [PATCH 6/8] Fix unit tests Signed-off-by: Parthvi Vala --- pkg/dev/podmandev/pod.go | 81 +----- pkg/dev/podmandev/pod_test.go | 479 +++++++++++++++------------------- 2 files changed, 227 insertions(+), 333 deletions(-) diff --git a/pkg/dev/podmandev/pod.go b/pkg/dev/podmandev/pod.go index 880f85ae4ea..922618d14dc 100644 --- a/pkg/dev/podmandev/pod.go +++ b/pkg/dev/podmandev/pod.go @@ -52,9 +52,6 @@ func createPodFromComponent( } var fwPorts []api.ForwardedPort - // if len(customForwardedPorts) != 0 { - // fwPorts = getCustomPortPairs(customForwardedPorts, ceMapping, usedPorts) - // } else { fwPorts, err = getPortMapping(devfileObj, debug, randomPorts, usedPorts, customForwardedPorts) if err != nil { return nil, nil, err @@ -247,7 +244,6 @@ func getPortMapping(devfileObj parser.DevfileObj, debug bool, randomPorts bool, freePort = getCustomLocalPort(ep.TargetPort, containerName) if freePort == 0 { for { - var err error freePort, err = util.NextFreePort(startPort, endPort, usedPorts) if err != nil { klog.Infof("%s", err) @@ -277,11 +273,17 @@ func getPortMapping(devfileObj parser.DevfileObj, debug bool, randomPorts bool, } } } else { - var err error - freePort, err = util.NextFreePort(startPort, endPort, usedPorts) - if err != nil { - klog.Infof("%s", err) - continue epLoop + for { + freePort, err = util.NextFreePort(startPort, endPort, usedPorts) + if err != nil { + klog.Infof("%s", err) + continue epLoop + } + if !isPortUsedInContainer(freePort) { + break + } + startPort = freePort + 1 + time.Sleep(100 * time.Millisecond) } startPort = freePort + 1 } @@ -322,64 +324,3 @@ func getUsedPorts(ports []api.ForwardedPort) []int { } return res } - -func getCustomPortPairs(definedPorts []api.ForwardedPort, ceMapping map[string][]v1alpha2.Endpoint, usedPorts []int) []api.ForwardedPort { - var result []api.ForwardedPort - customLocalPorts := make(map[int]struct{}) - - for _, dPort := range definedPorts { - customLocalPorts[dPort.LocalPort] = struct{}{} - } - // getCustomLocalPort analyzes the definedPorts i.e. custom port forwarding to see if a containerPort has a custom localPort, if a container name is provided, it also takes that into account. - getCustomLocalPort := func(containerPort int, container string) int { - for _, dp := range definedPorts { - if dp.ContainerPort == containerPort { - if dp.ContainerName != "" { - if dp.ContainerName == container { - return dp.LocalPort - } - } else { - return dp.LocalPort - } - } - } - return 0 - } - startPort := 20001 - endPort := startPort + 10000 - for containerName, endpoints := range ceMapping { - for _, ep := range endpoints { - freePort := getCustomLocalPort(ep.TargetPort, containerName) - if freePort == 0 { - for { - var err error - freePort, err = util.NextFreePort(startPort, endPort, usedPorts) - if err != nil { - klog.Infof("%s", err) - continue - } - if _, isPortUsed := customLocalPorts[freePort]; isPortUsed { - startPort = freePort + 1 - continue - } - break - } - startPort = freePort + 1 - } - fp := api.ForwardedPort{ - Platform: commonflags.PlatformPodman, - PortName: ep.Name, - IsDebug: libdevfile.IsDebugPort(ep.Name), - ContainerName: containerName, - LocalAddress: "127.0.0.1", - LocalPort: freePort, - ContainerPort: ep.TargetPort, - Exposure: string(ep.Exposure), - Protocol: string(ep.Protocol), - } - result = append(result, fp) - } - } - - return result -} diff --git a/pkg/dev/podmandev/pod_test.go b/pkg/dev/podmandev/pod_test.go index cb032ecd8ad..9ec1236aa11 100644 --- a/pkg/dev/podmandev/pod_test.go +++ b/pkg/dev/podmandev/pod_test.go @@ -35,7 +35,9 @@ var ( baseComponent = generator.GetContainerComponent(generator.ContainerComponentParams{ Name: "mycomponent", Container: v1alpha2.Container{ - Image: "myimage", + Image: "myimage", + Args: []string{"-f", "/dev/null"}, + Command: []string{"tail"}, }, }) @@ -953,15 +955,27 @@ func Test_createPodFromComponent(t *testing.T) { data, _ := data.NewDevfileData(string(data.APISchemaVersion200)) _ = data.AddCommands([]v1alpha2.Command{command}) cmp := baseComponent.DeepCopy() - cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ - Name: "http", - TargetPort: 8080, - }) - cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ - Name: "debug", - TargetPort: 5858, - }) - _ = data.AddComponents([]v1alpha2.Component{*cmp}) + cmp.Name = "runtime" + cmp.Container.Endpoints = []v1alpha2.Endpoint{ + { + Name: "http-8080", + TargetPort: 8080, + }, + { + Name: "debug", + TargetPort: 5858, + }, + } + + cmp2 := baseComponent.DeepCopy() + cmp2.Name = "tools" + cmp2.Container.Endpoints = []v1alpha2.Endpoint{ + { + Name: "http-5000", + TargetPort: 5000, + }, + } + _ = data.AddComponents([]v1alpha2.Component{*cmp, *cmp2}) return parser.DevfileObj{ Data: data, } @@ -971,6 +985,7 @@ func Test_createPodFromComponent(t *testing.T) { debug: true, customForwardedPorts: []api.ForwardedPort{ { + ContainerName: "runtime", LocalPort: 8080, ContainerPort: 8080, }, @@ -978,25 +993,39 @@ func Test_createPodFromComponent(t *testing.T) { }, wantPod: func(basePod *corev1.Pod) *corev1.Pod { pod := basePod.DeepCopy() - pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ - Name: "http", - ContainerPort: 8080, - HostPort: 8080, - Protocol: corev1.ProtocolTCP, - }) - pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ - Name: "debug", - ContainerPort: 5858, - HostPort: 20001, - Protocol: corev1.ProtocolTCP, - }) + pod.Spec.Containers[0].Name = "runtime" + pod.Spec.Containers[0].Ports = []corev1.ContainerPort{ + { + Name: "http-8080", + ContainerPort: 8080, + HostPort: 8080, + Protocol: corev1.ProtocolTCP, + }, + { + Name: "debug", + ContainerPort: 5858, + HostPort: 20001, + Protocol: corev1.ProtocolTCP, + }, + } + container2 := pod.Spec.Containers[0].DeepCopy() + container2.Name = "tools" + container2.Ports = []corev1.ContainerPort{ + { + Name: "http-5000", + ContainerPort: 5000, + HostPort: 20002, + Protocol: corev1.ProtocolTCP, + }, + } + pod.Spec.Containers = append(pod.Spec.Containers, *container2) return pod }, wantFwPorts: []api.ForwardedPort{ { Platform: "podman", - ContainerName: "mycomponent", - PortName: "http", + ContainerName: "runtime", + PortName: "http-8080", LocalAddress: "127.0.0.1", LocalPort: 8080, ContainerPort: 8080, @@ -1004,13 +1033,22 @@ func Test_createPodFromComponent(t *testing.T) { }, { Platform: "podman", - ContainerName: "mycomponent", + ContainerName: "runtime", PortName: "debug", LocalAddress: "127.0.0.1", LocalPort: 20001, ContainerPort: 5858, IsDebug: true, }, + { + Platform: "podman", + ContainerName: "tools", + PortName: "http-5000", + LocalAddress: "127.0.0.1", + LocalPort: 20002, + ContainerPort: 5000, + IsDebug: false, + }, }, }, { @@ -1020,15 +1058,28 @@ func Test_createPodFromComponent(t *testing.T) { data, _ := data.NewDevfileData(string(data.APISchemaVersion200)) _ = data.AddCommands([]v1alpha2.Command{command}) cmp := baseComponent.DeepCopy() - cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ - Name: "http", - TargetPort: 8080, - }) - cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ - Name: "debug", - TargetPort: 5858, - }) - _ = data.AddComponents([]v1alpha2.Component{*cmp}) + cmp.Name = "runtime" + cmp.Container.Endpoints = []v1alpha2.Endpoint{ + { + Name: "http-8080", + + TargetPort: 8080, + }, + { + Name: "debug", + TargetPort: 5858, + }, + } + cmp2 := baseComponent.DeepCopy() + cmp2.Name = "tools" + cmp2.Container.Endpoints = []v1alpha2.Endpoint{ + { + Name: "http-5000", + TargetPort: 5000, + }, + } + + _ = data.AddComponents([]v1alpha2.Component{*cmp, *cmp2}) return parser.DevfileObj{ Data: data, } @@ -1038,33 +1089,49 @@ func Test_createPodFromComponent(t *testing.T) { debug: true, customForwardedPorts: []api.ForwardedPort{ { - ContainerName: "mycomponent", LocalPort: 8080, ContainerPort: 8080, }, + { + LocalPort: 5000, + ContainerPort: 5000, + }, }, }, wantPod: func(basePod *corev1.Pod) *corev1.Pod { pod := basePod.DeepCopy() - pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ - Name: "http", - ContainerPort: 8080, - HostPort: 8080, - Protocol: corev1.ProtocolTCP, - }) - pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ - Name: "debug", - ContainerPort: 5858, - HostPort: 20001, - Protocol: corev1.ProtocolTCP, - }) + pod.Spec.Containers[0].Name = "runtime" + pod.Spec.Containers[0].Ports = []corev1.ContainerPort{ + { + Name: "http-8080", + ContainerPort: 8080, + HostPort: 8080, + Protocol: corev1.ProtocolTCP, + }, { + Name: "debug", + ContainerPort: 5858, + HostPort: 20001, + Protocol: corev1.ProtocolTCP, + }, + } + container2 := pod.Spec.Containers[0].DeepCopy() + container2.Name = "tools" + container2.Ports = []corev1.ContainerPort{ + { + Name: "http-5000", + ContainerPort: 5000, + HostPort: 5000, + Protocol: corev1.ProtocolTCP, + }, + } + pod.Spec.Containers = append(pod.Spec.Containers, *container2) return pod }, wantFwPorts: []api.ForwardedPort{ { Platform: "podman", - ContainerName: "mycomponent", - PortName: "http", + ContainerName: "runtime", + PortName: "http-8080", LocalAddress: "127.0.0.1", LocalPort: 8080, ContainerPort: 8080, @@ -1072,13 +1139,22 @@ func Test_createPodFromComponent(t *testing.T) { }, { Platform: "podman", - ContainerName: "mycomponent", + ContainerName: "runtime", PortName: "debug", LocalAddress: "127.0.0.1", LocalPort: 20001, ContainerPort: 5858, IsDebug: true, }, + { + Platform: "podman", + ContainerName: "tools", + PortName: "http-5000", + LocalAddress: "127.0.0.1", + LocalPort: 5000, + ContainerPort: 5000, + IsDebug: false, + }, }, }, { @@ -1088,15 +1164,32 @@ func Test_createPodFromComponent(t *testing.T) { data, _ := data.NewDevfileData(string(data.APISchemaVersion200)) _ = data.AddCommands([]v1alpha2.Command{command}) cmp := baseComponent.DeepCopy() - cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ - Name: "http", - TargetPort: 8080, - }) - cmp.Container.Endpoints = append(cmp.Container.Endpoints, v1alpha2.Endpoint{ - Name: "debug", - TargetPort: 5858, - }) - _ = data.AddComponents([]v1alpha2.Component{*cmp}) + cmp.Name = "runtime" + cmp.Container.Endpoints = []v1alpha2.Endpoint{ + { + Name: "http-8080", + TargetPort: 8080, + }, + { + Name: "debug", + TargetPort: 5858, + }, + } + + cmp2 := baseComponent.DeepCopy() + cmp2.Name = "tools" + cmp2.Container.Endpoints = []v1alpha2.Endpoint{ + { + Name: "http-9000", + TargetPort: 9000, + }, + { + Name: "http-5000", + TargetPort: 5000, + }, + } + + _ = data.AddComponents([]v1alpha2.Component{*cmp, *cmp2}) return parser.DevfileObj{ Data: data, } @@ -1106,33 +1199,60 @@ func Test_createPodFromComponent(t *testing.T) { debug: true, customForwardedPorts: []api.ForwardedPort{ { - ContainerName: "mycomponent", LocalPort: 20001, ContainerPort: 8080, }, + { + LocalPort: 20002, + ContainerPort: 9000, + }, + { + LocalPort: 5000, + ContainerPort: 5000, + }, }, }, wantPod: func(basePod *corev1.Pod) *corev1.Pod { pod := basePod.DeepCopy() - pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ - Name: "http", - ContainerPort: 8080, - HostPort: 20001, - Protocol: corev1.ProtocolTCP, - }) - pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{ - Name: "debug", - ContainerPort: 5858, - HostPort: 20002, - Protocol: corev1.ProtocolTCP, - }) + pod.Spec.Containers[0].Name = "runtime" + pod.Spec.Containers[0].Ports = []corev1.ContainerPort{ + { + Name: "http-8080", + ContainerPort: 8080, + HostPort: 20001, + Protocol: corev1.ProtocolTCP, + }, + { + Name: "debug", + ContainerPort: 5858, + HostPort: 20003, + Protocol: corev1.ProtocolTCP, + }, + } + container2 := pod.Spec.Containers[0].DeepCopy() + container2.Name = "tools" + container2.Ports = []corev1.ContainerPort{ + { + Name: "http-9000", + ContainerPort: 9000, + HostPort: 20002, + Protocol: corev1.ProtocolTCP, + }, + { + Name: "http-5000", + ContainerPort: 5000, + HostPort: 5000, + Protocol: corev1.ProtocolTCP, + }, + } + pod.Spec.Containers = append(pod.Spec.Containers, *container2) return pod }, wantFwPorts: []api.ForwardedPort{ { Platform: "podman", - ContainerName: "mycomponent", - PortName: "http", + ContainerName: "runtime", + PortName: "http-8080", LocalAddress: "127.0.0.1", LocalPort: 20001, ContainerPort: 8080, @@ -1140,13 +1260,31 @@ func Test_createPodFromComponent(t *testing.T) { }, { Platform: "podman", - ContainerName: "mycomponent", + ContainerName: "runtime", PortName: "debug", LocalAddress: "127.0.0.1", - LocalPort: 20002, + LocalPort: 20003, ContainerPort: 5858, IsDebug: true, }, + { + Platform: "podman", + ContainerName: "tools", + PortName: "http-9000", + LocalAddress: "127.0.0.1", + LocalPort: 20002, + ContainerPort: 9000, + IsDebug: false, + }, + { + Platform: "podman", + ContainerName: "tools", + PortName: "http-5000", + LocalAddress: "127.0.0.1", + LocalPort: 5000, + ContainerPort: 5000, + IsDebug: false, + }, }, }, } @@ -1174,194 +1312,9 @@ func Test_createPodFromComponent(t *testing.T) { if diff := cmp.Diff(tt.wantPod(basePod), got, cmpopts.EquateEmpty()); diff != "" { t.Errorf("createPodFromComponent() pod mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff(tt.wantFwPorts, gotFwPorts, cmpopts.EquateEmpty()); diff != "" { - t.Errorf("createPodFromComponent() fwPorts mismatch (-want +got):\n%s", diff) - } - }) - } -} -func Test_getCustomPortPairs(t *testing.T) { - type args struct { - definedPorts []api.ForwardedPort - ceMapping map[string][]v1alpha2.Endpoint - usedPorts []int - } - tests := []struct { - name string - args args - want []api.ForwardedPort - }{ - // TODO: Add test cases. - { - name: "ports are provided with container name", - args: args{ - definedPorts: []api.ForwardedPort{ - {ContainerName: "runtime", LocalPort: 8080, ContainerPort: 8000}, - }, - ceMapping: map[string][]v1alpha2.Endpoint{ - "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, - "tools": {{TargetPort: 5000}}, - }, - usedPorts: nil, - }, - want: []api.ForwardedPort{ - { - Platform: "podman", - ContainerName: "runtime", - LocalAddress: "127.0.0.1", - LocalPort: 8080, - ContainerPort: 8000, - }, - { - Platform: "podman", - ContainerName: "runtime", - LocalPort: 20001, - LocalAddress: "127.0.0.1", - ContainerPort: 9000, - }, - { - Platform: "podman", - ContainerName: "tools", - LocalAddress: "127.0.0.1", - LocalPort: 20002, - ContainerPort: 5000, - }, - }, - }, - { - name: "ports are provided without container name", - args: args{ - definedPorts: []api.ForwardedPort{ - {LocalPort: 8080, ContainerPort: 8000}, - {LocalPort: 5000, ContainerPort: 5000}, - }, - ceMapping: map[string][]v1alpha2.Endpoint{ - "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, - "tools": {{TargetPort: 5000}}, - }, - usedPorts: nil, - }, - want: []api.ForwardedPort{ - { - Platform: "podman", - ContainerName: "runtime", - LocalAddress: "127.0.0.1", - LocalPort: 8080, - ContainerPort: 8000, - }, - { - Platform: "podman", - ContainerName: "runtime", - LocalPort: 20001, - LocalAddress: "127.0.0.1", - ContainerPort: 9000, - }, - { - Platform: "podman", - ContainerName: "tools", - LocalAddress: "127.0.0.1", - LocalPort: 5000, - ContainerPort: 5000, - }, - }, - }, - - { - name: "custom local ports is same as one of the random ranged [20001-30001] ports", - args: args{ - definedPorts: []api.ForwardedPort{ - {LocalPort: 20001, ContainerPort: 8000}, - {LocalPort: 20002, ContainerPort: 9000}, - {LocalPort: 5000, ContainerPort: 5000}, - }, - ceMapping: map[string][]v1alpha2.Endpoint{ - "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, - "tools": {{TargetPort: 5000}, {TargetPort: 8080}}, - }, - usedPorts: nil, - }, - want: []api.ForwardedPort{ - { - Platform: "podman", - ContainerName: "runtime", - LocalAddress: "127.0.0.1", - LocalPort: 20001, - ContainerPort: 8000, - }, - { - Platform: "podman", - ContainerName: "runtime", - LocalPort: 20002, - LocalAddress: "127.0.0.1", - ContainerPort: 9000, - }, - { - Platform: "podman", - ContainerName: "tools", - LocalAddress: "127.0.0.1", - LocalPort: 5000, - ContainerPort: 5000, - }, - { - Platform: "podman", - ContainerName: "tools", - LocalAddress: "127.0.0.1", - LocalPort: 20003, - ContainerPort: 8080, - }, - }, - }, - { - name: "should reuse a port if it is defined in usedPorts", - args: args{ - definedPorts: []api.ForwardedPort{ - {LocalPort: 20001, ContainerPort: 8000}, - {LocalPort: 5000, ContainerPort: 5000}, - }, - ceMapping: map[string][]v1alpha2.Endpoint{ - "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, - "tools": {{TargetPort: 5000}, {TargetPort: 8080}}, - }, - usedPorts: []int{20002, 20003}, - }, - want: []api.ForwardedPort{ - { - Platform: "podman", - ContainerName: "runtime", - LocalAddress: "127.0.0.1", - LocalPort: 20001, - ContainerPort: 8000, - }, - { - Platform: "podman", - ContainerName: "runtime", - LocalPort: 20002, - LocalAddress: "127.0.0.1", - ContainerPort: 9000, - }, - { - Platform: "podman", - ContainerName: "tools", - LocalAddress: "127.0.0.1", - LocalPort: 5000, - ContainerPort: 5000, - }, - { - Platform: "podman", - ContainerName: "tools", - LocalAddress: "127.0.0.1", - LocalPort: 20003, - ContainerPort: 8000, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := getCustomPortPairs(tt.args.definedPorts, tt.args.ceMapping, tt.args.usedPorts) - if diff := cmp.Diff(got, tt.want); diff != "" { - t.Errorf("getCustomPortPairs() = %v", diff) + if diff := cmp.Diff(tt.wantFwPorts, gotFwPorts, cmpopts.EquateEmpty(), cmpopts.SortSlices(func(x, y api.ForwardedPort) bool { return x.ContainerName < y.ContainerName })); diff != "" { + t.Errorf("createPodFromComponent() fwPorts mismatch (-want +got):\n%s", diff) } }) } From 486c0a096252f03f96d2799dd9d1f9fd9f1d2260 Mon Sep 17 00:00:00 2001 From: Parthvi Vala Date: Wed, 12 Apr 2023 17:28:00 +0530 Subject: [PATCH 7/8] Refactor integration test Signed-off-by: Parthvi Vala Add dev --debug integration test for podman Signed-off-by: Parthvi Vala Add dev --debug integration test for podman Signed-off-by: Parthvi Vala Fix ci failures Signed-off-by: Parthvi Vala --- pkg/odo/cli/dev/dev.go | 13 +-- tests/integration/cmd_dev_debug_test.go | 85 +++++++++-------- tests/integration/cmd_dev_test.go | 116 +++++++++++------------- 3 files changed, 109 insertions(+), 105 deletions(-) diff --git a/pkg/odo/cli/dev/dev.go b/pkg/odo/cli/dev/dev.go index e6135627552..a9f34f703ca 100644 --- a/pkg/odo/cli/dev/dev.go +++ b/pkg/odo/cli/dev/dev.go @@ -4,16 +4,18 @@ import ( "context" "errors" "fmt" - "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/redhat-developer/odo/pkg/api" "io" - "k8s.io/klog" "path/filepath" "regexp" "sort" "strconv" "strings" + "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "k8s.io/klog" + + "github.com/redhat-developer/odo/pkg/api" + "github.com/spf13/cobra" ktemplates "k8s.io/kubectl/pkg/util/templates" @@ -200,11 +202,6 @@ func (o *DevOptions) Run(ctx context.Context) (err error) { genericclioptions.WarnIfDefaultNamespace(odocontext.GetNamespace(ctx), o.clientset.KubernetesClient) } - // TODO: Remove this once --port-forward has been implemented for podman. - if platform == commonflags.PlatformPodman && len(o.portForwardFlag) != 0 { - fmt.Println() - log.Warning("--port-forward flag has not been implemented for podman yet, we will use the normal port forwarding") - } // check for .gitignore file and add odo-file-index.json to .gitignore. // In case the .gitignore was created by odo, it is purposely not reported as candidate for deletion (via a call to files.ReportLocalFileGeneratedByOdo) // because a .gitignore file is more likely to be modified by the user afterward (for another usage). diff --git a/tests/integration/cmd_dev_debug_test.go b/tests/integration/cmd_dev_debug_test.go index ea9da402b7d..419e7fed991 100644 --- a/tests/integration/cmd_dev_debug_test.go +++ b/tests/integration/cmd_dev_debug_test.go @@ -2,10 +2,11 @@ package integration import ( "fmt" - "k8s.io/utils/pointer" "path/filepath" "strings" + "k8s.io/utils/pointer" + "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/redhat-developer/odo/pkg/labels" @@ -40,47 +41,59 @@ var _ = Describe("odo dev debug command tests", func() { helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-with-debugrun.yaml")).ShouldPass() Expect(helper.VerifyFileExists(".odo/env/env.yaml")).To(BeFalse()) }) - if !podman { - // TODO(pvala): Refactor this when custom port-mapping for port forwarding has been implemented for podman - When("running odo dev with debug flag and custom port mapping for port forwarding", func() { - var devSession helper.DevSession - var ports map[string]string - var ( - LocalPort = helper.GetRandomFreePort() - LocalDebugPort = helper.GetRandomFreePort() - ) - const ( - ContainerPort = "3000" - ContainerDebugPort = "5858" - ) - BeforeEach(func() { - opts := []string{"--debug", fmt.Sprintf("--port-forward=%s:%s", LocalPort, ContainerPort), fmt.Sprintf("--port-forward=%s:%s", LocalDebugPort, ContainerDebugPort)} - var err error - devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ - CmdlineArgs: opts, - NoRandomPorts: true, - }) - Expect(err).ToNot(HaveOccurred()) + When("running odo dev with debug flag and custom port mapping for port forwarding", helper.LabelPodmanIf(podman, func() { + var ( + devSession helper.DevSession + ports map[string]string + ) + var ( + LocalPort = helper.GetRandomFreePort() + LocalDebugPort = helper.GetRandomFreePort() + ) + const ( + ContainerPort = "3000" + ContainerDebugPort = "5858" + ) + + BeforeEach(func() { + opts := []string{"--debug", fmt.Sprintf("--port-forward=%s:%s", LocalPort, ContainerPort), fmt.Sprintf("--port-forward=%s:%s", LocalDebugPort, ContainerDebugPort)} + if podman { + // Debugging works with podman if we use --forward-localhost, but that will not use custom port-mapping; + // so we use --ignore-localhost to simply test if custom ports are still taken into account + opts = append(opts, "--ignore-localhost") + } + var err error + devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ + CmdlineArgs: opts, + NoRandomPorts: true, + RunOnPodman: podman, }) + Expect(err).ToNot(HaveOccurred()) + }) - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() + AfterEach(func() { + devSession.Stop() + devSession.WaitEnd() + }) + + It("should connect to relevant custom ports forwarded", func() { + By("connecting to the application port", func() { + helper.HttpWaitForWithStatus("http://"+ports[ContainerPort], "Hello from Node.js Starter Application!", 12, 5, 200) }) - It("should connect to relevant custom ports forwarded", func() { - By("connecting to the application port", func() { - helper.HttpWaitForWithStatus("http://"+ports[ContainerPort], "Hello from Node.js Starter Application!", 12, 5, 200) - }) - By("expecting a ws connection when tried to connect on default debug port locally", func() { - // 400 response expected because the endpoint expects a websocket request and we are doing a HTTP GET - // We are just using this to validate if nodejs agent is listening on the other side - url := fmt.Sprintf("http://%s", ports[ContainerDebugPort]) - Expect(url).To(ContainSubstring(LocalDebugPort)) + By("expecting a ws connection when tried to connect on default debug port locally", func() { + // 400 response expected because the endpoint expects a websocket request and we are doing a HTTP GET + // We are just using this to validate if nodejs agent is listening on the other side + url := fmt.Sprintf("http://%s", ports[ContainerDebugPort]) + Expect(url).To(ContainSubstring(LocalDebugPort)) + if !podman { + // this check doesn't work for podman unless we use --forward-localhost, + // but using it beats the purpose of testing with custom port mapping, so we skip this check helper.HttpWaitForWithStatus(url, "WebSockets request was expected", 12, 5, 400) - }) + } }) }) - } + })) + When("running odo dev with debug flag", helper.LabelPodmanIf(podman, func() { var devSession helper.DevSession var ports map[string]string diff --git a/tests/integration/cmd_dev_test.go b/tests/integration/cmd_dev_test.go index 06f370691ea..737f2837dba 100644 --- a/tests/integration/cmd_dev_test.go +++ b/tests/integration/cmd_dev_test.go @@ -131,6 +131,15 @@ var _ = Describe("odo dev command tests", func() { }) Expect(err).ToNot(HaveOccurred()) })) + + It("should fail when using --random-ports and --port-forward together", helper.LabelPodmanIf(podman, func() { + args := []string{"dev", "--random-ports", "--port-forward=8000:3000"} + if podman { + args = append(args, "--platform", "podman") + } + errOut := helper.Cmd("odo", args...).ShouldFail().Err() + Expect(errOut).To(ContainSubstring("--random-ports and --port-forward cannot be used together")) + })) } It("ensure that index information is updated", func() { @@ -712,69 +721,52 @@ ComponentSettings: }) } - for _, manual := range []bool{true, false} { - for _, customPortForwarding := range []bool{true, false} { - for _, podman := range []bool{true, false} { + for _, podman := range []bool{true, false} { + podman := podman + Context("port-forwarding for the component", helper.LabelPodmanIf(podman, func() { + for _, manual := range []bool{true, false} { manual := manual - customPortForwarding := customPortForwarding - podman := podman - Context("port-forwarding for the component", helper.LabelPodmanIf(podman, func() { - var NoRandomPorts bool - if customPortForwarding { - NoRandomPorts = true - } - When("a component is bootstrapped", func() { - BeforeEach(func() { - helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass() - }) - if customPortForwarding { - It("should fail when using --random-ports and --port-forward together", func() { - args := []string{"dev", "--random-ports", "--port-forward=8000:3000"} - if podman { - args = append(args, "--platform", "podman") - } - errOut := helper.Cmd("odo", args...).ShouldFail().Err() - Expect(errOut).To(ContainSubstring("--random-ports and --port-forward cannot be used together")) - }) + When("devfile has no endpoint", func() { + BeforeEach(func() { + if !podman { + helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() } + helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) + helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-no-endpoint.yaml")).ShouldPass() }) - if !customPortForwarding { - When("devfile has no endpoint", func() { - BeforeEach(func() { - if !podman { - helper.Cmd("odo", "set", "project", commonVar.Project).ShouldPass() - } - helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-no-endpoint.yaml")).ShouldPass() - }) - When("running odo dev", func() { - var devSession helper.DevSession - var ports map[string]string - BeforeEach(func() { - var err error - opts := []string{} - if manual { - opts = append(opts, "--no-watch") - } - devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ - CmdlineArgs: opts, - RunOnPodman: podman, - }) - Expect(err).ToNot(HaveOccurred()) - }) + When("running odo dev", func() { + var devSession helper.DevSession + var ports map[string]string + BeforeEach(func() { + var err error + opts := []string{} + if manual { + opts = append(opts, "--no-watch") + } + devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ + CmdlineArgs: opts, + RunOnPodman: podman, + }) + Expect(err).ToNot(HaveOccurred()) + }) - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() - }) + AfterEach(func() { + devSession.Stop() + devSession.WaitEnd() + }) - It("should have no endpoint forwarded", func() { - Expect(ports).To(BeEmpty()) - }) - }) + It(fmt.Sprintf("should have no endpoint forwarded (podman=%v, manual=%v)", podman, manual), func() { + Expect(ports).To(BeEmpty()) }) + }) + }) + + for _, customPortForwarding := range []bool{true, false} { + customPortForwarding := customPortForwarding + var NoRandomPorts bool + if customPortForwarding { + NoRandomPorts = true } When("devfile has single endpoint", func() { var ( @@ -813,7 +805,7 @@ ComponentSettings: devSession.WaitEnd() }) - It("should expose the endpoint on localhost", func() { + It(fmt.Sprintf("should expose the endpoint on localhost (podman=%v, manual=%v, customPortForwarding=%v)", podman, manual, customPortForwarding), func() { url := fmt.Sprintf("http://%s", ports[ContainerPort]) if customPortForwarding { Expect(url).To(ContainSubstring(LocalPort)) @@ -850,7 +842,7 @@ ComponentSettings: stderr = string(stderrBytes) }) - It("should react on the Devfile modification", func() { + It(fmt.Sprintf("should react on the Devfile modification (podman=%v, manual=%v, customPortForwarding=%v)", podman, manual, customPortForwarding), func() { if podman { By("warning users that odo dev needs to be restarted", func() { Expect(stdout).To(ContainSubstring( @@ -937,7 +929,7 @@ ComponentSettings: devSession.WaitEnd() }) - It("should expose all endpoints on localhost regardless of exposure", func() { + It(fmt.Sprintf("should expose all endpoints on localhost regardless of exposure(podman=%v, manual=%v, customPortForwarding=%v)", podman, manual, customPortForwarding), func() { By("not exposing debug endpoints", func() { for _, p := range []int{5005, 5006} { _, found := ports[strconv.Itoa(p)] @@ -1012,10 +1004,12 @@ ComponentSettings: }) }) - })) + + } } - } + })) } + for _, devfileHandlerCtx := range []struct { name string sourceHandler func(path string, originalCmpName string) From e48e03cf2e16956641f38cccaa55dab0cd44bb31 Mon Sep 17 00:00:00 2001 From: Parthvi Vala Date: Mon, 17 Apr 2023 20:53:30 +0530 Subject: [PATCH 8/8] Use order to iterate over ceMapping and fix a check with debug integration test Signed-off-by: Parthvi Vala --- pkg/dev/podmandev/pod.go | 14 ++++++++++++-- tests/integration/cmd_dev_debug_test.go | 11 +++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/pkg/dev/podmandev/pod.go b/pkg/dev/podmandev/pod.go index 922618d14dc..d2e4efe8f7a 100644 --- a/pkg/dev/podmandev/pod.go +++ b/pkg/dev/podmandev/pod.go @@ -3,6 +3,7 @@ package podmandev import ( "fmt" "math/rand" // #nosec + "sort" "time" "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" @@ -56,7 +57,6 @@ func createPodFromComponent( if err != nil { return nil, nil, err } - // } utils.AddOdoProjectVolume(&containers) utils.AddOdoMandatoryVolume(&containers) @@ -229,7 +229,17 @@ func getPortMapping(devfileObj parser.DevfileObj, debug bool, randomPorts bool, endPort := startPort + 10000 usedPortsCopy := make([]int, len(usedPorts)) copy(usedPortsCopy, usedPorts) - for containerName, endpoints := range ceMapping { + + // Prepare to iterate over the ceMapping in an orderly fashion + // This ensures we iterate over the ceMapping in the same way every time, obtain the same result every time and avoid any flakes with tests + var containers []string + for container := range ceMapping { + containers = append(containers, container) + } + sort.Strings(containers) + + for _, containerName := range containers { + endpoints := ceMapping[containerName] epLoop: for _, ep := range endpoints { portName := ep.Name diff --git a/tests/integration/cmd_dev_debug_test.go b/tests/integration/cmd_dev_debug_test.go index 419e7fed991..859ca5ae628 100644 --- a/tests/integration/cmd_dev_debug_test.go +++ b/tests/integration/cmd_dev_debug_test.go @@ -58,9 +58,7 @@ var _ = Describe("odo dev debug command tests", func() { BeforeEach(func() { opts := []string{"--debug", fmt.Sprintf("--port-forward=%s:%s", LocalPort, ContainerPort), fmt.Sprintf("--port-forward=%s:%s", LocalDebugPort, ContainerDebugPort)} if podman { - // Debugging works with podman if we use --forward-localhost, but that will not use custom port-mapping; - // so we use --ignore-localhost to simply test if custom ports are still taken into account - opts = append(opts, "--ignore-localhost") + opts = append(opts, "--forward-localhost") } var err error devSession, _, _, ports, err = helper.StartDevMode(helper.DevSessionOpts{ @@ -85,11 +83,8 @@ var _ = Describe("odo dev debug command tests", func() { // We are just using this to validate if nodejs agent is listening on the other side url := fmt.Sprintf("http://%s", ports[ContainerDebugPort]) Expect(url).To(ContainSubstring(LocalDebugPort)) - if !podman { - // this check doesn't work for podman unless we use --forward-localhost, - // but using it beats the purpose of testing with custom port mapping, so we skip this check - helper.HttpWaitForWithStatus(url, "WebSockets request was expected", 12, 5, 400) - } + + helper.HttpWaitForWithStatus(url, "WebSockets request was expected", 12, 5, 400) }) }) }))