From bd48c525407e8c877c0a4beb8165e8542dadf232 Mon Sep 17 00:00:00 2001 From: Dharmit Shah Date: Thu, 30 Apr 2020 16:28:23 +0530 Subject: [PATCH] Fixes odo describe for multiple URLs of unpushed component (#3008) * Fix odo describe for multiple ports & URLs * Update unit and integration tests * Change function name and add comment (PR feedback) * Change the way ports are specified in tests * Mention usage to configure ports in the CLI docs * Validate port value in config --- pkg/component/component.go | 34 ++++++++++++++++++++++++++++++++-- pkg/config/fakeConfig.go | 4 +++- pkg/odo/cli/config/set.go | 3 ++- tests/integration/component.go | 4 +++- 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/pkg/component/component.go b/pkg/component/component.go index 43e7429acd3..5825ad49192 100644 --- a/pkg/component/component.go +++ b/pkg/component/component.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "strconv" "strings" "github.com/openshift/odo/pkg/devfile/adapters/common" @@ -893,8 +894,21 @@ func GetComponentFromConfig(localConfig *config.LocalConfigInfo) (Component, err component.Spec.Source = util.GenFileURL(localConfig.GetSourceLocation()) } - for _, localURL := range localConfig.GetURL() { - component.Spec.URL = append(component.Spec.URL, localURL.Name) + urls := localConfig.GetURL() + if len(urls) > 0 { + // We will clean up the existing value of ports and re-populate it so that we don't panic in `odo describe` and don't show inconsistent info + // This will also help in the case where there are more URLs created than the number of ports exposed by a component #2776 + oldPortsProtocol, err := getPortsProtocolMapping(component.Spec.Ports) + if err != nil { + return Component{}, err + } + component.Spec.Ports = []string{} + + for _, url := range urls { + port := strconv.Itoa(url.Port) + component.Spec.Ports = append(component.Spec.Ports, fmt.Sprintf("%s/%s", port, oldPortsProtocol[port])) + component.Spec.URL = append(component.Spec.URL, url.Name) + } } for _, localEnv := range localConfig.GetEnvVars() { @@ -909,6 +923,22 @@ func GetComponentFromConfig(localConfig *config.LocalConfigInfo) (Component, err return Component{}, nil } +// This function returns a mapping of port and protocol. +// So for a value of ports {"8080/TCP", "45/UDP"} it will return a map {"8080": +// "TCP", "45": "UDP"} +func getPortsProtocolMapping(ports []string) (map[string]string, error) { + oldPortsProtocol := make(map[string]string, len(ports)) + for _, port := range ports { + portProtocol := strings.Split(port, "/") + if len(portProtocol) != 2 { + // this will be the case if value of a port is something like 8080/TCP/something-else or simply 8080 + return nil, errors.New("invalid mapping. Please update the component configuration") + } + oldPortsProtocol[portProtocol[0]] = portProtocol[1] + } + return oldPortsProtocol, nil +} + // ListIfPathGiven lists all available component in given path directory func ListIfPathGiven(client *occlient.Client, paths []string) (ComponentList, error) { var components []Component diff --git a/pkg/config/fakeConfig.go b/pkg/config/fakeConfig.go index 4d2555af257..87c45413b38 100644 --- a/pkg/config/fakeConfig.go +++ b/pkg/config/fakeConfig.go @@ -13,14 +13,16 @@ func GetOneExistingConfigInfo(componentName, applicationName, projectName string }, } - portsValue := []string{"8080/TCP,45/UDP"} + portsValue := []string{"8080/TCP", "45/UDP"} urlValue := []ConfigURL{ { Name: "example-url-0", + Port: 8080, }, { Name: "example-url-1", + Port: 45, }, } diff --git a/pkg/odo/cli/config/set.go b/pkg/odo/cli/config/set.go index 5c1a508bad6..8559e6ec8e2 100644 --- a/pkg/odo/cli/config/set.go +++ b/pkg/odo/cli/config/set.go @@ -33,6 +33,7 @@ var ( %[1]s %[9]s 0.5 %[1]s %[10]s 2 %[1]s %[11]s 1 + %[1]s %[12]s 8080/TCP,8443/TCP # Set a env variable in the local config %[1]s --env KAFKA_HOST=kafka --env KAFKA_PORT=6639 @@ -159,7 +160,7 @@ func NewCmdSet(name, fullName string) *cobra.Command { Short: "Set a value in odo config file", Long: fmt.Sprintf(setLongDesc, config.FormatLocallySupportedParameters()), Example: fmt.Sprintf(fmt.Sprint("\n", setExample), fullName, config.Type, - config.Name, config.MinMemory, config.MaxMemory, config.Memory, config.DebugPort, config.Ignore, config.MinCPU, config.MaxCPU, config.CPU), + config.Name, config.MinMemory, config.MaxMemory, config.Memory, config.DebugPort, config.Ignore, config.MinCPU, config.MaxCPU, config.CPU, config.Ports), Args: func(cmd *cobra.Command, args []string) error { if o.envArray != nil { // no args are needed diff --git a/tests/integration/component.go b/tests/integration/component.go index 5cb07d3f833..138bbd9c865 100644 --- a/tests/integration/component.go +++ b/tests/integration/component.go @@ -235,6 +235,7 @@ func componentTests(args ...string) { It("should describe the component when it is not pushed", func() { helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--context", context, "--app", "testing")...) helper.CmdShouldPass("odo", "url", "create", "url-1", "--context", context) + helper.CmdShouldPass("odo", "url", "create", "url-2", "--context", context) helper.CmdShouldPass("odo", "storage", "create", "storage-1", "--size", "1Gi", "--path", "/data1", "--context", context) helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,cmp-git", "Application,testing", "URL,0,Name,url-1") cmpDescribe := helper.CmdShouldPass("odo", append(args, "describe", "--context", context)...) @@ -242,12 +243,13 @@ func componentTests(args ...string) { Expect(cmpDescribe).To(ContainSubstring("cmp-git")) Expect(cmpDescribe).To(ContainSubstring("nodejs")) Expect(cmpDescribe).To(ContainSubstring("url-1")) + Expect(cmpDescribe).To(ContainSubstring("url-2")) Expect(cmpDescribe).To(ContainSubstring("https://github.com/openshift/nodejs-ex")) Expect(cmpDescribe).To(ContainSubstring("storage-1")) cmpDescribeJSON, err := helper.Unindented(helper.CmdShouldPass("odo", append(args, "describe", "-o", "json", "--context", context)...)) Expect(err).Should(BeNil()) - expected, err := helper.Unindented(`{"kind": "Component","apiVersion": "odo.openshift.io/v1alpha1","metadata": {"name": "cmp-git","namespace": "` + project + `","creationTimestamp": null},"spec":{"app": "testing","type":"nodejs","source": "https://github.com/openshift/nodejs-ex","sourceType": "git","url": ["url-1"],"storage": ["storage-1"],"ports": ["8080/TCP"]},"status": {"state": "Not Pushed"}}`) + expected, err := helper.Unindented(`{"kind": "Component","apiVersion": "odo.openshift.io/v1alpha1","metadata": {"name": "cmp-git","namespace": "` + project + `","creationTimestamp": null},"spec":{"app": "testing","type":"nodejs","source": "https://github.com/openshift/nodejs-ex","sourceType": "git","url": ["url-1", "url-2"],"storage": ["storage-1"],"ports": ["8080/TCP"]},"status": {"state": "Not Pushed"}}`) Expect(err).Should(BeNil()) Expect(cmpDescribeJSON).To(Equal(expected)) helper.CmdShouldPass("odo", append(args, "delete", "-f", "--all", "--context", context)...)