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

287-format pod container port name #51

Merged
merged 6 commits into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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 go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/devfile/library
go 1.13

require (
github.com/devfile/api v0.0.0-20201211221100-a68230324c7e
github.com/devfile/api/v2 v2.0.0-20210111205815-273464363288
github.com/fatih/color v1.7.0
github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
github.com/gobwas/glob v0.2.3
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7Do
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/devfile/api v0.0.0-20201211221100-a68230324c7e h1:Jy3C3ul05YvL4bJpAVhFwPZD8neOJUBZy7GuCcjc8nc=
github.com/devfile/api v0.0.0-20201211221100-a68230324c7e/go.mod h1:/aDiwWjDEW/fY1/Ig8umVtmneAXKZImnLvWqzMlcfrY=
github.com/devfile/api/v2 v2.0.0-20210111205815-273464363288 h1:3Ea0dVgtCEyWSjobneehLnqIGoK2pS6bWnGnJ1ek24Q=
github.com/devfile/api/v2 v2.0.0-20210111205815-273464363288/go.mod h1:ujP0i3nip2g/aSOXGEy3A7vTC6EIe1kZf9Cteja6OJg=
github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
github.com/docker/docker v0.7.3-0.20190327010347-be7ac8be2ae0/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
github.com/docker/go-units v0.3.3/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
Expand Down
4 changes: 2 additions & 2 deletions pkg/devfile/generator/generators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"reflect"
"testing"

v1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/api/pkg/attributes"
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking if we should call these aliases devfileApiv2 🤔 it will be less confusing as well

"github.com/devfile/api/v2/pkg/attributes"
"github.com/devfile/library/pkg/devfile/parser"
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
"github.com/devfile/library/pkg/testingutil"
Expand Down
21 changes: 16 additions & 5 deletions pkg/devfile/generator/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ import (
"path/filepath"
"strings"

v1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/pkg/devfile/parser"
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
"github.com/devfile/library/pkg/util"
buildv1 "github.com/openshift/api/build/v1"
routev1 "github.com/openshift/api/route/v1"
appsv1 "k8s.io/api/apps/v1"
Expand All @@ -34,13 +33,25 @@ func convertEnvs(vars []v1.EnvVar) []corev1.EnvVar {
// convertPorts converts endpoint variables from the devfile structure to kubernetes ContainerPort
func convertPorts(endpoints []v1.Endpoint) []corev1.ContainerPort {
containerPorts := []corev1.ContainerPort{}
portMap := make(map[string]bool)
for _, endpoint := range endpoints {
name := strings.TrimSpace(util.GetDNS1123Name(strings.ToLower(endpoint.Name)))
name = util.TruncateString(name, 15)
var portProtocol corev1.Protocol
portNumber := int32(endpoint.TargetPort)

if endpoint.Protocol == v1.UDPEndpointProtocol {
portProtocol = corev1.ProtocolUDP
} else {
portProtocol = corev1.ProtocolTCP
}
name := fmt.Sprintf("%d-%s", portNumber, strings.ToLower(string(portProtocol)))
if _, exist := portMap[name]; exist {
continue
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if _, exist := portMap[name]; exist {
continue
}
if _, exist := portMap[name]; !exist {
portMap[name] = true
containerPorts = append(containerPorts, corev1.ContainerPort{
Name: name,
ContainerPort: portNumber,
Protocol: portProtocol,
})
}

would probably do this way but upto you

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

portMap[name] = true
containerPorts = append(containerPorts, corev1.ContainerPort{
Name: name,
ContainerPort: int32(endpoint.TargetPort),
ContainerPort: portNumber,
Protocol: portProtocol,
})
}
return containerPorts
Expand Down
13 changes: 8 additions & 5 deletions pkg/devfile/generator/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import (
"strings"
"testing"

"github.com/devfile/api/pkg/attributes"
"github.com/devfile/api/v2/pkg/attributes"
"github.com/devfile/library/pkg/devfile/parser"
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
"github.com/devfile/library/pkg/testingutil"
buildv1 "github.com/openshift/api/build/v1"

v1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -108,8 +108,9 @@ func TestConvertPorts(t *testing.T) {
},
want: []corev1.ContainerPort{
{
Name: endpointsNames[0],
Name: "8080-tcp",
ContainerPort: int32(endpointsPorts[0]),
Protocol: "TCP",
},
},
},
Expand All @@ -127,12 +128,14 @@ func TestConvertPorts(t *testing.T) {
},
want: []corev1.ContainerPort{
{
Name: endpointsNames[0],
Name: "8080-tcp",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed, can you update teh test case name, it seems wrong

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

ContainerPort: int32(endpointsPorts[0]),
Protocol: "TCP",
},
{
Name: endpointsNames[1],
Name: "9090-tcp",
ContainerPort: int32(endpointsPorts[1]),
Protocol: "TCP",
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/devfile/parser/configurables.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"strconv"
"strings"

v1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
corev1 "k8s.io/api/core/v1"
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/devfile/parser/configurables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"reflect"
"testing"

v1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
devfileCtx "github.com/devfile/library/pkg/devfile/parser/context"
v2 "github.com/devfile/library/pkg/devfile/parser/data/v2"
"github.com/devfile/library/pkg/testingutil/filesystem"
Expand Down
10 changes: 5 additions & 5 deletions pkg/devfile/parser/context/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const (
],
"commands": [
{
"id": "download dependencies",
"id": "download-dependencies",
"exec": {
"component": "nodejs",
"commandLine": "npm install",
Expand All @@ -78,7 +78,7 @@ const (
}
},
{
"id": "run the app",
"id": "run-the-app",
"exec": {
"component": "nodejs",
"commandLine": "nodemon app.js",
Expand All @@ -90,7 +90,7 @@ const (
}
},
{
"id": "run the app (debugging enabled)",
"id": "run-the-app-debugging-enabled",
"exec": {
"component": "nodejs",
"commandLine": "nodemon --inspect app.js",
Expand All @@ -101,14 +101,14 @@ const (
}
},
{
"id": "stop the app",
"id": "stop-the-app",
"exec": {
"component": "nodejs",
"commandLine": "node_server_pids=$(pgrep -fx '.*nodemon (--inspect )?app.js' | tr \"\\\\n\" \" \") && echo \"Stopping node server with PIDs: ${node_server_pids}\" && kill -15 ${node_server_pids} &>/dev/null && echo 'Done.'"
}
},
{
"id": "Attach remote debugger",
"id": "attach-remote-debugger",
"vscodeLaunch": {
"inlined": "{\n \"version\": \"0.2.0\",\n \"configurations\": [\n {\n \"type\": \"node\",\n \"request\": \"attach\",\n \"name\": \"Attach to Remote\",\n \"address\": \"localhost\",\n \"port\": 9229,\n \"localRoot\": \"${workspaceFolder}\",\n \"remoteRoot\": \"${workspaceFolder}\"\n }\n ]\n}\n"
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/devfile/parser/data/interface.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package data

import (
v1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
devfilepkg "github.com/devfile/api/pkg/devfile"
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
devfilepkg "github.com/devfile/api/v2/pkg/devfile"
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
)

Expand Down
Loading