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

Wait until expected ports are opened in the container before starting port-forwarding #6701

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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ $ odo dev
✓ Syncing files into the container [171ms]
✓ Building your application in container (command: build) [7s]
• Executing the application (command: run) ...
✓ Waiting for the application to be ready [1s]
- Forwarding from 127.0.0.1:20001 -> 8080


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ $ odo dev
✓ Syncing files into the container [113ms]
✓ Building your application in container (command: build) [422ms]
• Executing the application (command: run) ...
✓ Waiting for the application to be ready [1s]
- Forwarding from 127.0.0.1:20001 -> 8080


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ $ odo dev
✓ Syncing files into the container [167ms]
✓ Building your application in container (command: build) [3m]
• Executing the application (command: run) ...
✓ Waiting for the application to be ready [1s]
- Forwarding from 127.0.0.1:20001 -> 8080


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ $ odo dev
✓ Syncing files into the container [193ms]
✓ Building your application in container (command: install) [5s]
• Executing the application (command: run) ...
✓ Waiting for the application to be ready [1s]
- Forwarding from 127.0.0.1:20001 -> 3000


Expand Down
19 changes: 19 additions & 0 deletions pkg/dev/podmandev/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io"
"path/filepath"
"strings"
"time"

devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/v2/pkg/devfile/parser"
Expand Down Expand Up @@ -122,6 +123,16 @@ func (o *DevClient) reconcile(
componentStatus.RunExecuted = true
}

// Check that the application is actually listening on the ports declared in the Devfile, so we are sure that port-forwarding will work
appReadySpinner := log.Spinner("Waiting for the application to be ready")
err = o.checkAppPorts(ctx, pod.Name, fwPorts)
appReadySpinner.End(err == nil)
if err != nil {
log.Warningf("Port forwarding might not work correctly: %v", err)
log.Warning("Running `odo logs --follow --platform podman` might help in identifying the problem.")
fmt.Fprintln(out)
}

// By default, Podman will not forward to container applications listening on the loopback interface.
// So we are trying to detect such cases and act accordingly.
// See https://github.com/redhat-developer/odo/issues/6510#issuecomment-1439986558
Expand Down Expand Up @@ -236,6 +247,14 @@ func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions) (*c
return pod, fwPorts, nil
}

func (o *DevClient) checkAppPorts(ctx context.Context, podName string, portsToFwd []api.ForwardedPort) error {
containerPortsMapping := make(map[string][]int)
for _, p := range portsToFwd {
containerPortsMapping[p.ContainerName] = append(containerPortsMapping[p.ContainerName], p.ContainerPort)
}
return port.CheckAppPortsListening(ctx, o.execClient, podName, containerPortsMapping, 1*time.Minute)
}

// handleLoopbackPorts tries to detect if any of the ports to forward (in fwPorts) is actually bound to the loopback interface within the specified pod.
// If that is the case, it will either return an error if options.IgnoreLocalhost is false, or no error otherwise.
//
Expand Down
22 changes: 22 additions & 0 deletions pkg/devfile/adapters/kubernetes/component/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"reflect"
"strings"
"time"

devfilefs "github.com/devfile/library/v2/pkg/testingutil/filesystem"
"golang.org/x/sync/errgroup"
Expand All @@ -25,6 +26,7 @@ import (
"github.com/redhat-developer/odo/pkg/libdevfile"
"github.com/redhat-developer/odo/pkg/log"
"github.com/redhat-developer/odo/pkg/machineoutput"
"github.com/redhat-developer/odo/pkg/port"
"github.com/redhat-developer/odo/pkg/portForward"
"github.com/redhat-developer/odo/pkg/preference"
"github.com/redhat-developer/odo/pkg/service"
Expand Down Expand Up @@ -350,6 +352,16 @@ func (a Adapter) Push(ctx context.Context, parameters adapters.PushParameters, c
a.portForwardClient.StopPortForwarding(a.ComponentName)
}

// Check that the application is actually listening on the ports declared in the Devfile, so we are sure that port-forwarding will work
appReadySpinner := log.Spinner("Waiting for the application to be ready")
err = a.checkAppPorts(ctx, pod.Name, portsToForward)
appReadySpinner.End(err == nil)
if err != nil {
log.Warningf("Port forwarding might not work correctly: %v", err)
log.Warning("Running `odo logs --follow` might help in identifying the problem.")
fmt.Fprintln(log.GetStdout())
}

err = a.portForwardClient.StartPortForwarding(a.Devfile, a.ComponentName, parameters.Debug, parameters.RandomPorts, log.GetStdout(), parameters.ErrOut, parameters.CustomForwardedPorts)
if err != nil {
return adapters.NewErrPortForward(err)
Expand Down Expand Up @@ -765,5 +777,15 @@ func (a Adapter) deleteServiceBindingSecrets(serviceBindingSecretsToRemove []uns
return nil
}

func (a *Adapter) checkAppPorts(ctx context.Context, podName string, portsToFwd map[string][]devfilev1.Endpoint) error {
containerPortsMapping := make(map[string][]int)
for c, ports := range portsToFwd {
for _, p := range ports {
containerPortsMapping[c] = append(containerPortsMapping[c], p.TargetPort)
}
}
return port.CheckAppPortsListening(ctx, a.execClient, podName, containerPortsMapping, 1*time.Minute)
}

// PushCommandsMap stores the commands to be executed as per their types.
type PushCommandsMap map[devfilev1.CommandGroupKind]devfilev1.Command
132 changes: 132 additions & 0 deletions pkg/port/port.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package port

import (
"context"
"errors"
"fmt"
"net"
"regexp"
"strconv"
"strings"
"time"

"github.com/segmentio/backo-go"
"golang.org/x/sync/errgroup"
"k8s.io/klog"

"github.com/redhat-developer/odo/pkg/api"
Expand Down Expand Up @@ -236,6 +241,133 @@ func GetConnections(execClient exec.Client, podName string, containerName string
return connections, nil
}

// CheckAppPortsListening checks whether all the specified ports are really opened and in LISTEN mode in each corresponding container
// of the pod specified.
// It does so by periodically looking inside the container for listening connections until it finds each of the specified ports,
// or until the specified timeout has elapsed.
func CheckAppPortsListening(
ctx context.Context,
execClient exec.Client,
podName string,
containerPortMapping map[string][]int,
timeout time.Duration,
) error {
if len(containerPortMapping) == 0 {
return nil
}

backOffBase := 1 * time.Second
if timeout <= backOffBase {
return fmt.Errorf("invalid timeout: %v, must be strictly greater than %v", timeout, backOffBase)
}

ctxWithTimeout, cancel := context.WithTimeout(ctx, timeout)
defer cancel()

hasPortFn := func(connections []Connection, p int) bool {
for _, c := range connections {
if p == c.LocalPort {
return true
}
}
return false
}

notListeningChan := make(chan map[string][]int)

g := new(errgroup.Group)
for container, ports := range containerPortMapping {
container := container
ports := ports

if len(ports) == 0 {
continue
}

g.Go(func() error {
b := backo.NewBacko(backOffBase, 2, 0, 10*time.Second)
ticker := b.NewTicker()
portsNotListening := make(map[int]struct{})

for {
select {
case <-ctxWithTimeout.Done():
if len(portsNotListening) != 0 {
m := make(map[string][]int)
for p := range portsNotListening {
m[container] = append(m[container], p)
}
notListeningChan <- m
}
return ctxWithTimeout.Err()

case <-ticker.C:
connections, err := GetListeningConnections(execClient, podName, container)
if err != nil {
klog.V(3).Infof("error getting listening connections in container %q: %v", container, err)
for _, p := range ports {
portsNotListening[p] = struct{}{}
}
} else {
for _, p := range ports {
if hasPortFn(connections, p) {
delete(portsNotListening, p)
continue
}
klog.V(3).Infof("port %d not listening in container %q", p, container)
portsNotListening[p] = struct{}{}
}
if len(portsNotListening) == 0 {
// no error and all ports expected to be opened are opened at this point
return nil
}
}
}
}
})
}

// Buffer of 1 because we want to close notListeningChan (because we are iterating over it).
errChan := make(chan error, 1)
go func() {
errChan <- g.Wait()
close(notListeningChan)
}()

notListening := make(map[string][]int)
for e := range notListeningChan {
for c, ports := range e {
notListening[c] = append(notListening[c], ports...)
}
}

klog.V(4).Infof("ports not listening: %v", notListening)

if err := <-errChan; err != nil {
msg := "error"
if errors.Is(err, context.DeadlineExceeded) {
msg = "timeout"
}
msg += " while checking for ports"
if len(notListening) == 0 {
klog.V(4).Infof("%s and no unreachable port detected: %v", msg, err)
return nil
}
var msgList []string
for c, ports := range notListening {
var l []string
for _, p := range ports {
l = append(l, strconv.Itoa(p))
}
msgList = append(msgList, fmt.Sprintf("%s in container %q", strings.Join(l, ", "), c))
}
msg += fmt.Sprintf("; ports not listening: (%s)", strings.Join(msgList, "; "))
return fmt.Errorf("%s: %w", msg, err)
}

return nil
}

func stateToString(state int) string {
if state < 1 || state > len(connectionStates) {
return ""
Expand Down
Loading