Skip to content

Commit

Permalink
Make sure to port-forward all endpoints, regardless of exposure (redh…
Browse files Browse the repository at this point in the history
…at-developer#6119)

Defining endpoints for Debug ports is required for Devfiles
to work properly with `odo dev --debug`.
This addresses the question of exposure for such endpoints.

Tools like DevSpaces currently set 'exposure: none' for debug endpoints.
This makes sense from the developer perspective,
as 'odo dev' aims at emulating a local developer machine.

As a consequence, `odo dev` should no longer ignore endpoints with
'exposure: none'. They should be port-forwarded just like other endpoints.
  • Loading branch information
rm3l authored and valaparthvi committed Sep 15, 2022
1 parent ee25841 commit b4bc9a8
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 92 deletions.
6 changes: 2 additions & 4 deletions pkg/libdevfile/libdevfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ func execDevfileEvent(devfileObj parser.DevfileObj, events []string, handler Han
return nil
}

// GetContainerEndpointMapping returns a map of container names and slice of its endpoints (in int) with exposure status other than none
// GetContainerEndpointMapping returns a map of container names and slice of its endpoints (in int)
func GetContainerEndpointMapping(containers []v1alpha2.Component) map[string][]int {
ceMapping := make(map[string][]int)
for _, container := range containers {
Expand All @@ -284,9 +284,7 @@ func GetContainerEndpointMapping(containers []v1alpha2.Component) map[string][]i

endpoints := container.Container.Endpoints
for _, e := range endpoints {
if e.Exposure != v1alpha2.NoneEndpointExposure {
ceMapping[k] = append(ceMapping[k], e.TargetPort)
}
ceMapping[k] = append(ceMapping[k], e.TargetPort)
}
}
return ceMapping
Expand Down
40 changes: 36 additions & 4 deletions pkg/libdevfile/libdevfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,17 @@ func TestGetContainerEndpointMapping(t *testing.T) {
},
})

containerWithOneNoneInternalEndpoint := generator.GetContainerComponent(generator.ContainerComponentParams{
Name: "container-none-endpoint",
Endpoints: []v1alpha2.Endpoint{
{
Name: "debug",
TargetPort: 9099,
Exposure: v1alpha2.NoneEndpointExposure,
},
},
})

tests := []struct {
name string
args args
Expand All @@ -552,16 +563,37 @@ func TestGetContainerEndpointMapping(t *testing.T) {
{
name: "multiple containers with varying types of endpoints",
args: args{
containers: []v1alpha2.Component{containerWithNoEndpoints, containerWithOnePublicEndpoint, containerWithOneInternalEndpoint},
containers: []v1alpha2.Component{
containerWithNoEndpoints,
containerWithOnePublicEndpoint,
containerWithOneInternalEndpoint,
containerWithOneNoneInternalEndpoint,
},
},
want: map[string][]int{
containerWithNoEndpoints.Name: {},
containerWithOnePublicEndpoint.Name: {8080},
containerWithOneInternalEndpoint.Name: {9090},
containerWithOneNoneInternalEndpoint.Name: {9099},
},
want: map[string][]int{containerWithNoEndpoints.Name: {}, containerWithOnePublicEndpoint.Name: {8080}, containerWithOneInternalEndpoint.Name: {9090}},
},
{
name: "invalid input - one image component with rest being containers",
args: args{
containers: []v1alpha2.Component{containerWithNoEndpoints, containerWithOnePublicEndpoint, containerWithOneInternalEndpoint, imageComponent},
containers: []v1alpha2.Component{
containerWithNoEndpoints,
containerWithOnePublicEndpoint,
containerWithOneInternalEndpoint,
containerWithOneNoneInternalEndpoint,
imageComponent,
},
},
want: map[string][]int{
containerWithNoEndpoints.Name: {},
containerWithOnePublicEndpoint.Name: {8080},
containerWithOneInternalEndpoint.Name: {9090},
containerWithOneNoneInternalEndpoint.Name: {9099},
},
want: map[string][]int{containerWithNoEndpoints.Name: {}, containerWithOnePublicEndpoint.Name: {8080}, containerWithOneInternalEndpoint.Name: {9090}},
},
}
for _, tt := range tests {
Expand Down
2 changes: 1 addition & 1 deletion pkg/odo/cli/dev/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func NewCmdDev(name, fullName string) *cobra.Command {
Use: name,
Short: "Deploy component to development cluster",
Long: `odo dev is a long running command that will automatically sync your source to the cluster.
It forwards endpoints with exposure values 'public' or 'internal' to a port on localhost.`,
It forwards endpoints with any exposure values ('public', 'internal' or 'none') to a port on localhost.`,
Example: fmt.Sprintf(devExample, fullName),
Args: cobra.MaximumNArgs(0),
Run: func(cmd *cobra.Command, args []string) {
Expand Down
108 changes: 25 additions & 83 deletions tests/integration/cmd_dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -568,23 +569,23 @@ ComponentSettings:
devSession.WaitEnd()
})

It("should expose two endpoints on localhost", func() {
url1 := fmt.Sprintf("http://%s", ports["3000"])
url2 := fmt.Sprintf("http://%s", ports["4567"])

resp1, err := http.Get(url1)
Expect(err).ToNot(HaveOccurred())
defer resp1.Body.Close()

resp2, err := http.Get(url2)
Expect(err).ToNot(HaveOccurred())
defer resp2.Body.Close()
It("should expose all endpoints on localhost regardless of exposure", func() {
getServerResponse := func(p int) string {
resp, err := http.Get(fmt.Sprintf("http://%s", ports[strconv.Itoa(p)]))
Expect(err).ToNot(HaveOccurred())
defer resp.Body.Close()

body1, _ := io.ReadAll(resp1.Body)
helper.MatchAllInOutput(string(body1), []string{"Hello from Node.js Starter Application!"})
body, _ := io.ReadAll(resp.Body)
return string(body)
}
containerPorts := []int{3000, 4567, 7890}

body2, _ := io.ReadAll(resp2.Body)
helper.MatchAllInOutput(string(body2), []string{"Hello from Node.js Starter Application!"})
for _, p := range containerPorts {
By(fmt.Sprintf("exposing a port targeting container port %d", p), func() {
r := getServerResponse(p)
helper.MatchAllInOutput(r, []string{"Hello from Node.js Starter Application!"})
})
}

helper.ReplaceString("server.js", "Hello from Node.js", "H3110 from Node.js")

Expand All @@ -596,76 +597,17 @@ ComponentSettings:
devSession.PressKey('p')
}

_, _, _, err = devSession.WaitSync()
_, _, _, err := devSession.WaitSync()
Expect(err).Should(Succeed())

Eventually(func() bool {
resp3, err := http.Get(url1)
if err != nil {
return false
}
defer resp3.Body.Close()

resp4, err := http.Get(url2)
if err != nil {
return false
}
defer resp4.Body.Close()

body3, _ := io.ReadAll(resp3.Body)
if string(body3) != "H3110 from Node.js Starter Application!" {
return false
}

body4, _ := io.ReadAll(resp4.Body)
return string(body4) == "H3110 from Node.js Starter Application!"
}, 180, 10).Should(Equal(true))
})

When("an endpoint is added after first run of odo dev", func() {

BeforeEach(func() {
helper.ReplaceString("devfile.yaml", "exposure: none", "exposure: public")

if manual {
if os.Getenv("SKIP_KEY_PRESS") == "true" {
Skip("This is a unix-terminal specific scenario, skipping")
}

devSession.PressKey('p')
}

var err error
_, _, ports, err = devSession.WaitSync()
Expect(err).Should(Succeed())

})
It("should expose three endpoints on localhost", func() {
url1 := fmt.Sprintf("http://%s", ports["3000"])
url2 := fmt.Sprintf("http://%s", ports["4567"])
url3 := fmt.Sprintf("http://%s", ports["7890"])

resp1, err := http.Get(url1)
Expect(err).ToNot(HaveOccurred())
defer resp1.Body.Close()

resp2, err := http.Get(url2)
Expect(err).ToNot(HaveOccurred())
defer resp2.Body.Close()

resp3, err := http.Get(url3)
Expect(err).ToNot(HaveOccurred())
defer resp3.Body.Close()

body1, _ := io.ReadAll(resp1.Body)
helper.MatchAllInOutput(string(body1), []string{"Hello from Node.js Starter Application!"})

body2, _ := io.ReadAll(resp2.Body)
helper.MatchAllInOutput(string(body2), []string{"Hello from Node.js Starter Application!"})

body3, _ := io.ReadAll(resp3.Body)
helper.MatchAllInOutput(string(body3), []string{"Hello from Node.js Starter Application!"})
})
for _, p := range containerPorts {
By(fmt.Sprintf("returning the right response when querying port forwarded for container port %d", p),
func() {
Eventually(func() string {
return getServerResponse(p)
}, 180, 10).Should(Equal("H3110 from Node.js Starter Application!"))
})
}
})
})

Expand Down

0 comments on commit b4bc9a8

Please sign in to comment.