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

* Remove API Server from experimental mode, set UI Server as experimental

* Adapt api-server integration tests

* Disable --api-server by default in the tests

They are enabled only if `options.StartAPIServer`
is explicitly enabled.
This is to avoid potential port conflicting issue,
especially on Windows - see [1]

[1] https://github.com/redhat-developer/odo/issues/6939

* Error out if `--api-server-port` is set but `--api-server` is false

---------

Co-authored-by: Armel Soro <asoro@redhat.com>

pkg/api/component.go

@@ -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 {

pkg/apiserver-impl/starterserver.go

@@ -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"
@@ -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, "")

pkg/component/describe/describe.go

@@ -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)

pkg/odo/cli/describe/component.go

@@ -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, "/"))
 		}

pkg/odo/cli/dev/dev.go

@@ -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"
@@ -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")
@@ -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,

pkg/odo/cli/feature/features.go

@@ -18,7 +18,7 @@ var (
 		isExperimental: false,
 	}
 
-	APIServerFlag = OdoFeature{
+	UIServer = OdoFeature{
 		isExperimental: true,
 	}
 )

pkg/state/state.go

@@ -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"
@@ -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
 }

tests/helper/helper_dev.go

@@ -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")

tests/integration/cmd_dev_api_server_test.go

@@ -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"))
 					})
 				}
@@ -90,26 +105,6 @@ 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() {
-						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))
-						}
-					})
-
-					It("should not describe the API Server in non-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")
-					})
-
 					It("should describe the API Server port in the experimental mode", func() {
 						args := []string{"describe", "component"}
 						if podman {
@@ -147,9 +142,43 @@ var _ = Describe("odo dev command with api server tests", func() {
 						helper.JsonPathContentIs(stdout, "devControlPlane.0.apiServerPath", "/api/v1/")
 						helper.JsonPathContentIs(stdout, "devControlPlane.0.webInterfacePath", "/")
 					})
+
+					It("should describe the API Server port", func() {
+						args := []string{"describe", "component"}
+						if podman {
+							args = append(args, "--platform", "podman")
+						}
+						stdout := helper.Cmd("odo", args...).ShouldPass().Out()
+						Expect(stdout).To(ContainSubstring("Dev Control Plane"))
+						Expect(stdout).To(ContainSubstring("API: http://%s", devSession.APIServerEndpoint))
+						Expect(stdout).ToNot(ContainSubstring("Web UI: http://localhost:%d/", localPort))
+					})
+
+					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...).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.JsonPathDoesNotExist(stdout, "devControlPlane.0.webInterfacePath")
+					})
 				})
-			}))
-		}
+			}
+		}))
 
 		When("the component is bootstrapped", helper.LabelPodmanIf(podman, func() {
 			BeforeEach(func() {