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

Fix: customize demo cluster port #5969

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions flytectl/cmd/config/subcommand/sandbox/config_flags.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions flytectl/cmd/config/subcommand/sandbox/config_flags_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 15 additions & 2 deletions flytectl/cmd/config/subcommand/sandbox/sandbox_config.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package sandbox

import "github.com/flyteorg/flyte/flytectl/pkg/docker"
import (
"fmt"

"github.com/flyteorg/flyte/flytectl/pkg/docker"
)

// Config holds configuration flags for sandbox command.
type Config struct {
Expand Down Expand Up @@ -36,9 +40,18 @@
DryRun bool `json:"dryRun" pflag:",Optional. Only print the docker commands to bring up flyte sandbox/demo container.This will still call github api's to get the latest flyte release to use'"`

Force bool `json:"force" pflag:",Optional. Forcefully delete existing sandbox cluster if it exists."`

// Allow user to specify the port for the sandbox
Port string `json:"port" pflag:",Optional. Specify the port for the sandbox."`
vincent0426 marked this conversation as resolved.
Show resolved Hide resolved
}

//go:generate pflags Config --default-var DefaultConfig --bind-default-var
var (
DefaultConfig = &Config{}
DefaultConfig = &Config{
Port: "6443", // Default port for the sandbox
}
)

func (c Config) GetK8sEndpoint() string {
return fmt.Sprintf("https://127.0.0.1:%s", c.Port)

Check warning on line 56 in flytectl/cmd/config/subcommand/sandbox/sandbox_config.go

View check run for this annotation

Codecov / codecov/patch

flytectl/cmd/config/subcommand/sandbox/sandbox_config.go#L55-L56

Added lines #L55 - L56 were not covered by tests
}
5 changes: 0 additions & 5 deletions flytectl/cmd/demo/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@ import (
"github.com/spf13/cobra"
)

const (
flyteNs = "flyte"
K8sEndpoint = "https://127.0.0.1:6443"
)

// Long descriptions are whitespace sensitive when generating docs using sphinx.
const (
demoShort = `Helps with demo interactions like start, teardown, status, and exec.`
Expand Down
37 changes: 3 additions & 34 deletions flytectl/cmd/demo/reload.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@ import (
"context"
"fmt"

sandboxCmdConfig "github.com/flyteorg/flyte/flytectl/cmd/config/subcommand/sandbox"
cmdCore "github.com/flyteorg/flyte/flytectl/cmd/core"
"github.com/flyteorg/flyte/flytectl/pkg/docker"
"github.com/flyteorg/flyte/flytectl/pkg/k8s"
"github.com/flyteorg/flyte/flytestdlib/logger"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/flyteorg/flyte/flytectl/pkg/sandbox"
)

const (
internalBootstrapAgent = "flyte-sandbox-bootstrap"
labelSelector = "app.kubernetes.io/name=flyte-binary"
)
const (
reloadShort = "Power cycle the Flyte executable pod, effectively picking up an updated config."
Expand Down Expand Up @@ -73,7 +71,7 @@ func reloadDemoCluster(ctx context.Context, args []string, cmdCtx cmdCore.Comman
return err
}
if useLegacyMethod {
return legacyReloadDemoCluster(ctx)
return sandbox.LegacyReloadDemoCluster(ctx, sandboxCmdConfig.DefaultConfig)
}

// At this point we know that we are on a modern sandbox, and we can use the
Expand All @@ -88,32 +86,3 @@ func reloadDemoCluster(ctx context.Context, args []string, cmdCtx cmdCore.Comman

return nil
}

// legacyReloadDemoCluster will kill the flyte binary pod so the new one can pick up a new config file
func legacyReloadDemoCluster(ctx context.Context) error {
k8sClient, err := k8s.GetK8sClient(docker.Kubeconfig, K8sEndpoint)
if err != nil {
fmt.Println("Could not get K8s client")
return err
}
pi := k8sClient.CoreV1().Pods(flyteNs)
podList, err := pi.List(ctx, v1.ListOptions{LabelSelector: labelSelector})
if err != nil {
fmt.Println("could not list pods")
return err
}
if len(podList.Items) != 1 {
return fmt.Errorf("should only have one pod running, %d found, %v", len(podList.Items), podList.Items)
}
logger.Debugf(ctx, "Found %d pods\n", len(podList.Items))
var grace = int64(0)
err = pi.Delete(ctx, podList.Items[0].Name, v1.DeleteOptions{
GracePeriodSeconds: &grace,
})
if err != nil {
fmt.Printf("Could not delete Flyte pod, old configuration may still be in effect. Err: %s\n", err)
return err
}

return nil
}
5 changes: 5 additions & 0 deletions flytectl/cmd/demo/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ Starts the demo cluster without any source code:

flytectl demo start

Starts the demo cluster with different port:
::

flytectl demo start --port 6443

Runs a dev cluster, which only has minio and postgres pod.
::

Expand Down
14 changes: 7 additions & 7 deletions flytectl/pkg/docker/docker_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,14 @@ func GetSandboxPorts() (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding, e
}

// GetDemoPorts will return demo ports
func GetDemoPorts() (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding, error) {
func GetDemoPorts(k8sPort string) (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding, error) {
return nat.ParsePortSpecs([]string{
"0.0.0.0:6443:6443", // K3s API Port
"0.0.0.0:30080:30080", // HTTP Port
"0.0.0.0:30000:30000", // Registry Port
"0.0.0.0:30001:30001", // Postgres Port
"0.0.0.0:30002:30002", // Minio API Port (use HTTP port for minio console)
"0.0.0.0:30003:30003", // Buildkit Port
fmt.Sprintf("0.0.0.0:%s:6443", k8sPort), // K3s API Port
"0.0.0.0:30080:30080", // HTTP Port
"0.0.0.0:30000:30000", // Registry Port
"0.0.0.0:30001:30001", // Postgres Port
"0.0.0.0:30002:30002", // Minio API Port (use HTTP port for minio console)
"0.0.0.0:30003:30003", // Buildkit Port
})
}

Expand Down
2 changes: 1 addition & 1 deletion flytectl/pkg/docker/docker_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ func TestGetOrCreateVolume(t *testing.T) {
}

func TestDemoPorts(t *testing.T) {
_, ports, _ := GetDemoPorts()
_, ports, _ := GetDemoPorts("6443")
assert.Equal(t, 6, len(ports))
}

Expand Down
47 changes: 47 additions & 0 deletions flytectl/pkg/sandbox/reload.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package sandbox

import (
"context"
"fmt"

sandboxCmdConfig "github.com/flyteorg/flyte/flytectl/cmd/config/subcommand/sandbox"
"github.com/flyteorg/flyte/flytectl/pkg/docker"
"github.com/flyteorg/flyte/flytectl/pkg/k8s"
"github.com/flyteorg/flyte/flytestdlib/logger"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
flyteNs = "flyte"
labelSelector = "app.kubernetes.io/name=flyte-binary"
)

// LegacyReloadDemoCluster will kill the flyte binary pod so the new one can pick up a new config file
func LegacyReloadDemoCluster(ctx context.Context, sandboxConfig *sandboxCmdConfig.Config) error {
k8sEndpoint := sandboxConfig.GetK8sEndpoint()
k8sClient, err := k8s.GetK8sClient(docker.Kubeconfig, k8sEndpoint)
if err != nil {
fmt.Println("Could not get K8s client")
return err
}
pi := k8sClient.CoreV1().Pods(flyteNs)
podList, err := pi.List(ctx, v1.ListOptions{LabelSelector: labelSelector})
if err != nil {
fmt.Println("could not list pods")
return err
}
if len(podList.Items) != 1 {
return fmt.Errorf("should only have one pod running, %d found, %v", len(podList.Items), podList.Items)
}
logger.Debugf(ctx, "Found %d pods\n", len(podList.Items))
var grace = int64(0)
err = pi.Delete(ctx, podList.Items[0].Name, v1.DeleteOptions{
GracePeriodSeconds: &grace,
})
if err != nil {
fmt.Printf("Could not delete Flyte pod, old configuration may still be in effect. Err: %s\n", err)
return err
}

Check warning on line 44 in flytectl/pkg/sandbox/reload.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/sandbox/reload.go#L20-L44

Added lines #L20 - L44 were not covered by tests

return nil

Check warning on line 46 in flytectl/pkg/sandbox/reload.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/sandbox/reload.go#L46

Added line #L46 was not covered by tests
}
8 changes: 4 additions & 4 deletions flytectl/pkg/sandbox/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
taintEffect = "NoSchedule"
sandboxContextName = "flyte-sandbox"
sandboxDockerContext = "default"
K8sEndpoint = "https://127.0.0.1:6443"
sandboxK8sEndpoint = "https://127.0.0.1:30086"
sandboxImageName = "cr.flyte.org/flyteorg/flyte-sandbox"
demoImageName = "cr.flyte.org/flyteorg/flyte-sandbox-bundled"
Expand Down Expand Up @@ -280,12 +279,13 @@
return err
}

k8sEndpoint := sandboxConfig.GetK8sEndpoint()

Check warning on line 282 in flytectl/pkg/sandbox/start.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/sandbox/start.go#L282

Added line #L282 was not covered by tests
if reader != nil {
var k8sClient k8s.K8s
err = retry.Do(
func() error {
// This should wait for the kubeconfig file being there.
k8sClient, err = k8s.GetK8sClient(docker.Kubeconfig, K8sEndpoint)
k8sClient, err = k8s.GetK8sClient(docker.Kubeconfig, k8sEndpoint)

Check warning on line 288 in flytectl/pkg/sandbox/start.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/sandbox/start.go#L288

Added line #L288 was not covered by tests
return err
},
retry.Attempts(10),
Expand All @@ -299,7 +299,7 @@
err = retry.Do(
func() error {
// Have to get a new client every time because you run into x509 errors if not
k8sClient, err = k8s.GetK8sClient(docker.Kubeconfig, K8sEndpoint)
k8sClient, err = k8s.GetK8sClient(docker.Kubeconfig, k8sEndpoint)

Check warning on line 302 in flytectl/pkg/sandbox/start.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/sandbox/start.go#L302

Added line #L302 was not covered by tests
if err != nil {
logger.Debugf(ctx, "Error getting K8s client in liveness check %s", err)
return err
Expand Down Expand Up @@ -398,7 +398,7 @@

func StartDemoCluster(ctx context.Context, args []string, sandboxConfig *sandboxCmdConfig.Config) error {
sandboxImagePrefix := "sha"
exposedPorts, portBindings, err := docker.GetDemoPorts()
exposedPorts, portBindings, err := docker.GetDemoPorts(sandboxConfig.Port)
if err != nil {
return err
}
Expand Down
Loading