Refactors url List() to use URL Client (redhat-developer#4366)

* Refactors url List() to use URL Client. It also adds ListCluster() to get urls from the cluster. Signed-off-by: mik-dass <mrinald7@gmail.com> * Adds comments and fixes variable names * Fixes occlient error while creating new URL client
kadel · Feb 4, 2021 · f3d4cff · f3d4cff
1 parent 9eaee42
commit f3d4cff
Show file tree

Hide file tree

Showing 20 changed files with 1,515 additions and 1,967 deletions.
diff --git a/pkg/component/component.go b/pkg/component/component.go
@@ -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"
@@ -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
@@ -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 {
@@ -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
@@ -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,
 	})
 }
 

diff --git a/pkg/component/component_full_description.go b/pkg/component/component_full_description.go
@@ -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"
@@ -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)
 	}

diff --git a/pkg/config/url.go b/pkg/config/url.go
@@ -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

diff --git a/pkg/devfile/adapters/docker/component/utils_test.go b/pkg/devfile/adapters/docker/component/utils_test.go
@@ -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"
 )
@@ -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"

diff --git a/pkg/devfile/adapters/kubernetes/component/adapter.go b/pkg/devfile/adapters/kubernetes/component/adapter.go
@@ -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 {
@@ -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.")
 	}