From 64c0798a578798e8c7869f092a1512e4484395b7 Mon Sep 17 00:00:00 2001 From: mustard Date: Fri, 14 Oct 2022 18:49:22 +0000 Subject: [PATCH 1/4] [supervisor] respond PortsStatus with order --- .../gitpod-protocol/go/gitpod-service.go | 1 + .../supervisor/pkg/ports/ports-config.go | 6 ++++- components/supervisor/pkg/ports/ports.go | 23 ++++++++++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/components/gitpod-protocol/go/gitpod-service.go b/components/gitpod-protocol/go/gitpod-service.go index 1b037f2a4cdc40..7cb8355f24a2e2 100644 --- a/components/gitpod-protocol/go/gitpod-service.go +++ b/components/gitpod-protocol/go/gitpod-service.go @@ -1756,6 +1756,7 @@ type PortConfig struct { Visibility string `json:"visibility,omitempty"` Description string `json:"description,omitempty"` Name string `json:"name,omitempty"` + Sort uint32 `json:"sort,omitempty"` } // TaskConfig is the TaskConfig message type diff --git a/components/supervisor/pkg/ports/ports-config.go b/components/supervisor/pkg/ports/ports-config.go index 50faa79ab317e3..9118e1e803dbb5 100644 --- a/components/supervisor/pkg/ports/ports-config.go +++ b/components/supervisor/pkg/ports/ports-config.go @@ -21,6 +21,7 @@ type RangeConfig struct { *gitpod.PortsItems Start uint32 End uint32 + Sort uint32 } // Configs provides access to port configurations. @@ -79,6 +80,7 @@ func (configs *Configs) Get(port uint32) (*gitpod.PortConfig, ConfigKind, bool) Visibility: rangeConfig.Visibility, Description: rangeConfig.Description, Name: rangeConfig.Name, + Sort: rangeConfig.Sort, }, RangeConfigKind, true } } @@ -184,7 +186,7 @@ func parseWorkspaceConfigs(ports []*gitpod.PortConfig) (portConfigs map[uint32]* } func parseInstanceConfigs(ports []*gitpod.PortsItems) (portConfigs map[uint32]*gitpod.PortConfig, rangeConfigs []*RangeConfig) { - for _, config := range ports { + for index, config := range ports { if config == nil { continue } @@ -204,6 +206,7 @@ func parseInstanceConfigs(ports []*gitpod.PortsItems) (portConfigs map[uint32]*g Visibility: config.Visibility, Description: config.Description, Name: config.Name, + Sort: uint32(index), } } continue @@ -224,6 +227,7 @@ func parseInstanceConfigs(ports []*gitpod.PortsItems) (portConfigs map[uint32]*g PortsItems: config, Start: uint32(start), End: uint32(end), + Sort: uint32(index), }) } return portConfigs, rangeConfigs diff --git a/components/supervisor/pkg/ports/ports.go b/components/supervisor/pkg/ports/ports.go index 4e9f6ebc889cba..1570f51af7eaae 100644 --- a/components/supervisor/pkg/ports/ports.go +++ b/components/supervisor/pkg/ports/ports.go @@ -740,8 +740,29 @@ func (pm *Manager) Subscribe() (*Subscription, error) { func (pm *Manager) getStatus() []*api.PortsStatus { res := make([]*api.PortsStatus, 0, len(pm.state)) for port := range pm.state { - res = append(res, pm.getPortStatus(port)) + portStatus := pm.getPortStatus(port) + // Filter out ports that not served and not inside `.gitpod.yml` + if _, _, ok := pm.configs.Get(port); !ok && !portStatus.Served { + continue + } + res = append(res, portStatus) } + sort.SliceStable(res, func(i, j int) bool { + // Max number of port 65536 + score1 := 100000 + res[i].LocalPort + score2 := 100000 + res[j].LocalPort + if c, _, ok := pm.configs.Get(res[i].LocalPort); ok { + score1 = c.Sort + } + if c, _, ok := pm.configs.Get(res[j].LocalPort); ok { + score2 = c.Sort + } + if score1 != score2 { + return score1 < score2 + } + // Ranged ports + return res[i].LocalPort < res[j].LocalPort + }) return res } From 137363e6f828f7d52ac943e88039450152618390 Mon Sep 17 00:00:00 2001 From: mustard Date: Fri, 14 Oct 2022 18:50:00 +0000 Subject: [PATCH 2/4] [supervisor] add test cases for ports order --- components/supervisor/pkg/ports/ports_test.go | 108 ++++++++++++++++-- 1 file changed, 98 insertions(+), 10 deletions(-) diff --git a/components/supervisor/pkg/ports/ports_test.go b/components/supervisor/pkg/ports/ports_test.go index 509eef1644d078..7156b236ec8686 100644 --- a/components/supervisor/pkg/ports/ports_test.go +++ b/components/supervisor/pkg/ports/ports_test.go @@ -12,14 +12,13 @@ import ( "testing" "time" + "github.com/gitpod-io/gitpod/common-go/log" + gitpod "github.com/gitpod-io/gitpod/gitpod-protocol" + "github.com/gitpod-io/gitpod/supervisor/api" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/sirupsen/logrus" "golang.org/x/sync/errgroup" - - "github.com/gitpod-io/gitpod/common-go/log" - gitpod "github.com/gitpod-io/gitpod/gitpod-protocol" - "github.com/gitpod-io/gitpod/supervisor/api" ) func TestPortsUpdateState(t *testing.T) { @@ -64,8 +63,8 @@ func TestPortsUpdateState(t *testing.T) { []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private}}, []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}}, []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}, {LocalPort: 60000, Served: true}}, - []*api.PortsStatus{{LocalPort: 8080, Served: false, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}, {LocalPort: 60000, Served: true}}, - []*api.PortsStatus{{LocalPort: 8080, Served: false, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}}, + []*api.PortsStatus{{LocalPort: 60000, Served: true}}, + {}, }, }, { @@ -86,15 +85,18 @@ func TestPortsUpdateState(t *testing.T) { { Desc: "basic port publically exposed", Changes: []Change{ - {Exposed: []ExposedPort{{LocalPort: 8080, Public: false, URL: "foobar"}}}, - {Exposed: []ExposedPort{{LocalPort: 8080, Public: true, URL: "foobar"}}}, {Served: []ServedPort{{Port: 8080}}}, + {Exposed: []ExposedPort{{LocalPort: 8080, Public: true, URL: "foobar"}}}, + {Exposed: []ExposedPort{{LocalPort: 8080, Public: false, URL: "foobar"}}}, + }, + ExpectedExposure: ExposureExpectation{ + {LocalPort: 8080}, }, ExpectedUpdates: UpdateExpectation{ {}, - []*api.PortsStatus{{LocalPort: 8080, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_private, Url: "foobar", OnExposed: api.OnPortExposedAction_notify_private}}}, - []*api.PortsStatus{{LocalPort: 8080, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_public, Url: "foobar", OnExposed: api.OnPortExposedAction_notify_private}}}, + []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private}}, []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_public, Url: "foobar", OnExposed: api.OnPortExposedAction_notify_private}}}, + []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_private, Url: "foobar", OnExposed: api.OnPortExposedAction_notify_private}}}, }, }, { @@ -746,3 +748,89 @@ func TestPortsConcurrentSubscribe(t *testing.T) { wg.Wait() } + +func TestManager_getStatus(t *testing.T) { + type fields struct { + orderInYaml []any + state []uint32 + } + tests := []struct { + name string + fields fields + want []uint32 + }{ + { + name: "happy path", + fields: fields{ + // The port number (e.g. 1337) or range (e.g. 3000-3999) to expose. + orderInYaml: []any{1002, 1000, "3000-3999", 1001}, + state: []uint32{1000, 1001, 1002, 3003, 3001, 3002, 4002, 4000, 5000, 5005}, + }, + want: []uint32{1002, 1000, 3001, 3002, 3003, 1001, 4000, 4002, 5000, 5005}, + }, + { + name: "order for ranged ports and inside ranged order by number ASC", + fields: fields{ + orderInYaml: []any{1002, "3000-3999", 1009, "4000-4999"}, + state: []uint32{5000, 1000, 1009, 4000, 4001, 3000, 3009}, + }, + want: []uint32{3000, 3009, 1009, 4000, 4001, 1000, 5000}, + }, + { + name: "served ports order by number ASC", + fields: fields{ + orderInYaml: []any{}, + state: []uint32{4000, 4003, 4007, 4001, 4006}, + }, + want: []uint32{4000, 4001, 4003, 4006, 4007}, + }, + + // It will not works because we do not `Run` ports Manger + // As ports Manger will autoExpose those ports (but not ranged port) in yaml + // and they will exists in state + // { + // name: "not ignore ports that not served but exists in yaml", + // fields: fields{ + // orderInYaml: []any{1002, 1000, 1001}, + // state: []uint32{}, + // }, + // want: []uint32{1002, 1000, 1001}, + // }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + state := make(map[uint32]*managedPort) + for _, port := range tt.fields.state { + state[port] = &managedPort{ + Served: true, + LocalhostPort: port, + TunneledTargetPort: port, + TunneledClients: map[string]uint32{}, + } + } + portsItems := []*gitpod.PortsItems{} + for _, port := range tt.fields.orderInYaml { + portsItems = append(portsItems, &gitpod.PortsItems{Port: port}) + } + portsConfig, rangeConfig := parseInstanceConfigs(portsItems) + pm := &Manager{ + configs: &Configs{ + instancePortConfigs: portsConfig, + instanceRangeConfigs: rangeConfig, + }, + state: state, + } + got := pm.getStatus() + if len(got) != len(tt.want) { + t.Errorf("Manager.getStatus() length = %v, want %v", len(got), len(tt.want)) + } + gotPorts := []uint32{} + for _, g := range got { + gotPorts = append(gotPorts, g.LocalPort) + } + if diff := cmp.Diff(gotPorts, tt.want); diff != "" { + t.Errorf("unexpected exposures (-want +got):\n%s", diff) + } + }) + } +} From 54f0678ea56bbe9904f900c1a87712f351b0034a Mon Sep 17 00:00:00 2001 From: mustard Date: Wed, 19 Oct 2022 08:31:05 +0000 Subject: [PATCH 3/4] DEBUG: update code image to use order from supervisor --- WORKSPACE.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WORKSPACE.yaml b/WORKSPACE.yaml index 3fe881eac0c567..b2b83964f3ab59 100644 --- a/WORKSPACE.yaml +++ b/WORKSPACE.yaml @@ -7,7 +7,7 @@ defaultArgs: publishToNPM: true publishToJBMarketplace: true localAppVersion: unknown - codeCommit: 239fa626a44652cebf293a994a3330e90e873daa + codeCommit: f604b771695de9ac3744a1aec9d32112c8873843 codeQuality: stable noVerifyJBPlugin: false intellijDownloadUrl: "https://download.jetbrains.com/idea/ideaIU-2022.2.3.tar.gz" From 25692160e92f4f414153ed3762094eb8ed7ebfb5 Mon Sep 17 00:00:00 2001 From: mustard Date: Wed, 19 Oct 2022 10:14:24 +0000 Subject: [PATCH 4/4] [gp-cli] remove sort in gp ports list cmd --- components/gitpod-cli/cmd/ports-list.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/components/gitpod-cli/cmd/ports-list.go b/components/gitpod-cli/cmd/ports-list.go index 55bf6c2c065e87..100b24d0af6da4 100644 --- a/components/gitpod-cli/cmd/ports-list.go +++ b/components/gitpod-cli/cmd/ports-list.go @@ -8,7 +8,6 @@ import ( "context" "fmt" "os" - "sort" "time" supervisor_helper "github.com/gitpod-io/gitpod/gitpod-cli/pkg/supervisor-helper" @@ -38,10 +37,6 @@ var listPortsCmd = &cobra.Command{ return } - sort.Slice(ports, func(i, j int) bool { - return int(ports[i].LocalPort) < int(ports[j].LocalPort) - }) - table := tablewriter.NewWriter(os.Stdout) table.SetHeader([]string{"Port", "Status", "URL", "Name & Description"}) table.SetBorders(tablewriter.Border{Left: true, Top: false, Right: true, Bottom: false})