Refactors storage List() to use Storage Client. It also adds ListFrom…

…Cluster() to get storage from the cluster.

pkg/component/component_full_description.go

@@ -50,40 +50,27 @@ func (cfd *ComponentFullDescription) copyFromComponentDesc(component *Component)
 }
 
 // loadStoragesFromClientAndLocalConfig collects information about storages both locally and from the cluster.
-func (cfd *ComponentFullDescription) loadStoragesFromClientAndLocalConfig(client *occlient.Client, kClient *kclient.Client, envinfo *envinfo.EnvSpecificInfo, localConfigInfo *config.LocalConfigInfo, componentName string, applicationName string, componentDesc *Component) error {
+func (cfd *ComponentFullDescription) loadStoragesFromClientAndLocalConfig(client *occlient.Client, kClient *kclient.Client, configProvider localConfigProvider.LocalConfigProvider, componentName string, applicationName string, componentDesc *Component) error {
  var storages storage.StorageList
  var err error
 
- isDevfile := envinfo != nil
- var devfile devfileParser.DevfileObj
- if isDevfile {
- devfile, err = devfileParser.Parse(envinfo.GetDevfilePath())
- if err != nil {
- return err
- }
- }
-
  // if component is pushed call ListWithState which gets storages from localconfig and cluster
  // this result is already in mc readable form
  if componentDesc.Status.State == StateTypePushed {
- if isDevfile {
- storages, err = storage.DevfileList(kClient, devfile.Data, envinfo.GetName())
- } else {
- storages, err = storage.ListStorageWithState(client, localConfigInfo, componentName, applicationName)
- }
+ storageClient := storage.NewClient(storage.ClientOptions{
+ OCClient: *client,
+ LocalConfigProvider: configProvider,
+ })
+
+ storages, err = storageClient.List()
  if err != nil {
  return err
  }
  } else {
  // otherwise simply fetch storagelist locally
- if isDevfile {
- storages = storage.GetLocalDevfileStorage(devfile.Data)
- storages = storage.GetMachineReadableFormatForList(storages.Items)
- } else {
- storageLocal := localConfigInfo.ListStorage()
- // convert to machine readable format
- storages = storage.ConvertListLocalToMachine(storageLocal)
- }
+ storageLocal := configProvider.ListStorage()
+ // convert to machine readable format
+ storages = storage.ConvertListLocalToMachine(storageLocal)
  }
  cfd.Spec.Storage = storages
  return nil
@@ -154,18 +141,23 @@ func NewComponentFullDescriptionFromClientAndLocalConfig(client *occlient.Client
  return cfd, e
  }
  var components []devfilev1.Component
- var configProvider localConfigProvider.LocalConfigProvider = localConfigInfo
+
+ 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)
  if err != nil {
  log.Warningf("URLs couldn't not be retrieved: %v", err)
  }
  cfd.Spec.URL = urls
 
- err = cfd.loadStoragesFromClientAndLocalConfig(client, kClient, envInfo, localConfigInfo, componentName, applicationName, &componentDesc)
+ err = cfd.loadStoragesFromClientAndLocalConfig(client, kClient, configProvider, componentName, applicationName, &componentDesc)
  if err != nil {
  return cfd, err
  }

pkg/config/config.go

@@ -438,6 +438,19 @@ func (lc *LocalConfig) GetDebugPort() int {
  return util.GetIntOrDefault(lc.componentSettings.DebugPort, DefaultDebugPort)
 }
 
+// GetContainers returns the Container components from the config
+// returns empty list if nil
+func (lc *LocalConfig) GetContainers() []localConfigProvider.LocalContainer {
+ if lc.GetName() == "" {
+ return []localConfigProvider.LocalContainer{}
+ }
+ return []localConfigProvider.LocalContainer{
+ {
+ Name: lc.GetName(),
+ },
+ }
+}
+
 // GetIgnore returns the Ignore, returns default if nil
 func (lc *LocalConfig) GetIgnore() bool {
  return util.GetBoolOrDefault(lc.componentSettings.Ignore, false)

pkg/envinfo/envinfo.go

@@ -353,6 +353,22 @@ func (ei *EnvInfo) GetDebugPort() int {
  return *ei.componentSettings.DebugPort
 }
 
+// GetContainers returns the Container components from the devfile
+// returns empty list if nil
+func (ei *EnvInfo) GetContainers() []localConfigProvider.LocalContainer {
+ var localContainers []localConfigProvider.LocalContainer
+
+ for _, component := range ei.devfileObj.Data.GetComponents() {
+ if component.Container == nil {
+ continue
+ }
+ localContainers = append(localContainers, localConfigProvider.LocalContainer{
+ Name: component.Name,
+ })
+ }
+ return localContainers
+}
+
 // IsUserCreatedDevfile returns the UserCreatedDevfile
 func (ei *EnvInfo) IsUserCreatedDevfile() bool {
  return ei.componentSettings.UserCreatedDevfile

pkg/localConfigProvider/localConfigProvider.go

@@ -45,13 +45,19 @@ type LocalStorage struct {
  Container string `yaml:"-" json:"-"`
 }
 
+// LocalContainer holds the container related information
+type LocalContainer struct {
+ Name string `yaml:"Name" json:"Name"`
+}
+
 // LocalConfigProvider is an interface which all local config providers need to implement
 // currently for openshift there is localConfigInfo and for devfile its EnvInfo + devfile.yaml
 type LocalConfigProvider interface {
  GetApplication() string
  GetName() string
  GetNamespace() string
  GetDebugPort() int
+ GetContainers() []LocalContainer
 
  GetURL(name string) *LocalURL
  CompleteURL(url *LocalURL) error

pkg/odo/cli/storage/list.go

@@ -3,18 +3,12 @@ package storage
 import (
  "fmt"
  "os"
- "path/filepath"
  "text/tabwriter"
 
- devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
- "github.com/devfile/library/pkg/devfile"
- "github.com/devfile/library/pkg/devfile/parser"
- "github.com/openshift/odo/pkg/devfile/validate"
- "github.com/openshift/odo/pkg/odo/cli/component"
- odoutil "github.com/openshift/odo/pkg/util"
-
+ "github.com/openshift/odo/pkg/localConfigProvider"
  "github.com/openshift/odo/pkg/log"
  "github.com/openshift/odo/pkg/machineoutput"
+ "github.com/openshift/odo/pkg/odo/cli/component"
  "github.com/openshift/odo/pkg/odo/util/completion"
  "github.com/openshift/odo/pkg/storage"
 
@@ -36,70 +30,56 @@ var (
  `)
 )
 
-type StorageListOptions struct {
+type ListOptions struct {
  componentContext string
  *genericclioptions.Context
 
- isDevfile bool
- parser.DevfileObj
+ client storage.Client
 }
 
-// NewStorageListOptions creates a new StorageListOptions instance
-func NewStorageListOptions() *StorageListOptions {
- return &StorageListOptions{}
+// NewStorageListOptions creates a new ListOptions instance
+func NewStorageListOptions() *ListOptions {
+ return &ListOptions{}
 }
 
-// Complete completes StorageListOptions after they've been created
-func (o *StorageListOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
- devFilePath := filepath.Join(o.componentContext, component.DevfilePath)
- o.isDevfile = odoutil.CheckPathExists(devFilePath)
- if o.isDevfile {
- o.Context = genericclioptions.NewDevfileContext(cmd)
+// Complete completes ListOptions after they've been created
+func (o *ListOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
+ o.Context, err = genericclioptions.New(genericclioptions.CreateParameters{
+ Cmd: cmd,
+ DevfilePath: component.DevfilePath,
+ ComponentContext: o.componentContext,
+ })
 
- o.DevfileObj, err = devfile.ParseAndValidate(devFilePath)
- if err != nil {
- return err
- }
- err = validate.ValidateDevfileData(o.DevfileObj.Data)
- if err != nil {
- return err
- }
- } else {
- // this also initializes the context as well
- o.Context = genericclioptions.NewContext(cmd)
+ if err != nil {
+ return err
  }
+
+ o.client = storage.NewClient(storage.ClientOptions{
+ LocalConfigProvider: o.Context.LocalConfigProvider,
+ OCClient: *o.Context.Client,
+ })
+
  return
 }
 
-// Validate validates the StorageListOptions based on completed values
-func (o *StorageListOptions) Validate() (err error) {
+// Validate validates the ListOptions based on completed values
+func (o *ListOptions) Validate() (err error) {
  return nil
 }
 
-func (o *StorageListOptions) Run() (err error) {
- var storageList storage.StorageList
- var componentName string
- if o.isDevfile {
- componentName = o.EnvSpecificInfo.GetName()
- storageList, err = storage.DevfileList(o.KClient, o.DevfileObj.Data, o.EnvSpecificInfo.GetName())
- if err != nil {
- return err
- }
- } else {
- componentName = o.LocalConfigInfo.GetName()
- storageList, err = storage.ListStorageWithState(o.Client, o.LocalConfigInfo, o.Component(), o.Application)
- if err != nil {
- return err
- }
+func (o *ListOptions) Run() (err error) {
+ storageList, err := o.client.List()
+ if err != nil {
+ return err
  }
 
  if log.IsJSON() {
  machineoutput.OutputSuccess(storageList)
  } else {
- if o.isDevfile && isContainerDisplay(storageList, o.DevfileObj.Data.GetComponents()) {
- printStorageWithContainer(storageList, componentName)
+ if !o.Context.LocalConfigInfo.Exists() && isContainerDisplay(storageList, o.Context.LocalConfigProvider.GetContainers()) {
+ printStorageWithContainer(storageList, o.Context.LocalConfigProvider.GetName())
  } else {
- printStorage(storageList, componentName)
+ printStorage(storageList, o.Context.LocalConfigProvider.GetName())
  }
  }
 
@@ -161,14 +141,12 @@ func printStorageWithContainer(storageList storage.StorageList, compName string)
 }
 
 // isContainerDisplay checks whether the container name should be included in the output
-func isContainerDisplay(storageList storage.StorageList, components []devfilev1.Component) bool {
+func isContainerDisplay(storageList storage.StorageList, components []localConfigProvider.LocalContainer) bool {
 
  // get all the container names
  componentsMap := make(map[string]bool)
  for _, comp := range components {
- if comp.Container != nil {
- componentsMap[comp.Name] = true
- }
+ componentsMap[comp.Name] = true
  }
 
  storageCompMap := make(map[string][]string)

pkg/odo/cli/storage/list_test.go

@@ -3,9 +3,8 @@ package storage
 import (
  "testing"
 
- devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
+ "github.com/openshift/odo/pkg/localConfigProvider"
  "github.com/openshift/odo/pkg/storage"
- "github.com/openshift/odo/pkg/testingutil"
 )
 
 func Test_isContainerDisplay(t *testing.T) {
@@ -17,7 +16,7 @@ func Test_isContainerDisplay(t *testing.T) {
 
  type args struct {
  storageList storage.StorageList
- obj []devfilev1.Component
+ obj []localConfigProvider.LocalContainer
  }
  tests := []struct {
  name string
@@ -33,9 +32,13 @@ func Test_isContainerDisplay(t *testing.T) {
  generateStorage(storage.GetMachineReadableFormat("pvc-1", "1Gi", "/data"), storage.StateTypePushed, "container-1"),
  },
  },
- obj: []devfilev1.Component{
- testingutil.GetFakeContainerComponent("container-0"),
- testingutil.GetFakeContainerComponent("container-1"),
+ obj: []localConfigProvider.LocalContainer{
+ {
+ Name: "container-0",
+ },
+ {
+ Name: "container-1",
+ },
  },
  },
  want: false,
@@ -49,9 +52,13 @@ func Test_isContainerDisplay(t *testing.T) {
  generateStorage(storage.GetMachineReadableFormat("pvc-1", "1Gi", "/path"), storage.StateTypePushed, "container-1"),
  },
  },
- obj: []devfilev1.Component{
- testingutil.GetFakeContainerComponent("container-0"),
- testingutil.GetFakeContainerComponent("container-1"),
+ obj: []localConfigProvider.LocalContainer{
+ {
+ Name: "container-0",
+ },
+ {
+ Name: "container-1",
+ },
  },
  },
  want: true,
@@ -65,9 +72,13 @@ func Test_isContainerDisplay(t *testing.T) {
  generateStorage(storage.GetMachineReadableFormat("pvc-1", "1Gi", "/data"), storage.StateTypeNotPushed, "container-1"),
  },
  },
- obj: []devfilev1.Component{
- testingutil.GetFakeContainerComponent("container-0"),
- testingutil.GetFakeContainerComponent("container-1"),
+ obj: []localConfigProvider.LocalContainer{
+ {
+ Name: "container-0",
+ },
+ {
+ Name: "container-1",
+ },
  },
  },
  want: true,
@@ -80,9 +91,13 @@ func Test_isContainerDisplay(t *testing.T) {
  generateStorage(storage.GetMachineReadableFormat("pvc-1", "1Gi", "/data"), storage.StateTypePushed, "container-0"),
  },
  },
- obj: []devfilev1.Component{
- testingutil.GetFakeContainerComponent("container-0"),
- testingutil.GetFakeContainerComponent("container-1"),
+ obj: []localConfigProvider.LocalContainer{
+ {
+ Name: "container-0",
+ },
+ {
+ Name: "container-1",
+ },
  },
  },
  want: true,
@@ -96,8 +111,10 @@ func Test_isContainerDisplay(t *testing.T) {
  generateStorage(storage.GetMachineReadableFormat("pvc-1", "1Gi", "/data"), storage.StateTypePushed, "container-1"),
  },
  },
- obj: []devfilev1.Component{
- testingutil.GetFakeContainerComponent("container-0"),
+ obj: []localConfigProvider.LocalContainer{
+ {
+ Name: "container-0",
+ },
  },
  },
  want: true,