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.

Sign up for GitHub

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

Jump to bottom

Refactors url List() to use URL Client #4366

Merged
merged 3 commits into from
Feb 3, 2021
Merged

Refactors url List() to use URL Client #4366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0f779e
Refactors url List() to use URL Client.
mik-dass Jan 15, 2021
af19576
Adds comments and fixes variable names
mik-dass Jan 27, 2021
0f16b05
Fixes occlient error while creating new URL client
mik-dass Jan 29, 2021
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
32 changes: 14 additions & 18 deletions pkg/component/component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/pkg/errors"
"k8s.io/klog"

devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
applabels "github.com/openshift/odo/pkg/application/labels"
"github.com/openshift/odo/pkg/catalog"
componentlabels "github.com/openshift/odo/pkg/component/labels"
Expand Down Expand Up @@ -590,16 +589,13 @@ func ensureAndLogProperResourceUsage(resourceMin, resourceMax *string, resourceN
// Parameters:
// client: occlient instance
// kClient: kclient instance
// appName: Name of application of which the component is a part
// componentName: Name of the component which is being patched with config
// componentConfig: Component configuration
// envSpecificInfo: Component environment specific information, available if uses devfile
// cmpExist: true if components exists in the cluster
// endpointMap: value is devfile endpoint entry, key is the TargetPort for each endpoint entry
// isS2I: Legacy option. Set as true if you want to use the old S2I method as it differentiates slightly.
// Returns:
// err: Errors if any else nil
func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConfig config.LocalConfigInfo, envSpecificInfo envinfo.EnvSpecificInfo, stdout io.Writer, cmpExist bool, containerComponents []devfilev1.Component, isS2I bool) (err error) {
func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConfig config.LocalConfigInfo, envSpecificInfo envinfo.EnvSpecificInfo, stdout io.Writer, cmpExist bool, isS2I bool) (err error) {

if client == nil {
var err error
Expand All @@ -611,6 +607,7 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf
client.Namespace = kClient.Namespace
}

var configProvider localConfigProvider.LocalConfigProvider
if isS2I {
// if component exist then only call the update function
if cmpExist {
Expand All @@ -620,14 +617,10 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf
}
}

var componentName string
var applicationName string
if isS2I || kClient == nil {
componentName = componentConfig.GetName()
applicationName = componentConfig.GetApplication()
configProvider = &componentConfig
} else {
componentName = envSpecificInfo.GetName()
applicationName = envSpecificInfo.GetApplication()
configProvider = &envSpecificInfo
}

isRouteSupported := false
Expand All @@ -636,14 +629,17 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf
isRouteSupported = false
}

return urlpkg.Push(client, kClient, urlpkg.PushParameters{
ComponentName: componentName,
ApplicationName: applicationName,
ConfigURLs: componentConfig.ListURLs(),
EnvURLS: envSpecificInfo.ListURLs(),
urlClient := urlpkg.NewClient(urlpkg.ClientOptions{
OCClient: *client,
IsRouteSupported: isRouteSupported,
ContainerComponents: containerComponents,
IsS2I: isS2I,
LocalConfigProvider: configProvider,
})

return urlpkg.Push(client, kClient, urlpkg.PushParameters{
LocalConfig: configProvider,
URLClient: urlClient,
IsRouteSupported: isRouteSupported,
IsS2I: isS2I,
})
}

Expand Down
11 changes: 7 additions & 4 deletions pkg/component/component_full_description.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/openshift/odo/pkg/localConfigProvider"
"strings"

devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
devfileParser "github.com/devfile/library/pkg/devfile/parser"
"github.com/openshift/odo/pkg/envinfo"
"github.com/openshift/odo/pkg/kclient"
Expand Down Expand Up @@ -140,18 +139,22 @@ func NewComponentFullDescriptionFromClientAndLocalConfig(client *occlient.Client
if e != nil {
return cfd, e
}
var components []devfilev1.Component

var configProvider localConfigProvider.LocalConfigProvider
if envInfo != nil {
envInfo.SetDevfileObj(devfile)
configProvider = envInfo
components = devfile.Data.GetDevfileContainerComponents()
} else {
configProvider = localConfigInfo
}

urls, err = urlpkg.ListIngressAndRoute(client, configProvider, components, componentName, routeSupported)
urlClient := urlpkg.NewClient(urlpkg.ClientOptions{
LocalConfigProvider: configProvider,
OCClient: *client,
IsRouteSupported: routeSupported,
})

urls, err = urlClient.List()
if err != nil {
log.Warningf("URLs couldn't not be retrieved: %v", err)
}
Expand Down
13 changes: 12 additions & 1 deletion pkg/config/url.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,18 @@ func (lc *LocalConfig) ListURLs() []localConfigProvider.LocalURL {
if lc.componentSettings.URL == nil {
return []localConfigProvider.LocalURL{}
}
return *lc.componentSettings.URL
var resultURLs []localConfigProvider.LocalURL
for _, url := range *lc.componentSettings.URL {
resultURLs = append(resultURLs, localConfigProvider.LocalURL{
Name: url.Name,
Port: url.Port,
Secure: url.Secure,
Host: url.Host,
Path: "/",
Kind: localConfigProvider.ROUTE,
})
}
return resultURLs
}

// DeleteURL is used to delete config from local odo config
Expand Down
150 changes: 0 additions & 150 deletions pkg/devfile/adapters/docker/component/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
package component

import (
"github.com/openshift/odo/pkg/localConfigProvider"
"reflect"
"strings"
"testing"

"github.com/docker/go-connections/nat"

devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
devfileParser "github.com/devfile/library/pkg/devfile/parser"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/mount"
adaptersCommon "github.com/openshift/odo/pkg/devfile/adapters/common"
"github.com/openshift/odo/pkg/envinfo"
"github.com/openshift/odo/pkg/lclient"
"github.com/openshift/odo/pkg/testingutil"
)
Expand Down Expand Up @@ -322,152 +318,6 @@ func TestStartContainer(t *testing.T) {

}

func TestGenerateAndGetHostConfig(t *testing.T) {
fakeClient := lclient.FakeNew()
testComponentName := "test"

endpointName := []string{"8080/tcp", "9090/tcp", "9080/tcp"}
var endpointPort = []int{8080, 9090, 9080}
var expectPortNameMapping = map[nat.Port]string{
nat.Port("8080/tcp"): "url1",
nat.Port("9090/tcp"): "url2",
nat.Port("9080/tcp"): "url3",
}

tests := []struct {
name string
urlValue []localConfigProvider.LocalURL
expectResult nat.PortMap
client *lclient.Client
endpoints []devfilev1.Endpoint
}{
{
name: "Case 1: no port mappings",
urlValue: []localConfigProvider.LocalURL{},
expectResult: nil,
client: fakeClient,
endpoints: []devfilev1.Endpoint{},
},
{
name: "Case 2: only one port mapping",
urlValue: []localConfigProvider.LocalURL{
{Name: "url1", Port: 8080, ExposedPort: 65432},
},
expectResult: nat.PortMap{
"8080/tcp": []nat.PortBinding{
{
HostIP: LocalhostIP,
HostPort: "65432",
},
},
},
client: fakeClient,
endpoints: []devfilev1.Endpoint{
{
Name: endpointName[0],
TargetPort: endpointPort[0],
},
},
},
{
name: "Case 3: multiple port mappings",
urlValue: []localConfigProvider.LocalURL{
{Name: "url1", Port: 8080, ExposedPort: 65432},
{Name: "url2", Port: 9090, ExposedPort: 54321},
{Name: "url3", Port: 9080, ExposedPort: 45678},
},
expectResult: nat.PortMap{
"8080/tcp": []nat.PortBinding{
{
HostIP: LocalhostIP,
HostPort: "65432",
},
},
"9090/tcp": []nat.PortBinding{
{
HostIP: LocalhostIP,
HostPort: "54321",
},
},
"9080/tcp": []nat.PortBinding{
{
HostIP: LocalhostIP,
HostPort: "45678",
},
},
},
client: fakeClient,
endpoints: []devfilev1.Endpoint{
{
Name: endpointName[0],
TargetPort: endpointPort[0],
},
{
Name: endpointName[1],
TargetPort: endpointPort[1],
},
{
Name: endpointName[2],
TargetPort: endpointPort[2],
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

devObj := devfileParser.DevfileObj{
Data: &testingutil.TestDevfileData{
Components: []devfilev1.Component{},
},
}

adapterCtx := adaptersCommon.AdapterContext{
ComponentName: testComponentName,
Devfile: devObj,
}

esi, err := envinfo.NewEnvSpecificInfo("")
if err != nil {
t.Error(err)
}
for _, url := range tt.urlValue {
err = esi.SetConfiguration("URL", url)
if err != nil {
t.Error(err)
}
}
componentAdapter := New(adapterCtx, *tt.client)
hostConfig, portURLNameMapping, err := componentAdapter.generateAndGetHostConfig(tt.endpoints)
if err != nil {
t.Error(err)
}

if len(hostConfig.PortBindings) != len(tt.expectResult) {
t.Errorf("host config PortBindings length mismatch: actual value %v, expected value %v", len(hostConfig.PortBindings), len(tt.expectResult))
}
if len(hostConfig.PortBindings) != 0 {
for key, value := range hostConfig.PortBindings {
if tt.expectResult[key][0].HostIP != value[0].HostIP || tt.expectResult[key][0].HostPort != value[0].HostPort {
t.Errorf("host config PortBindings mismatch: actual value %v, expected value %v", hostConfig.PortBindings, tt.expectResult)
}
}
}
if len(portURLNameMapping) != 0 {
for key, value := range portURLNameMapping {
if expectPortNameMapping[key] != value {
t.Errorf("port and urlName mapping mismatch for port %v: actual value %v, expected value %v", key, value, expectPortNameMapping[key])
}
}
}
err = esi.DeleteEnvInfoFile()
if err != nil {
t.Error(err)
}
})
}
}

func TestExecDevfile(t *testing.T) {

testComponentName := "test"
Expand Down
4 changes: 2 additions & 2 deletions pkg/devfile/adapters/kubernetes/component/adapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
if currentMode != previousMode {
parameters.RunModeChanged = true
}
containerComponents := a.Devfile.Data.GetDevfileContainerComponents()

err = a.createOrUpdateComponent(componentExists, parameters.EnvSpecificInfo)
if err != nil {
Expand All @@ -177,7 +176,8 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
return errors.Wrapf(err, "unable to get pod for component %s", a.ComponentName)
}

err = component.ApplyConfig(nil, &a.Client, config.LocalConfigInfo{}, parameters.EnvSpecificInfo, color.Output, componentExists, containerComponents, false)
parameters.EnvSpecificInfo.SetDevfileObj(a.Devfile)
err = component.ApplyConfig(nil, &a.Client, config.LocalConfigInfo{}, parameters.EnvSpecificInfo, color.Output, componentExists, false)
if err != nil {
odoutil.LogErrorAndExit(err, "Failed to update config to component deployed.")
}
Expand Down
Loading