Refactor & cleanup kclient generators code before migrating to devfil…

…e/parser (#4134) * Cleanup & Refactor kclient gen code 1 Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Clean up podTemplateSpec usage Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Move GetContainers to kclient Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Move certificate signature to secrets Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Clean up sync folder code Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * add GetService() Signed-off-by: Stephanie <yangcao@redhat.com> * add get route Signed-off-by: Stephanie <yangcao@redhat.com> * get adapter to use generator getService Signed-off-by: Stephanie <yangcao@redhat.com> * Move endpoint validations to validate pkg Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Update generators & validators test 1 Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Update unit tests 2 Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * add new unit tests Signed-off-by: Stephanie <yangcao@redhat.com> * add unit test for getPortExposure() Signed-off-by: Stephanie <yangcao@redhat.com> * modify unit test due to change to createOrUpdateComponent Signed-off-by: Stephanie <yangcao@redhat.com> * add unit tests for convert functions Signed-off-by: Stephanie <yangcao@redhat.com> * Update unit tests 3 - volume Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Add unit tests 4 - sync folder Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Update adapter unit test Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Rebase and update PR Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Move util func to generators pkg Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Remove illogical devfile 1.0 test case Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Unexport some generator funcs Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Clean up prestart code Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Move project vol mount out of the generators - updated Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * PR Review Feedback Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * Rename generate to get, add build config func Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> * PR feedback - func rename and loc Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com> Co-authored-by: Stephanie <yangcao@redhat.com>
redhat-developer · Nov 9, 2020 · fce3c77 · fce3c77
1 parent 30d9478
commit fce3c77
Show file tree

Hide file tree

Showing 44 changed files with 2,691 additions and 2,022 deletions.
diff --git a/pkg/component/component_full_description.go b/pkg/component/component_full_description.go
@@ -5,11 +5,11 @@ import (
 	"fmt"
 	"strings"
 
-	adaptersCommon "github.com/openshift/odo/pkg/devfile/adapters/common"
 	devfileParser "github.com/openshift/odo/pkg/devfile/parser"
 	"github.com/openshift/odo/pkg/devfile/parser/data/common"
 	"github.com/openshift/odo/pkg/envinfo"
 	"github.com/openshift/odo/pkg/kclient"
+	"github.com/openshift/odo/pkg/kclient/generator"
 
 	"github.com/openshift/odo/pkg/config"
 	"github.com/openshift/odo/pkg/log"
@@ -160,7 +160,7 @@ func NewComponentFullDescriptionFromClientAndLocalConfig(client *occlient.Client
 	var configProvider envinfo.LocalConfigProvider = localConfigInfo
 	if envInfo != nil {
 		configProvider = envInfo
-		components = adaptersCommon.GetDevfileContainerComponents(devfile.Data)
+		components = generator.GetDevfileContainerComponents(devfile.Data)
 	}
 	urls, err = urlpkg.ListIngressAndRoute(client, configProvider, components, componentName, routeSupported)
 	if err != nil {

diff --git a/pkg/devfile/adapters/common/types.go b/pkg/devfile/adapters/common/types.go
@@ -57,7 +57,7 @@ type SyncParameters struct {
 type ComponentInfo struct {
 	PodName       string
 	ContainerName string
-	SourceMount   string
+	SyncFolder    string
 }
 
 func (ci ComponentInfo) IsEmpty() bool {

diff --git a/pkg/devfile/adapters/common/utils.go b/pkg/devfile/adapters/common/utils.go
@@ -9,6 +9,7 @@ import (
 	devfileParser "github.com/openshift/odo/pkg/devfile/parser"
 	"github.com/openshift/odo/pkg/devfile/parser/data"
 	"github.com/openshift/odo/pkg/devfile/parser/data/common"
+	"github.com/openshift/odo/pkg/kclient/generator"
 )
 
 // PredefinedDevfileCommands encapsulates constants for predefined devfile commands
@@ -118,30 +119,6 @@ func GetBootstrapperImage() string {
 	return defaultBootstrapperImage
 }
 
-// GetDevfileContainerComponents iterates through the components in the devfile and returns a list of devfile container components
-func GetDevfileContainerComponents(data data.DevfileData) []common.DevfileComponent {
-	var components []common.DevfileComponent
-	// Only components with aliases are considered because without an alias commands cannot reference them
-	for _, comp := range data.GetAliasedComponents() {
-		if comp.Container != nil {
-			components = append(components, comp)
-		}
-	}
-	return components
-}
-
-// GetDevfileVolumeComponents iterates through the components in the devfile and returns a map of devfile volume components
-func GetDevfileVolumeComponents(data data.DevfileData) map[string]common.DevfileComponent {
-	volumeNameToVolumeComponent := make(map[string]common.DevfileComponent)
-	// Only components with aliases are considered because without an alias commands cannot reference them
-	for _, comp := range data.GetComponents() {
-		if comp.Volume != nil {
-			volumeNameToVolumeComponent[comp.Name] = comp
-		}
-	}
-	return volumeNameToVolumeComponent
-}
-
 // getCommandsByGroup gets commands by the group kind
 func getCommandsByGroup(data data.DevfileData, groupType common.DevfileCommandGroupType) []common.DevfileCommand {
 	var commands []common.DevfileCommand
@@ -168,8 +145,8 @@ func GetVolumeMountPath(volumeMount common.VolumeMount) string {
 
 // GetVolumes iterates through the components in the devfile and returns a map of container name to the devfile volumes
 func GetVolumes(devfileObj devfileParser.DevfileObj) map[string][]DevfileVolume {
-	containerComponents := GetDevfileContainerComponents(devfileObj.Data)
-	volumeNameToVolumeComponent := GetDevfileVolumeComponents(devfileObj.Data)
+	containerComponents := generator.GetDevfileContainerComponents(devfileObj.Data)
+	volumeComponents := generator.GetDevfileVolumeComponents(devfileObj.Data)
 
 	// containerNameToVolumes is a map of the Devfile container name to the Devfile container Volumes
 	containerNameToVolumes := make(map[string][]DevfileVolume)
@@ -178,9 +155,9 @@ func GetVolumes(devfileObj devfileParser.DevfileObj) map[string][]DevfileVolume
 			size := DefaultVolumeSize
 
 			// check if there is a volume component name against the container component volume mount name
-			if volumeComp, ok := volumeNameToVolumeComponent[volumeMount.Name]; ok {
-				// If there is a volume size mentioned in the devfile, use it
-				if len(volumeComp.Volume.Size) > 0 {
+			for _, volumeComp := range volumeComponents {
+				if volumeComp.Name == volumeMount.Name && len(volumeComp.Volume.Size) > 0 {
+					// If there is a volume size mentioned in the devfile, use it
 					size = volumeComp.Volume.Size
 				}
 			}

diff --git a/pkg/devfile/adapters/common/utils_test.go b/pkg/devfile/adapters/common/utils_test.go
@@ -11,114 +11,6 @@ import (
 	"github.com/openshift/odo/pkg/testingutil"
 )
 
-func TestGetDevfileContainerComponents(t *testing.T) {
-
-	tests := []struct {
-		name                 string
-		component            []versionsCommon.DevfileComponent
-		alias                []string
-		expectedMatchesCount int
-	}{
-		{
-			name:                 "Case 1: Invalid devfile",
-			component:            []versionsCommon.DevfileComponent{},
-			expectedMatchesCount: 0,
-		},
-		{
-			name:                 "Case 2: Valid devfile with wrong component type (Openshift)",
-			component:            []versionsCommon.DevfileComponent{{Openshift: &versionsCommon.Openshift{}}},
-			expectedMatchesCount: 0,
-		},
-		{
-			name:                 "Case 3: Valid devfile with wrong component type (Kubernetes)",
-			component:            []versionsCommon.DevfileComponent{{Kubernetes: &versionsCommon.Kubernetes{}}},
-			expectedMatchesCount: 0,
-		},
-
-		{
-			name:                 "Case 4 : Valid devfile with correct component type (Container)",
-			component:            []versionsCommon.DevfileComponent{testingutil.GetFakeContainerComponent("comp1"), testingutil.GetFakeContainerComponent("comp2")},
-			expectedMatchesCount: 2,
-		},
-
-		{
-			name:                 "Case 5: Valid devfile with correct component type (Container) without name",
-			component:            []versionsCommon.DevfileComponent{testingutil.GetFakeContainerComponent("comp1"), testingutil.GetFakeContainerComponent("")},
-			expectedMatchesCount: 1,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			devObj := devfileParser.DevfileObj{
-				Data: &testingutil.TestDevfileData{
-					Components: tt.component,
-				},
-			}
-
-			devfileComponents := GetDevfileContainerComponents(devObj.Data)
-
-			if len(devfileComponents) != tt.expectedMatchesCount {
-				t.Errorf("TestGetDevfileContainerComponents error: wrong number of components matched: expected %v, actual %v", tt.expectedMatchesCount, len(devfileComponents))
-			}
-		})
-	}
-
-}
-
-func TestGetDevfileVolumeComponents(t *testing.T) {
-
-	tests := []struct {
-		name                 string
-		component            []versionsCommon.DevfileComponent
-		alias                []string
-		expectedMatchesCount int
-	}{
-		{
-			name:                 "Case 1: Invalid devfile",
-			component:            []versionsCommon.DevfileComponent{},
-			expectedMatchesCount: 0,
-		},
-		{
-			name:                 "Case 2: Valid devfile with wrong component type (Openshift)",
-			component:            []versionsCommon.DevfileComponent{{Openshift: &versionsCommon.Openshift{}}},
-			expectedMatchesCount: 0,
-		},
-		{
-			name:                 "Case 3: Valid devfile with wrong component type (Kubernetes)",
-			component:            []versionsCommon.DevfileComponent{{Kubernetes: &versionsCommon.Kubernetes{}}},
-			expectedMatchesCount: 0,
-		},
-
-		{
-			name:                 "Case 4 : Valid devfile with wrong component type (Container)",
-			component:            []versionsCommon.DevfileComponent{testingutil.GetFakeContainerComponent("comp1"), testingutil.GetFakeContainerComponent("comp2")},
-			expectedMatchesCount: 0,
-		},
-
-		{
-			name:                 "Case 5: Valid devfile with correct component type (Volume)",
-			component:            []versionsCommon.DevfileComponent{testingutil.GetFakeContainerComponent("comp1"), testingutil.GetFakeVolumeComponent("myvol", "4Gi")},
-			expectedMatchesCount: 1,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			devObj := devfileParser.DevfileObj{
-				Data: &testingutil.TestDevfileData{
-					Components: tt.component,
-				},
-			}
-
-			devfileComponents := GetDevfileVolumeComponents(devObj.Data)
-
-			if len(devfileComponents) != tt.expectedMatchesCount {
-				t.Errorf("TestGetDevfileVolumeComponents error: wrong number of components matched: expected %v, actual %v", tt.expectedMatchesCount, len(devfileComponents))
-			}
-		})
-	}
-
-}
-
 func TestGetVolumes(t *testing.T) {
 
 	size := "4Gi"

diff --git a/pkg/devfile/adapters/docker/component/adapter.go b/pkg/devfile/adapters/docker/component/adapter.go
@@ -146,7 +146,7 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
 	// podChanged is defaulted to false, since docker volume is always present even if container goes down
 	compInfo := common.ComponentInfo{
 		ContainerName: containerID,
-		SourceMount:   sourceMount,
+		SyncFolder:    sourceMount,
 	}
 	syncParams := common.SyncParameters{
 		PushParams:      parameters,

diff --git a/pkg/devfile/adapters/docker/component/utils.go b/pkg/devfile/adapters/docker/component/utils.go
@@ -2,11 +2,12 @@ package component
 
 import (
 	"fmt"
+	"os"
+	"strconv"
+
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/mount"
 	"github.com/docker/go-connections/nat"
-	"os"
-	"strconv"
 
 	"github.com/pkg/errors"
 	"k8s.io/klog"
@@ -16,6 +17,7 @@ import (
 	"github.com/openshift/odo/pkg/devfile/adapters/docker/utils"
 	versionsCommon "github.com/openshift/odo/pkg/devfile/parser/data/common"
 	"github.com/openshift/odo/pkg/envinfo"
+	"github.com/openshift/odo/pkg/kclient/generator"
 	"github.com/openshift/odo/pkg/lclient"
 	"github.com/openshift/odo/pkg/log"
 )
@@ -29,7 +31,7 @@ func (a Adapter) createComponent() (err error) {
 
 	log.Infof("\nCreating Docker resources for component %s", a.ComponentName)
 
-	containerComponents := common.GetDevfileContainerComponents(a.Devfile.Data)
+	containerComponents := generator.GetDevfileContainerComponents(a.Devfile.Data)
 	if len(containerComponents) == 0 {
 		return fmt.Errorf("no valid components found in the devfile")
 	}
@@ -72,7 +74,7 @@ func (a Adapter) updateComponent() (componentExists bool, err error) {
 	stoAdapter := storage.New(a.AdapterContext, a.Client)
 	err = stoAdapter.Create(a.uniqueStorage)
 
-	containerComponents := common.GetDevfileContainerComponents(a.Devfile.Data)
+	containerComponents := generator.GetDevfileContainerComponents(a.Devfile.Data)
 	if len(containerComponents) == 0 {
 		return componentExists, fmt.Errorf("no valid components found in the devfile")
 	}

diff --git a/pkg/devfile/adapters/docker/utils/utils.go b/pkg/devfile/adapters/docker/utils/utils.go
@@ -9,6 +9,7 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/mount"
+	"github.com/openshift/odo/pkg/kclient/generator"
 
 	adaptersCommon "github.com/openshift/odo/pkg/devfile/adapters/common"
 	"github.com/openshift/odo/pkg/devfile/parser/data"
@@ -37,7 +38,7 @@ func ComponentExists(client lclient.Client, data data.DevfileData, name string)
 		return false, errors.Wrapf(err, "unable to get the containers for component %s", name)
 	}
 
-	containerComponents := adaptersCommon.GetDevfileContainerComponents(data)
+	containerComponents := generator.GetDevfileContainerComponents(data)
 
 	var componentExists bool
 	if len(containers) == 0 {