Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Port-forward Debug endpoints only when running odo dev with --debug #6505

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions pkg/dev/podmandev/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/redhat-developer/odo/pkg/component"
"github.com/redhat-developer/odo/pkg/devfile/adapters/kubernetes/utils"
"github.com/redhat-developer/odo/pkg/labels"
"github.com/redhat-developer/odo/pkg/libdevfile"
"github.com/redhat-developer/odo/pkg/odo/commonflags"
"github.com/redhat-developer/odo/pkg/storage"
"github.com/redhat-developer/odo/pkg/util"
Expand All @@ -23,6 +24,7 @@ func createPodFromComponent(
devfileObj parser.DevfileObj,
componentName string,
appName string,
debug bool,
buildCommand string,
runCommand string,
debugCommand string,
Expand All @@ -43,7 +45,7 @@ func createPodFromComponent(
utils.AddOdoProjectVolume(&containers)
utils.AddOdoMandatoryVolume(&containers)

fwPorts := addHostPorts(containers, usedPorts)
fwPorts := addHostPorts(containers, debug, usedPorts)

volumes := []corev1.Volume{
{
Expand Down Expand Up @@ -109,12 +111,19 @@ func getVolumeName(volume string, componentName string, appName string) string {
return volume + "-" + componentName + "-" + appName
}

func addHostPorts(containers []corev1.Container, usedPorts []int) []api.ForwardedPort {
func addHostPorts(containers []corev1.Container, debug bool, usedPorts []int) []api.ForwardedPort {
var result []api.ForwardedPort
startPort := 40001
endPort := startPort + 10000
for i := range containers {
for j := range containers[i].Ports {
var ports []corev1.ContainerPort
for _, port := range containers[i].Ports {
portName := port.Name
if !debug && libdevfile.IsDebugPort(portName) {
klog.V(4).Infof("not running in Debug mode, so skipping container Debug port: %v:%v:%v",
containers[i].Name, portName, port.ContainerPort)
continue
}
freePort, err := util.NextFreePort(startPort, endPort, usedPorts)
if err != nil {
klog.Infof("%s", err)
Expand All @@ -125,11 +134,13 @@ func addHostPorts(containers []corev1.Container, usedPorts []int) []api.Forwarde
ContainerName: containers[i].Name,
LocalAddress: "127.0.0.1",
LocalPort: freePort,
ContainerPort: int(containers[i].Ports[j].ContainerPort),
ContainerPort: int(port.ContainerPort),
})
containers[i].Ports[j].HostPort = int32(freePort)
port.HostPort = int32(freePort)
ports = append(ports, port)
startPort = freePort + 1
}
containers[i].Ports = ports
}
return result
}
Expand Down
58 changes: 56 additions & 2 deletions pkg/dev/podmandev/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func Test_createPodFromComponent(t *testing.T) {
devfileObj func() parser.DevfileObj
componentName string
appName string
debug bool
buildCommand string
runCommand string
debugCommand string
Expand Down Expand Up @@ -237,7 +238,7 @@ func Test_createPodFromComponent(t *testing.T) {
},
},
{
name: "basic component + application endpoint + debug endpoint",
name: "basic component + application endpoint + debug endpoint - without debug",
args: args{
devfileObj: func() parser.DevfileObj {
data, _ := data.NewDevfileData(string(data.APISchemaVersion200))
Expand All @@ -259,6 +260,50 @@ func Test_createPodFromComponent(t *testing.T) {
componentName: devfileName,
appName: appName,
},
wantPod: func() *corev1.Pod {
pod := basePod.DeepCopy()
pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{
Name: "http",
ContainerPort: 8080,
Protocol: "TCP",
HostPort: 40001,
})
return pod
},
wantFwPorts: []api.ForwardedPort{
{
Platform: "podman",
ContainerName: "mycomponent",
LocalAddress: "127.0.0.1",
LocalPort: 40001,
ContainerPort: 8080,
},
},
},
{
name: "basic component + application endpoint + debug endpoint - with debug",
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,
},
wantPod: func() *corev1.Pod {
pod := basePod.DeepCopy()
pod.Spec.Containers[0].Ports = append(pod.Spec.Containers[0].Ports, corev1.ContainerPort{
Expand Down Expand Up @@ -335,7 +380,16 @@ func Test_createPodFromComponent(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, gotFwPorts, err := createPodFromComponent(tt.args.devfileObj(), tt.args.componentName, tt.args.appName, tt.args.buildCommand, tt.args.runCommand, tt.args.debugCommand, []int{40001, 40002})
got, gotFwPorts, err := createPodFromComponent(
tt.args.devfileObj(),
tt.args.componentName,
tt.args.appName,
tt.args.debug,
tt.args.buildCommand,
tt.args.runCommand,
tt.args.debugCommand,
[]int{40001, 40002},
)
if (err != nil) != tt.wantErr {
t.Errorf("createPodFromComponent() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
1 change: 1 addition & 0 deletions pkg/dev/podmandev/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions) (*c
*devfileObj,
componentName,
appName,
options.Debug,
options.BuildCommand,
options.RunCommand,
"",
Expand Down
4 changes: 2 additions & 2 deletions pkg/devfile/adapters/kubernetes/component/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (a Adapter) Push(ctx context.Context, parameters adapters.PushParameters, c
}

// Check if endpoints changed in Devfile
portsToForward, err := a.portForwardClient.GetPortsToForward(a.Devfile)
portsToForward, err := a.portForwardClient.GetPortsToForward(a.Devfile, parameters.Debug)
if err != nil {
return err
}
Expand Down Expand Up @@ -342,7 +342,7 @@ func (a Adapter) Push(ctx context.Context, parameters adapters.PushParameters, c
a.portForwardClient.StopPortForwarding()
}

err = a.portForwardClient.StartPortForwarding(a.Devfile, a.ComponentName, parameters.RandomPorts, parameters.ErrOut)
err = a.portForwardClient.StartPortForwarding(a.Devfile, a.ComponentName, parameters.Debug, parameters.RandomPorts, parameters.ErrOut)
if err != nil {
return adapters.NewErrPortForward(err)
}
Expand Down
23 changes: 12 additions & 11 deletions pkg/libdevfile/libdevfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,25 +277,26 @@ func execDevfileEvent(devfileObj parser.DevfileObj, events []string, handler Han
}

// GetContainerEndpointMapping returns a map of container names and slice of its endpoints (in int)
func GetContainerEndpointMapping(containers []v1alpha2.Component) map[string][]int {
// Debug ports will be included only if includeDebug is true.
func GetContainerEndpointMapping(containers []v1alpha2.Component, includeDebug bool) map[string][]int {
ceMapping := make(map[string][]int)
for _, container := range containers {
if container.ComponentUnion.Container == nil {
// this is not a container component; continue prevents panic when accessing Endpoints field
continue
}
endpoints := container.Container.Endpoints
if len(endpoints) == 0 {
continue
}

k := container.Name
if _, ok := ceMapping[k]; !ok {
ceMapping[k] = []int{}
var ports []int
for _, e := range container.Container.Endpoints {
if !includeDebug && IsDebugEndpoint(e) {
klog.V(4).Infof("not running in Debug mode, so ignored Debug port for container %v:%v:%v",
container.Name, e.Name, e.TargetPort)
continue
}
ports = append(ports, e.TargetPort)
}

for _, e := range endpoints {
ceMapping[k] = append(ceMapping[k], e.TargetPort)
if len(ports) != 0 {
ceMapping[container.Name] = ports
}
}
return ceMapping
Expand Down
126 changes: 113 additions & 13 deletions pkg/libdevfile/libdevfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,8 @@ func TestBuild(t *testing.T) {

func TestGetContainerEndpointMapping(t *testing.T) {
type args struct {
containers []v1alpha2.Component
containers []v1alpha2.Component
includeDebug bool
}

imageComponent := generator.GetImageComponent(generator.ImageComponentParams{
Expand Down Expand Up @@ -626,7 +627,7 @@ func TestGetContainerEndpointMapping(t *testing.T) {
},
})

containerWithOneNoneInternalEndpoint := generator.GetContainerComponent(generator.ContainerComponentParams{
containerWithOneNoneDebugEndpoint := generator.GetContainerComponent(generator.ContainerComponentParams{
Name: "container-none-endpoint",
Endpoints: []v1alpha2.Endpoint{
{
Expand All @@ -636,6 +637,64 @@ func TestGetContainerEndpointMapping(t *testing.T) {
},
},
})
multiportContainer1 := generator.GetContainerComponent(generator.ContainerComponentParams{
Name: "container-multiple-endpoints-1",
Endpoints: []v1alpha2.Endpoint{
{
Name: "http-3000",
TargetPort: 3000,
Exposure: v1alpha2.PublicEndpointExposure,
},
{
Name: "http-8888",
TargetPort: 8888,
Exposure: v1alpha2.InternalEndpointExposure,
},
{
Name: "debug",
TargetPort: 5005,
Exposure: v1alpha2.NoneEndpointExposure,
},
{
Name: "debug-udp",
TargetPort: 15005,
Exposure: v1alpha2.NoneEndpointExposure,
Protocol: v1alpha2.UDPEndpointProtocol,
},
{
Name: "debug-ws",
TargetPort: 25005,
Exposure: v1alpha2.NoneEndpointExposure,
Protocol: v1alpha2.WSEndpointProtocol,
},
},
})
multiportContainer2 := generator.GetContainerComponent(generator.ContainerComponentParams{
Name: "container-multiple-endpoints-2",
Endpoints: []v1alpha2.Endpoint{
{
Name: "http-8080",
TargetPort: 8080,
Exposure: v1alpha2.PublicEndpointExposure,
},
{
Name: "http-9100",
TargetPort: 9100,
Exposure: v1alpha2.InternalEndpointExposure,
},
{
Name: "debug",
TargetPort: 18080,
Exposure: v1alpha2.NoneEndpointExposure,
},
{
Name: "debug-udp",
TargetPort: 19100,
Exposure: v1alpha2.NoneEndpointExposure,
Protocol: v1alpha2.UDPEndpointProtocol,
},
},
})

tests := []struct {
name string
Expand All @@ -657,42 +716,83 @@ func TestGetContainerEndpointMapping(t *testing.T) {
want: map[string][]int{},
},
{
name: "multiple containers with varying types of endpoints",
name: "multiple containers with varying types of endpoints - without debug",
args: args{
containers: []v1alpha2.Component{
containerWithNoEndpoints,
containerWithOnePublicEndpoint,
containerWithOneInternalEndpoint,
containerWithOneNoneInternalEndpoint,
containerWithOneNoneDebugEndpoint,
multiportContainer1,
multiportContainer2,
},
},
want: map[string][]int{
containerWithOnePublicEndpoint.Name: {8080},
containerWithOneInternalEndpoint.Name: {9090},
multiportContainer1.Name: {3000, 8888},
multiportContainer2.Name: {8080, 9100},
},
},
{
name: "multiple containers with varying types of endpoints - with debug",
args: args{
containers: []v1alpha2.Component{
containerWithNoEndpoints,
containerWithOnePublicEndpoint,
containerWithOneInternalEndpoint,
containerWithOneNoneDebugEndpoint,
multiportContainer1,
multiportContainer2,
},
includeDebug: true,
},
want: map[string][]int{
containerWithOnePublicEndpoint.Name: {8080},
containerWithOneInternalEndpoint.Name: {9090},
containerWithOneNoneDebugEndpoint.Name: {9099},
multiportContainer1.Name: {3000, 8888, 5005, 15005, 25005},
multiportContainer2.Name: {8080, 9100, 18080, 19100},
},
},
{
name: "invalid input - one image component with rest being containers - without debug",
args: args{
containers: []v1alpha2.Component{
containerWithNoEndpoints,
containerWithOnePublicEndpoint,
containerWithOneInternalEndpoint,
containerWithOneNoneDebugEndpoint,
imageComponent,
},
},
want: map[string][]int{
containerWithOnePublicEndpoint.Name: {8080},
containerWithOneInternalEndpoint.Name: {9090},
containerWithOneNoneInternalEndpoint.Name: {9099},
containerWithOnePublicEndpoint.Name: {8080},
containerWithOneInternalEndpoint.Name: {9090},
},
},
{
name: "invalid input - one image component with rest being containers",
name: "invalid input - one image component with rest being containers - with debug",
args: args{
containers: []v1alpha2.Component{
containerWithNoEndpoints,
containerWithOnePublicEndpoint,
containerWithOneInternalEndpoint,
containerWithOneNoneInternalEndpoint,
containerWithOneNoneDebugEndpoint,
imageComponent,
},
includeDebug: true,
},
want: map[string][]int{
containerWithOnePublicEndpoint.Name: {8080},
containerWithOneInternalEndpoint.Name: {9090},
containerWithOneNoneInternalEndpoint.Name: {9099},
containerWithOnePublicEndpoint.Name: {8080},
containerWithOneInternalEndpoint.Name: {9090},
containerWithOneNoneDebugEndpoint.Name: {9099},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := GetContainerEndpointMapping(tt.args.containers)
got := GetContainerEndpointMapping(tt.args.containers, tt.args.includeDebug)

if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("GetContainerEndpointMapping() mismatch (-want +got):\n%s", diff)
Expand Down
Loading