Skip to content

Commit

Permalink
Global flag refactor (#792)
Browse files Browse the repository at this point in the history
  • Loading branch information
kensipe committed Sep 12, 2019
1 parent 50a6626 commit d895faf
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 95 deletions.
4 changes: 1 addition & 3 deletions pkg/kudoctl/cmd/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,16 @@ import (

// newGetCmd creates a command that lists the instances in the cluster
func newGetCmd() *cobra.Command {
options := get.DefaultOptions
getCmd := &cobra.Command{
Use: "get instances",
Short: "Gets all available instances.",
Long: `
# Get all available instances
kudoctl get instances`,
RunE: func(cmd *cobra.Command, args []string) error {
return get.Run(args, options, &Settings)
return get.Run(args, &Settings)
},
}

getCmd.Flags().StringVar(&options.Namespace, "namespace", "default", "The namespace where the operator watches for changes.")
return getCmd
}
26 changes: 8 additions & 18 deletions pkg/kudoctl/cmd/get/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,20 @@ import (
"github.com/xlab/treeprint"
)

// Options defines configuration options for the get command
type Options struct {
Namespace string
}

// DefaultOptions initializes the get command options to its defaults
var DefaultOptions = &Options{
Namespace: "default",
}

// Run returns the errors associated with cmd env
func Run(args []string, options *Options, settings *env.Settings) error {
func Run(args []string, settings *env.Settings) error {

err := validate(args, options)
err := validate(args)
if err != nil {
return err
}

kc, err := kudo.NewClient(options.Namespace, settings.KubeConfig)
kc, err := kudo.NewClient(settings.Namespace, settings.KubeConfig)
if err != nil {
return errors.Wrap(err, "creating kudo client")
}

p, err := getInstances(kc, options)
p, err := getInstances(kc, settings)
if err != nil {
log.Printf("Error: %v", err)
}
Expand All @@ -43,12 +33,12 @@ func Run(args []string, options *Options, settings *env.Settings) error {
for _, plan := range p {
tree.AddBranch(plan)
}
fmt.Printf("List of current installed instances in namespace \"%s\":\n", options.Namespace)
fmt.Printf("List of current installed instances in namespace \"%s\":\n", settings.Namespace)
fmt.Println(tree.String())
return err
}

func validate(args []string, options *Options) error {
func validate(args []string) error {
if len(args) != 1 {
return fmt.Errorf("expecting exactly one argument - \"instances\"")
}
Expand All @@ -61,9 +51,9 @@ func validate(args []string, options *Options) error {

}

func getInstances(kc *kudo.Client, options *Options) ([]string, error) {
func getInstances(kc *kudo.Client, settings *env.Settings) ([]string, error) {

instanceList, err := kc.ListInstances(options.Namespace)
instanceList, err := kc.ListInstances(settings.Namespace)
if err != nil {
return nil, errors.Wrap(err, "getting instances")
}
Expand Down
26 changes: 13 additions & 13 deletions pkg/kudoctl/cmd/get/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,26 @@ import (

"github.com/kudobuilder/kudo/pkg/apis/kudo/v1alpha1"
"github.com/kudobuilder/kudo/pkg/client/clientset/versioned/fake"
"github.com/kudobuilder/kudo/pkg/kudoctl/env"
"github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestValidate(t *testing.T) {
tests := []struct {
arg []string
opt Options
err string
}{
{nil, *DefaultOptions, "expecting exactly one argument - \"instances\""}, // 1
{[]string{"arg", "arg2"}, *DefaultOptions, "expecting exactly one argument - \"instances\""}, // 2
{[]string{}, *DefaultOptions, "expecting exactly one argument - \"instances\""}, // 3
{[]string{"somethingelse"}, *DefaultOptions, "expecting \"instances\" and not \"somethingelse\""}, // 4
{nil, "expecting exactly one argument - \"instances\""}, // 1
{[]string{"arg", "arg2"}, "expecting exactly one argument - \"instances\""}, // 2
{[]string{}, "expecting exactly one argument - \"instances\""}, // 3
{[]string{"somethingelse"}, "expecting \"instances\" and not \"somethingelse\""}, // 4
}

for _, tt := range tests {
err := validate(tt.arg, DefaultOptions)
err := validate(tt.arg)
if err != nil {
if err.Error() != tt.err {
t.Errorf("Expecting error message '%s' but got '%s'", tt.err, err)
Expand Down Expand Up @@ -57,21 +58,20 @@ func TestGetInstances(t *testing.T) {
}
tests := []struct {
arg []string
opt Options
err string
instances []string
}{
{nil, *DefaultOptions, "expecting exactly one argument - \"instances\"", nil}, // 1
{[]string{"arg", "arg2"}, *DefaultOptions, "expecting exactly one argument - \"instances\"", nil}, // 2
{[]string{}, *DefaultOptions, "expecting exactly one argument - \"instances\"", nil}, // 3
{[]string{"somethingelse"}, *DefaultOptions, "expecting \"instances\" and not \"somethingelse\"", nil}, // 4
{[]string{"instances"}, *DefaultOptions, "expecting \"instances\" and not \"somethingelse\"", []string{"test"}}, // 5
{nil, "expecting exactly one argument - \"instances\"", nil}, // 1
{[]string{"arg", "arg2"}, "expecting exactly one argument - \"instances\"", nil}, // 2
{[]string{}, "expecting exactly one argument - \"instances\"", nil}, // 3
{[]string{"somethingelse"}, "expecting \"instances\" and not \"somethingelse\"", nil}, // 4
{[]string{"instances"}, "expecting \"instances\" and not \"somethingelse\"", []string{"test"}}, // 5
}

for i, tt := range tests {
kc := newTestClient()
kc.InstallInstanceObjToCluster(testInstance, "default")
instanceList, err := getInstances(kc, DefaultOptions)
instanceList, err := getInstances(kc, env.DefaultSettings)
if err != nil {
if err.Error() != tt.err {
t.Errorf("%d: Expecting error message '%s' but got '%s'", i+1, tt.err, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kudoctl/cmd/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func newInstallCmd(fs afero.Fs) *cobra.Command {
}

installCmd.Flags().StringVar(&options.InstanceName, "instance", "", "The instance name. (default to Operator name)")
installCmd.Flags().StringVar(&options.Namespace, "namespace", "default", "The namespace used for the package installation. (default \"default\"")
installCmd.Flags().StringArrayVarP(&parameters, "parameter", "p", nil, "The parameter name and value separated by '='")
installCmd.Flags().StringVarP(&options.PackageVersion, "version", "v", "", "A specific package version on the official GitHub repo. (default to the most recent)")
installCmd.Flags().StringVar(&options.RepoName, "repo", "", "Name of repository configuration to use. (default defined by context)")
installCmd.Flags().BoolVar(&options.SkipInstance, "skip-instance", false, "If set, install will install the Operator and OperatorVersion, but not an instance. (default \"false\")")
return installCmd
}
37 changes: 20 additions & 17 deletions pkg/kudoctl/cmd/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,22 @@ import (
"github.com/spf13/afero"
)

// RepositoryOptions defines the options necessary for any cmd working with repository
type RepositoryOptions struct {
RepoName string
}

// Options defines configuration options for the install command
type Options struct {
RepositoryOptions
InstanceName string
Namespace string
Parameters map[string]string
PackageVersion string
SkipInstance bool
}

// DefaultOptions initializes the install command options to its defaults
var DefaultOptions = &Options{
Namespace: "default",
}
var DefaultOptions = &Options{}

// Run returns the errors associated with cmd env
func Run(args []string, options *Options, fs afero.Fs, settings *env.Settings) error {
Expand Down Expand Up @@ -89,12 +92,12 @@ func GetPackageCRDs(name string, version string, repository repo.Repository) (*b
// installOperator is installing single operator into cluster and returns error in case of error
func installOperator(operatorArgument string, options *Options, fs afero.Fs, settings *env.Settings) error {

repository, err := repo.ClientFromSettings(fs, settings.Home, settings.RepoName)
repository, err := repo.ClientFromSettings(fs, settings.Home, options.RepoName)
if err != nil {
return errors.WithMessage(err, "could not build operator repository")
}

kc, err := kudo.NewClient(options.Namespace, settings.KubeConfig)
kc, err := kudo.NewClient(settings.Namespace, settings.KubeConfig)
if err != nil {
return errors.Wrap(err, "creating kudo client")
}
Expand All @@ -104,10 +107,10 @@ func installOperator(operatorArgument string, options *Options, fs afero.Fs, set
return errors.Wrapf(err, "failed to resolve package CRDs for operator: %s", operatorArgument)
}

return installCrds(crds, kc, options)
return installCrds(crds, kc, options, settings)
}

func installCrds(crds *bundle.PackageCRDs, kc *kudo.Client, options *Options) error {
func installCrds(crds *bundle.PackageCRDs, kc *kudo.Client, options *Options, settings *env.Settings) error {
// PRE-INSTALLATION SETUP
operatorName := crds.Operator.ObjectMeta.Name
operatorVersion := crds.OperatorVersion.Spec.Version
Expand All @@ -122,21 +125,21 @@ func installCrds(crds *bundle.PackageCRDs, kc *kudo.Client, options *Options) er
// Operator part

// Check if Operator exists
if !kc.OperatorExistsInCluster(crds.Operator.ObjectMeta.Name, options.Namespace) {
if err := installSingleOperatorToCluster(operatorName, options.Namespace, crds.Operator, kc); err != nil {
if !kc.OperatorExistsInCluster(crds.Operator.ObjectMeta.Name, settings.Namespace) {
if err := installSingleOperatorToCluster(operatorName, settings.Namespace, crds.Operator, kc); err != nil {
return errors.Wrap(err, "installing single Operator")
}
}

// OperatorVersion part

versionsInstalled, err := kc.OperatorVersionsInstalled(operatorName, options.Namespace)
versionsInstalled, err := kc.OperatorVersionsInstalled(operatorName, settings.Namespace)
if err != nil {
return errors.Wrap(err, "retrieving existing operator versions")
}
if !VersionExists(versionsInstalled, operatorVersion) {
// this version does not exist in the cluster
if err := installSingleOperatorVersionToCluster(operatorName, options.Namespace, kc, crds.OperatorVersion); err != nil {
if err := installSingleOperatorVersionToCluster(operatorName, settings.Namespace, kc, crds.OperatorVersion); err != nil {
return errors.Wrapf(err, "installing OperatorVersion CRD for operator: %s", operatorName)
}
}
Expand All @@ -153,20 +156,20 @@ func installCrds(crds *bundle.PackageCRDs, kc *kudo.Client, options *Options) er
// Check if Instance exists in cluster
// It won't create the Instance if any in combination with given Operator Name, OperatorVersion and Instance OperatorName exists
instanceName := crds.Instance.ObjectMeta.Name
instanceExists, err := kc.InstanceExistsInCluster(operatorName, options.Namespace, crds.OperatorVersion.Spec.Version, instanceName)
instanceExists, err := kc.InstanceExistsInCluster(operatorName, settings.Namespace, crds.OperatorVersion.Spec.Version, instanceName)
if err != nil {
return errors.Wrapf(err, "verifying the instance does not already exist")
}

if !instanceExists {
if err := installSingleInstanceToCluster(operatorName, crds.Instance, kc, options); err != nil {
if err := installSingleInstanceToCluster(operatorName, crds.Instance, kc, options, settings); err != nil {
return errors.Wrap(err, "installing single instance")

}

} else {
return fmt.Errorf("can not install instance '%s' of operator '%s-%s' because instance of that name already exists in namespace %s",
instanceName, operatorName, crds.OperatorVersion.Spec.Version, options.Namespace)
instanceName, operatorName, crds.OperatorVersion.Spec.Version, settings.Namespace)
}
return nil
}
Expand Down Expand Up @@ -225,8 +228,8 @@ func installSingleOperatorVersionToCluster(name, namespace string, kc *kudo.Clie

// installSingleInstanceToCluster installs a given Instance to the cluster
// TODO: needs more testing
func installSingleInstanceToCluster(name string, instance *v1alpha1.Instance, kc *kudo.Client, options *Options) error {
if _, err := kc.InstallInstanceObjToCluster(instance, options.Namespace); err != nil {
func installSingleInstanceToCluster(name string, instance *v1alpha1.Instance, kc *kudo.Client, options *Options, settings *env.Settings) error {
if _, err := kc.InstallInstanceObjToCluster(instance, settings.Namespace); err != nil {
return errors.Wrapf(err, "installing instance %s", name)
}
fmt.Printf("instance.%s/%s created\n", instance.APIVersion, instance.Name)
Expand Down
3 changes: 2 additions & 1 deletion pkg/kudoctl/cmd/install/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/kudobuilder/kudo/pkg/kudoctl/bundle"
"github.com/kudobuilder/kudo/pkg/kudoctl/env"
util "github.com/kudobuilder/kudo/pkg/util/kudo"

"github.com/kudobuilder/kudo/pkg/apis/kudo/v1alpha1"
Expand Down Expand Up @@ -111,7 +112,7 @@ func TestParameterValidation_InstallCrds(t *testing.T) {
options.Parameters = tt.installParameters
options.SkipInstance = tt.skipInstance

err := installCrds(&testCrds, kc, options)
err := installCrds(&testCrds, kc, options, env.DefaultSettings)
if err != nil && err.Error() != tt.err {
t.Errorf("%s: Expected error '%s', got '%s'", tt.name, tt.err, err.Error())
}
Expand Down
19 changes: 8 additions & 11 deletions pkg/kudoctl/cmd/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,11 @@ var (

type updateOptions struct {
InstanceName string
Namespace string
Parameters map[string]string
}

// defaultOptions initializes the install command options to its defaults
var defaultUpdateOptions = &updateOptions{
Namespace: "default",
}
var defaultUpdateOptions = &updateOptions{}

// newUpdateCmd creates the install command for the CLI
func newUpdateCmd() *cobra.Command {
Expand All @@ -55,7 +52,7 @@ func newUpdateCmd() *cobra.Command {

updateCmd.Flags().StringVar(&options.InstanceName, "instance", "", "The instance name.")
updateCmd.Flags().StringArrayVarP(&parameters, "parameter", "p", nil, "The parameter name and value separated by '='")
updateCmd.Flags().StringVar(&options.Namespace, "namespace", defaultOptions.Namespace, "The namespace where the instance you want to upgrade is installed in.")

return updateCmd
}

Expand All @@ -80,26 +77,26 @@ func runUpdate(args []string, options *updateOptions, settings *env.Settings) er
}
instanceToUpdate := options.InstanceName

kc, err := kudo.NewClient(options.Namespace, settings.KubeConfig)
kc, err := kudo.NewClient(settings.Namespace, settings.KubeConfig)
if err != nil {
return errors.Wrap(err, "creating kudo client")
}

return update(instanceToUpdate, kc, options)
return update(instanceToUpdate, kc, options, settings)
}

func update(instanceToUpdate string, kc *kudo.Client, options *updateOptions) error {
func update(instanceToUpdate string, kc *kudo.Client, options *updateOptions, settings *env.Settings) error {
// Make sure the instance you want to upgrade exists
instance, err := kc.GetInstance(instanceToUpdate, options.Namespace)
instance, err := kc.GetInstance(instanceToUpdate, settings.Namespace)
if err != nil {
return errors.Wrapf(err, "verifying the instance does not already exist")
}
if instance == nil {
return fmt.Errorf("instance %s in namespace %s does not exist in the cluster", instanceToUpdate, options.Namespace)
return fmt.Errorf("instance %s in namespace %s does not exist in the cluster", instanceToUpdate, settings.Namespace)
}

// Update arguments
err = kc.UpdateInstance(instanceToUpdate, options.Namespace, nil, options.Parameters)
err = kc.UpdateInstance(instanceToUpdate, settings.Namespace, nil, options.Parameters)
if err != nil {
return errors.Wrapf(err, "updating instance %s", instanceToUpdate)
}
Expand Down
7 changes: 3 additions & 4 deletions pkg/kudoctl/cmd/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"strings"
"testing"

"github.com/kudobuilder/kudo/pkg/kudoctl/env"

"github.com/kudobuilder/kudo/pkg/apis/kudo/v1alpha1"
util "github.com/kudobuilder/kudo/pkg/util/kudo"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -76,10 +78,7 @@ func TestUpdate(t *testing.T) {
c.InstallInstanceObjToCluster(&testInstance, installNamespace)
}

err := update(testInstance.Name, c, &updateOptions{
Namespace: installNamespace,
Parameters: tt.parameters,
})
err := update(testInstance.Name, c, &updateOptions{Parameters: tt.parameters}, env.DefaultSettings)
if err != nil {
if !strings.Contains(err.Error(), tt.errMessageContains) {
t.Errorf("%s: expected error '%s' but got '%v'", tt.name, tt.errMessageContains, err)
Expand Down
Loading

0 comments on commit d895faf

Please sign in to comment.