Skip to content

Commit

Permalink
Fixes odo describe for multiple URLs of unpushed component (#3008)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
dharmit authored Apr 30, 2020
1 parent 6687e71 commit bd48c52
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 5 deletions.
34 changes: 32 additions & 2 deletions pkg/component/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io"
"os"
"path/filepath"
"strconv"
"strings"

"github.com/openshift/odo/pkg/devfile/adapters/common"
Expand Down Expand Up @@ -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() {
Expand All @@ -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 <port/protocol> 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
Expand Down
4 changes: 3 additions & 1 deletion pkg/config/fakeConfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/odo/cli/config/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,19 +235,21 @@ 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)...)

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)...)
Expand Down

0 comments on commit bd48c52

Please sign in to comment.