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

Remove API Server from experimental mode, set UI Server as experimental #6985

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
2 changes: 1 addition & 1 deletion pkg/api/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type DevControlPlane struct {
Platform string `json:"platform,omitempty"`
LocalPort int `json:"localPort"`
APIServerPath string `json:"apiServerPath"`
WebInterfacePath string `json:"webInterfacePath"`
WebInterfacePath string `json:"webInterfacePath,omitempty"`
}

func (o DevControlPlane) GetPlatform() string {
Expand Down
17 changes: 11 additions & 6 deletions pkg/apiserver-impl/starterserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
openapi "github.com/redhat-developer/odo/pkg/apiserver-gen/go"
"github.com/redhat-developer/odo/pkg/apiserver-impl/sse"
"github.com/redhat-developer/odo/pkg/kclient"
"github.com/redhat-developer/odo/pkg/odo/cli/feature"
"github.com/redhat-developer/odo/pkg/podman"
"github.com/redhat-developer/odo/pkg/preference"
"github.com/redhat-developer/odo/pkg/state"
Expand Down Expand Up @@ -71,13 +72,17 @@ func StartServer(
swaggerServer := http.FileServer(http.FS(fSysSwagger))
router.PathPrefix("/swagger-ui/").Handler(http.StripPrefix("/swagger-ui/", swaggerServer))

fSys, err := fs.Sub(staticFiles, "ui")
if err != nil {
// Assertion, error can only happen if the path "ui" is not valid
panic(err)
if feature.IsEnabled(ctx, feature.UIServer) {
var fSys fs.FS
fSys, err = fs.Sub(staticFiles, "ui")
if err != nil {
// Assertion, error can only happen if the path "ui" is not valid
panic(err)
}

staticServer := http.FileServer(http.FS(fSys))
router.PathPrefix("/").Handler(staticServer)
}
staticServer := http.FileServer(http.FS(fSys))
router.PathPrefix("/").Handler(staticServer)

if port == 0 && !randomPort {
port, err = util.NextFreePort(20000, 30001, nil, "")
Expand Down
11 changes: 4 additions & 7 deletions pkg/component/describe/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,18 @@ func DescribeDevfileComponent(
kubeClient = nil
}

isApiServerFeatureEnabled := feature.IsEnabled(ctx, feature.APIServerFlag)
// TODO(feloy) Pass PID with `--pid` flag
allControlPlaneData, err := stateClient.GetAPIServerPorts(ctx)
if err != nil {
return api.Component{}, nil, err
}
if isApiServerFeatureEnabled {
for i := range allControlPlaneData {
if allControlPlaneData[i].Platform == "" {
allControlPlaneData[i].Platform = commonflags.PlatformCluster
}
for i := range allControlPlaneData {
if allControlPlaneData[i].Platform == "" {
allControlPlaneData[i].Platform = commonflags.PlatformCluster
}
}

devControlPlaneData := filterByPlatform(ctx, isApiServerFeatureEnabled, allControlPlaneData)
devControlPlaneData := filterByPlatform(ctx, true, allControlPlaneData)

// TODO(feloy) Pass PID with `--pid` flag
allFwdPorts, err := stateClient.GetForwardedPorts(ctx)
Expand Down
9 changes: 6 additions & 3 deletions pkg/odo/cli/describe/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,16 @@ func printHumanReadableOutput(ctx context.Context, cmp api.Component, devfileObj
fmt.Println()
}

if feature.IsEnabled(ctx, feature.APIServerFlag) && len(cmp.DevControlPlane) != 0 {
if len(cmp.DevControlPlane) != 0 {
var webui string
if feature.IsEnabled(ctx, feature.UIServer) {
webui = "\n Web UI: http://%[2]s:%[3]d/"
}
const ctrlPlaneHost = "localhost"
log.Info("Dev Control Plane:")
for _, dcp := range cmp.DevControlPlane {
log.Printf(`%[1]s
API: http://%[2]s:%[3]d/%[4]s
Web UI: http://%[2]s:%[3]d/`,
API: http://%[2]s:%[3]d/%[4]s`+webui,
log.Sbold(dcp.Platform),
ctrlPlaneHost, dcp.LocalPort, strings.TrimPrefix(dcp.APIServerPath, "/"))
}
Expand Down
11 changes: 6 additions & 5 deletions pkg/odo/cli/dev/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"strings"

apiserver_impl "github.com/redhat-developer/odo/pkg/apiserver-impl"
"github.com/redhat-developer/odo/pkg/odo/cli/feature"

"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -180,6 +179,10 @@ func (o *DevOptions) Validate(ctx context.Context) error {
return err
}

if !o.apiServerFlag && o.apiServerPortFlag != 0 {
return errors.New("--api-server-port makes sense only if --api-server is enabled")
}

if o.apiServerFlag && o.apiServerPortFlag != 0 {
if o.randomPortsFlag {
return errors.New("--random-ports and --api-server-port cannot be used together")
Expand Down Expand Up @@ -415,11 +418,9 @@ It forwards endpoints with any exposure values ('public', 'internal' or 'none')
devCmd.Flags().BoolVar(&o.noCommandsFlag, "no-commands", false, "Do not run any commands; just start the development environment.")
devCmd.Flags().BoolVar(&o.syncGitDirFlag, "sync-git-dir", false, "Synchronize the .git directory to the container. By default, this directory is not synchronized.")
devCmd.Flags().BoolVar(&o.logsFlag, "logs", false, "Follow logs of component")
devCmd.Flags().BoolVar(&o.apiServerFlag, "api-server", true, "Start the API Server")
devCmd.Flags().IntVar(&o.apiServerPortFlag, "api-server-port", 0, "Define custom port for API Server; this flag should be used in combination with --api-server flag.")

if feature.IsExperimentalModeEnabled(ctx) {
devCmd.Flags().BoolVar(&o.apiServerFlag, "api-server", false, "Start the API Server; this is an experimental feature")
devCmd.Flags().IntVar(&o.apiServerPortFlag, "api-server-port", 0, "Define custom port for API Server; this flag should be used in combination with --api-server flag.")
}
clientset.Add(devCmd,
clientset.BINDING,
clientset.DEV,
Expand Down
2 changes: 1 addition & 1 deletion pkg/odo/cli/feature/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var (
isExperimental: false,
}

APIServerFlag = OdoFeature{
UIServer = OdoFeature{
isExperimental: true,
}
)
Expand Down
16 changes: 10 additions & 6 deletions pkg/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"regexp"

"github.com/redhat-developer/odo/pkg/api"
"github.com/redhat-developer/odo/pkg/odo/cli/feature"
"github.com/redhat-developer/odo/pkg/odo/commonflags"
fcontext "github.com/redhat-developer/odo/pkg/odo/commonflags/context"
odocontext "github.com/redhat-developer/odo/pkg/odo/context"
Expand Down Expand Up @@ -127,12 +128,15 @@ func (o *State) GetAPIServerPorts(ctx context.Context) ([]api.DevControlPlane, e
if content.APIServerPort == 0 {
continue
}
result = append(result, api.DevControlPlane{
Platform: platform,
LocalPort: content.APIServerPort,
APIServerPath: "/api/v1/",
WebInterfacePath: "/",
})
controlPlane := api.DevControlPlane{
Platform: platform,
LocalPort: content.APIServerPort,
APIServerPath: "/api/v1/",
}
if feature.IsEnabled(ctx, feature.UIServer) {
controlPlane.WebInterfacePath = "/"
}
result = append(result, controlPlane)
}
return result, nil
}
Expand Down
11 changes: 5 additions & 6 deletions tests/helper/helper_dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,11 @@ func StartDevMode(options DevSessionOpts) (devSession DevSession, err error) {
if options.CustomAddress != "" {
args = append(args, "--address", options.CustomAddress)
}
if options.StartAPIServer {
env = append(env, "ODO_EXPERIMENTAL_MODE=true")
args = append(args, "--api-server")
if options.APIServerPort != 0 {
args = append(args, "--api-server-port", fmt.Sprintf("%d", options.APIServerPort))
}
if !options.StartAPIServer {
args = append(args, "--api-server=false")
}
if options.APIServerPort != 0 {
args = append(args, fmt.Sprintf("--api-server-port=%d", options.APIServerPort))
}
if options.SyncGitDir {
args = append(args, "--sync-git-dir")
Expand Down
85 changes: 57 additions & 28 deletions tests/integration/cmd_dev_api_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,43 @@ var _ = Describe("odo dev command with api server tests", func() {
var _ = AfterEach(func() {
helper.CommonAfterEach(commonVar)
})

for _, podman := range []bool{false, true} {
podman := podman
for _, customPort := range []bool{false, true} {
customPort := customPort
When("the component is bootstrapped", helper.LabelPodmanIf(podman, func() {
BeforeEach(func() {
helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context)
helper.CopyExampleDevFile(filepath.Join("source", "devfiles", "nodejs", "devfile.yaml"), filepath.Join(commonVar.Context, "devfile.yaml"), cmpName)
})

When("the component is bootstrapped", helper.LabelPodmanIf(podman, func() {
BeforeEach(func() {
helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context)
helper.CopyExampleDevFile(filepath.Join("source", "devfiles", "nodejs", "devfile.yaml"), filepath.Join(commonVar.Context, "devfile.yaml"), cmpName)
})

It("should fail if --api-server is false but --api-server-port is true", func() {
args := []string{
"dev",
"--api-server=false",
fmt.Sprintf("--api-server-port=%d", helper.GetCustomStartPort()),
}
if podman {
args = append(args, "--platform=podman")
}
errOut := helper.Cmd("odo", args...).ShouldFail().Err()
Expect(errOut).To(ContainSubstring("--api-server-port makes sense only if --api-server is enabled"))
})

for _, customPort := range []bool{false, true} {
customPort := customPort

if customPort {
It("should fail if --random-ports and --api-server-port are used together", func() {
args := []string{
"dev",
"--random-ports",
"--api-server",
fmt.Sprintf("--api-server-port=%d", helper.GetCustomStartPort()),
}
if podman {
args = append(args, "--platform=podman")
}
errOut := helper.Cmd("odo", args...).AddEnv("ODO_EXPERIMENTAL_MODE=true").ShouldFail().Err()
errOut := helper.Cmd("odo", args...).ShouldFail().Err()
Expect(errOut).Should(ContainSubstring("--random-ports and --api-server-port cannot be used together"))
})
}
Expand Down Expand Up @@ -90,47 +105,61 @@ var _ = Describe("odo dev command with api server tests", func() {
Expect(resp.StatusCode).To(BeEquivalentTo(http.StatusOK))
})

It("should not describe the API Server in non-experimental mode", func() {
It("should describe the API Server port in the experimental mode", func() {
args := []string{"describe", "component"}
if podman {
args = append(args, "--platform", "podman")
}
stdout := helper.Cmd("odo", args...).ShouldPass().Out()
for _, s := range []string{"Dev Control Plane", "API Server"} {
Expect(stdout).ShouldNot(ContainSubstring(s))
stdout := helper.Cmd("odo", args...).AddEnv("ODO_EXPERIMENTAL_MODE=true").ShouldPass().Out()
Expect(stdout).To(ContainSubstring("Dev Control Plane"))
Expect(stdout).To(ContainSubstring("API: http://%s", devSession.APIServerEndpoint))
if customPort {
Expect(stdout).To(ContainSubstring("Web UI: http://localhost:%d/", localPort))
} else {
Expect(stdout).To(MatchRegexp("Web UI: http:\\/\\/localhost:[0-9]+\\/"))
}
})

It("should not describe the API Server in non-experimental mode (JSON)", func() {
It("should describe the API Server port in the experimental mode (JSON)", func() {
args := []string{"describe", "component", "-o", "json"}
if podman {
args = append(args, "--platform", "podman")
}
stdout := helper.Cmd("odo", args...).ShouldPass().Out()
helper.JsonPathDoesNotExist(stdout, "devControlPlane")
stdout := helper.Cmd("odo", args...).AddEnv("ODO_EXPERIMENTAL_MODE=true").ShouldPass().Out()
helper.IsJSON(stdout)
helper.JsonPathExist(stdout, "devControlPlane")
plt := "cluster"
if podman {
plt = "podman"
}
helper.JsonPathContentHasLen(stdout, "devControlPlane", 1)
helper.JsonPathContentIs(stdout, "devControlPlane.0.platform", plt)
if customPort {
helper.JsonPathContentIs(stdout, "devControlPlane.0.localPort", strconv.Itoa(localPort))
} else {
helper.JsonPathContentIsValidUserPort(stdout, "devControlPlane.0.localPort")
}
helper.JsonPathContentIs(stdout, "devControlPlane.0.apiServerPath", "/api/v1/")
helper.JsonPathContentIs(stdout, "devControlPlane.0.webInterfacePath", "/")
})

It("should describe the API Server port in the experimental mode", func() {
It("should describe the API Server port", func() {
args := []string{"describe", "component"}
if podman {
args = append(args, "--platform", "podman")
}
stdout := helper.Cmd("odo", args...).AddEnv("ODO_EXPERIMENTAL_MODE=true").ShouldPass().Out()
stdout := helper.Cmd("odo", args...).ShouldPass().Out()
Expect(stdout).To(ContainSubstring("Dev Control Plane"))
Expect(stdout).To(ContainSubstring("API: http://%s", devSession.APIServerEndpoint))
if customPort {
Expect(stdout).To(ContainSubstring("Web UI: http://localhost:%d/", localPort))
} else {
Expect(stdout).To(MatchRegexp("Web UI: http:\\/\\/localhost:[0-9]+\\/"))
}
Expect(stdout).ToNot(ContainSubstring("Web UI: http://localhost:%d/", localPort))
})

It("should describe the API Server port in the experimental mode (JSON)", func() {
It("should describe the API Server port (JSON)", func() {
args := []string{"describe", "component", "-o", "json"}
if podman {
args = append(args, "--platform", "podman")
}
stdout := helper.Cmd("odo", args...).AddEnv("ODO_EXPERIMENTAL_MODE=true").ShouldPass().Out()
stdout := helper.Cmd("odo", args...).ShouldPass().Out()
helper.IsJSON(stdout)
helper.JsonPathExist(stdout, "devControlPlane")
plt := "cluster"
Expand All @@ -145,11 +174,11 @@ var _ = Describe("odo dev command with api server tests", func() {
helper.JsonPathContentIsValidUserPort(stdout, "devControlPlane.0.localPort")
}
helper.JsonPathContentIs(stdout, "devControlPlane.0.apiServerPath", "/api/v1/")
helper.JsonPathContentIs(stdout, "devControlPlane.0.webInterfacePath", "/")
helper.JsonPathDoesNotExist(stdout, "devControlPlane.0.webInterfacePath")
})
})
}))
}
}
}))

When("the component is bootstrapped", helper.LabelPodmanIf(podman, func() {
BeforeEach(func() {
Expand Down