Skip to content

Commit

Permalink
Adds comments and fixes variable names
Browse files Browse the repository at this point in the history
  • Loading branch information
@mik-dass
mik-dass committed Feb 3, 2021
1 parent e0f779e commit af19576
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 197 deletions.
21 changes: 5 additions & 16 deletions pkg/component/component.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,12 +589,9 @@ 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
Expand All @@ -610,7 +607,7 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf
client.Namespace = kClient.Namespace
}

var localConfig localConfigProvider.LocalConfigProvider
var configProvider localConfigProvider.LocalConfigProvider
if isS2I {
// if component exist then only call the update function
if cmpExist {
Expand All @@ -620,16 +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()
localConfig = &componentConfig
configProvider = &componentConfig
} else {
componentName = envSpecificInfo.GetName()
applicationName = envSpecificInfo.GetApplication()
localConfig = &envSpecificInfo
configProvider = &envSpecificInfo
}

isRouteSupported := false
Expand All @@ -641,13 +632,11 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf
urlClient := urlpkg.NewClient(urlpkg.ClientOptions{
OCClient: *client,
IsRouteSupported: isRouteSupported,
LocalConfigProvider: localConfig,
LocalConfigProvider: configProvider,
})

return urlpkg.Push(client, kClient, urlpkg.PushParameters{
ComponentName: componentName,
ApplicationName: applicationName,
LocalConfig: localConfig,
LocalConfig: configProvider,
URLClient: urlClient,
IsRouteSupported: isRouteSupported,
IsS2I: isS2I,
Expand Down
150 changes: 0 additions & 150 deletions pkg/devfile/adapters/docker/component/utils_test.go
View file
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
3 changes: 1 addition & 2 deletions pkg/odo/cli/component/status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,10 @@ func (so *StatusOptions) Complete(name string, cmd *cobra.Command, args []string

// If devfile is present
if so.isDevfile {
envSpecificInfo, err := envinfo.NewEnvSpecificInfo(so.componentContext)
so.EnvSpecificInfo, err = envinfo.NewEnvSpecificInfo(so.componentContext)
if err != nil {
return errors.Wrap(err, "unable to retrieve configuration information")
}
so.EnvSpecificInfo = envSpecificInfo
so.Context = genericclioptions.NewDevfileContext(cmd)

// Get the component name
Expand Down
12 changes: 10 additions & 2 deletions pkg/url/kubernetes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ import (
"k8s.io/klog"
)

// kubernetesClient contains information required for devfile based URL based operations
type kubernetesClient struct {
generic
isRouteSupported bool
client occlient.Client
}

// ListCluster lists both route and ingress based URLs from the cluster
func (k kubernetesClient) ListCluster() (URLList, error) {
func (k kubernetesClient) ListFromCluster() (URLList, error) {
labelSelector := fmt.Sprintf("%v=%v", componentlabels.ComponentLabel, k.componentName)
klog.V(4).Infof("Listing ingresses with label selector: %v", labelSelector)
ingresses, err := k.client.GetKubeClient().ListIngresses(labelSelector)
Expand Down Expand Up @@ -54,8 +55,9 @@ func (k kubernetesClient) ListCluster() (URLList, error) {

// List lists both route/ingress based URLs and local URLs with respective states
func (k kubernetesClient) List() (URLList, error) {
// get the URLs present on the cluster
clusterURLMap := make(map[string]URL)
clusterURLs, err := k.ListCluster()
clusterURLs, err := k.ListFromCluster()
if err != nil {
return URLList{}, errors.Wrap(err, "unable to list routes")
}
Expand All @@ -66,6 +68,7 @@ func (k kubernetesClient) List() (URLList, error) {

localMap := make(map[string]URL)
if k.localConfig != nil {
// get the URLs present on the localConfigProvider
localURLS := k.localConfig.ListURLs()
for _, url := range localURLS {
if !k.isRouteSupported && url.Kind == localConfigProvider.ROUTE {
Expand All @@ -78,6 +81,9 @@ func (k kubernetesClient) List() (URLList, error) {
}
}

// find the URLs which are present on the cluster but not on the localConfigProvider
// if not found on the localConfigProvider, mark them as 'StateTypeLocallyDeleted'
// else mark them as 'StateTypePushed'
var urls sortableURLs
for URLName, clusterURL := range clusterURLMap {
_, found := localMap[URLName]
Expand All @@ -92,6 +98,8 @@ func (k kubernetesClient) List() (URLList, error) {
}
}

// find the URLs which are present on the localConfigProvider but not on the cluster
// if not found on the cluster, mark them as 'StateTypeNotPushed'
for localName, localURL := range localMap {
_, remoteURLFound := clusterURLMap[localName]
if !remoteURLFound {
Expand Down
6 changes: 3 additions & 3 deletions pkg/url/kubernetes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,13 @@ func Test_kubernetesClient_ListCluster(t *testing.T) {
isRouteSupported: tt.fields.isRouteSupported,
client: *fkocclient,
}
got, err := k.ListCluster()
got, err := k.ListFromCluster()
if (err != nil) != tt.wantErr {
t.Errorf("ListCluster() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("ListFromCluster() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("ListCluster() error: %v", pretty.Compare(got, tt.want))
t.Errorf("ListFromCluster() error: %v", pretty.Compare(got, tt.want))
}
})
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/url/mock_Client.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions pkg/url/s2i.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import (
"k8s.io/klog"
)

// s2iClient contains information required for s2i based URL based operations
type s2iClient struct {
generic
client occlient.Client
}

func (s s2iClient) ListCluster() (URLList, error) {
// ListCluster lists route based URLs from the cluster
func (s s2iClient) ListFromCluster() (URLList, error) {
labelSelector := fmt.Sprintf("%v=%v", applabels.ApplicationLabel, s.localConfig.GetApplication())

if s.localConfig.GetName() != "" {
Expand All @@ -31,9 +33,6 @@ func (s s2iClient) ListCluster() (URLList, error) {

var urls []URL
for _, r := range routes {
if r.OwnerReferences != nil && r.OwnerReferences[0].Kind == "Ingress" {
continue
}
a := getMachineReadableFormat(r)
urls = append(urls, a)
}
Expand All @@ -42,16 +41,17 @@ func (s s2iClient) ListCluster() (URLList, error) {
return urlList, nil
}

// List lists both route based URLs and local URLs with respective states
func (s s2iClient) List() (URLList, error) {
var urls []URL

clusterUrls, err := s.ListCluster()
clusterUrls, err := s.ListFromCluster()
if err != nil {
return URLList{}, errors.Wrap(err, "unable to list route names")
}

for _, clusterURL := range clusterUrls.Items {
var found bool = false
var found = false
for _, configURL := range s.localConfig.ListURLs() {
localURL := ConvertConfigURL(configURL)
if localURL.Name == clusterURL.Name {
Expand Down
Loading

0 comments on commit af19576

Please sign in to comment.