Skip to content

Commit

Permalink
fail install if getPort() on existing cluster fails
Browse files Browse the repository at this point in the history
  • Loading branch information
abuchanan-airbyte committed Aug 30, 2024
1 parent 498bb61 commit a8fd7bf
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 12 deletions.
31 changes: 27 additions & 4 deletions internal/cmd/local/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,27 +127,50 @@ func getPort(ctx context.Context, clusterName string) (int, error) {

ci, err := dockerClient.Client.ContainerInspect(ctx, container)
if err != nil {
return 0, fmt.Errorf("unable to inspect container: %w", err)
return 0, fmt.Errorf("%w: %w", ErrUnableToInspect, err)
}
if ci.State == nil || ci.State.Status != "running" {
status := "unknown"
if ci.State != nil {
status = ci.State.Status
}
return 0, fmt.Errorf("container %q is not running (status = %q)", container, status)
return 0, ContainerNotRunningError{container, status}
}

for _, bindings := range ci.HostConfig.PortBindings {
for _, ipPort := range bindings {
if ipPort.HostIP == "0.0.0.0" {
port, err := strconv.Atoi(ipPort.HostPort)
if err != nil {
return 0, fmt.Errorf("unable to convert host port %s to integer: %w", ipPort.HostPort, err)
return 0, InvalidPortError{ipPort.HostPort, err}
}
return port, nil
}
}
}

return 0, fmt.Errorf("no matching ports found on container %q", container)
return 0, fmt.Errorf("%w on container %q", ErrPortNotFound, container)
}

var ErrPortNotFound = errors.New("no matching port found")
var ErrUnableToInspect = errors.New("unable to inspect container")

type ContainerNotRunningError struct {
Container string
Status string
}

func (e ContainerNotRunningError) Error() string {
return fmt.Sprintf("container %q is not running (status = %q)", e.Container, e.Status)
}

type InvalidPortError struct {
Port string
Inner error
}
func (e InvalidPortError) Unwrap() error {
return e.Inner
}
func (e InvalidPortError) Error() string {
return fmt.Sprintf("unable to convert host port %s to integer: %s", e.Port, e.Inner)
}
31 changes: 26 additions & 5 deletions internal/cmd/local/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,9 @@ func TestGetPort_NotRunning(t *testing.T) {
}

_, err := getPort(context.Background(), "test")
if err == nil {
t.Error("expected error")

if !errors.Is(err, ContainerNotRunningError{"test-control-plane", "stopped"}) {
t.Errorf("expected container not running error but got %v", err)
}
}

Expand Down Expand Up @@ -234,7 +235,7 @@ func TestGetPort_Invalid(t *testing.T) {
HostConfig: &container.HostConfig{
PortBindings: nat.PortMap{
"tcp/80": {{
HostIP: "1.2.3.4",
HostIP: "0.0.0.0",
HostPort: "NaN",
}},
},
Expand All @@ -246,8 +247,28 @@ func TestGetPort_Invalid(t *testing.T) {
}

_, err := getPort(context.Background(), "test")
if err == nil {
t.Error("expected error")
var invalidPortErr InvalidPortError
if !errors.As(err, &invalidPortErr) {
t.Errorf("expected invalid port error but got %v", err)
}
}

func TestGetPort_InpsectErr(t *testing.T) {
t.Cleanup(func() {
dockerClient = nil
})

dockerClient = &docker.Docker{
Client: dockertest.MockClient{
FnContainerInspect: func(ctx context.Context, containerID string) (types.ContainerJSON, error) {
return types.ContainerJSON{}, errors.New("test err")
},
},
}

_, err := getPort(context.Background(), "test")
if !errors.Is(err, ErrUnableToInspect) {
t.Errorf("expected ErrUnableToInspect but got %v", err)
}
}

Expand Down
7 changes: 4 additions & 3 deletions internal/cmd/local/local_install.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,11 @@ func (i *InstallCmd) Run(ctx context.Context, provider k8s.Provider, telClient t
providedPort := i.Port
i.Port, err = getPort(ctx, provider.ClusterName)
if err != nil {
pterm.Warning.Printfln("Unable to determine which port the existing cluster was configured to use.\n" +
"Installation will continue but may ultimately fail, in which case it will be necessarily to uninstall first.")
return err
// pterm.Warning.Printfln("Unable to determine which port the existing cluster was configured to use.\n" +
// "Installation will continue but may ultimately fail, in which case it will be necessarily to uninstall first.")
// since we can't verify the port is correct, push forward with the provided port
i.Port = providedPort
// i.Port = providedPort
}
if providedPort != i.Port {
pterm.Warning.Printfln("The existing cluster was found to be using port %d, which differs from the provided port %d.\n"+
Expand Down

0 comments on commit a8fd7bf

Please sign in to comment.