Skip to content

Rename cmdutils.ResourceCmd to cmdutils.Cmd #1127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
errordeveloper merged 1 commit into from
Aug 9, 2019
Merged

Rename cmdutils.ResourceCmd to cmdutils.Cmd #1127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 29 additions & 28 deletions pkg/ctl/cmdutils/resource.go → pkg/ctl/cmdutils/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import (
"github.com/weaveworks/eksctl/pkg/eks"
)

// ResourceCmd holds attributes that most of the commands use
type ResourceCmd struct {
Command *cobra.Command
// Cmd holds attributes that are common between commands;
// not all commands use each attribute, but but they can if needed
type Cmd struct {
CobraCommand *cobra.Command
Copy link
Contributor Author

@errordeveloper errordeveloper Aug 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed this to avoid ambiguity, otherwise it'd be cmd.Command...

FlagSetGroup *NamedFlagSetGroup

Plan, Wait, Validate bool
Expand All @@ -30,19 +31,19 @@ type ResourceCmd struct {
// NewCtl performs common defaulting and validation and constructs a new
// instance of eks.ClusterProvider, it may return an error if configuration
// is invalid or region is not supported
func (rc *ResourceCmd) NewCtl() (*eks.ClusterProvider, error) {
api.SetClusterConfigDefaults(rc.ClusterConfig)
func (c *Cmd) NewCtl() (*eks.ClusterProvider, error) {
api.SetClusterConfigDefaults(c.ClusterConfig)

if err := api.ValidateClusterConfig(rc.ClusterConfig); err != nil {
if rc.Validate {
if err := api.ValidateClusterConfig(c.ClusterConfig); err != nil {
if c.Validate {
return nil, err
}
logger.Warning("ignoring validation error: %s", err.Error())
}

for i, ng := range rc.ClusterConfig.NodeGroups {
for i, ng := range c.ClusterConfig.NodeGroups {
if err := api.ValidateNodeGroup(i, ng); err != nil {
if rc.Validate {
if c.Validate {
return nil, err
}
logger.Warning("ignoring validation error: %s", err.Error())
Expand All @@ -52,50 +53,50 @@ func (rc *ResourceCmd) NewCtl() (*eks.ClusterProvider, error) {
api.SetNodeGroupDefaults(i, ng)
}

ctl := eks.New(rc.ProviderConfig, rc.ClusterConfig)
ctl := eks.New(c.ProviderConfig, c.ClusterConfig)

if !ctl.IsSupportedRegion() {
return nil, ErrUnsupportedRegion(rc.ProviderConfig)
return nil, ErrUnsupportedRegion(c.ProviderConfig)
}

return ctl, nil
}

// AddResourceCmd create a registers a new command under the given verb command
func AddResourceCmd(flagGrouping *FlagGrouping, parentVerbCmd *cobra.Command, newResourceCmd func(*ResourceCmd)) {
resource := &ResourceCmd{
Command: &cobra.Command{},
func AddResourceCmd(flagGrouping *FlagGrouping, parentVerbCmd *cobra.Command, newCmd func(*Cmd)) {
c := &Cmd{
CobraCommand: &cobra.Command{},
ProviderConfig: &api.ProviderConfig{},

Plan: true, // always on by default
Wait: false, // varies in some commands
Validate: true, // also on by default
}
resource.FlagSetGroup = flagGrouping.New(resource.Command)
newResourceCmd(resource)
resource.FlagSetGroup.AddTo(resource.Command)
parentVerbCmd.AddCommand(resource.Command)
c.FlagSetGroup = flagGrouping.New(c.CobraCommand)
newCmd(c)
c.FlagSetGroup.AddTo(c.CobraCommand)
parentVerbCmd.AddCommand(c.CobraCommand)
}

// SetDescription sets usage along with short and long descriptions as well as aliases
func (rc *ResourceCmd) SetDescription(use, short, long string, aliases ...string) {
rc.Command.Use = use
rc.Command.Short = short
rc.Command.Long = long
rc.Command.Aliases = aliases
func (c *Cmd) SetDescription(use, short, long string, aliases ...string) {
c.CobraCommand.Use = use
c.CobraCommand.Short = short
c.CobraCommand.Long = long
c.CobraCommand.Aliases = aliases
}

// SetRunFunc registers a command function
func (rc *ResourceCmd) SetRunFunc(cmd func() error) {
rc.Command.Run = func(_ *cobra.Command, _ []string) {
func (c *Cmd) SetRunFunc(cmd func() error) {
c.CobraCommand.Run = func(_ *cobra.Command, _ []string) {
run(cmd)
}
}

// SetRunFuncWithNameArg registers a command function with an optional name argument
func (rc *ResourceCmd) SetRunFuncWithNameArg(cmd func() error) {
rc.Command.Run = func(_ *cobra.Command, args []string) {
rc.NameArg = GetNameArg(args)
func (c *Cmd) SetRunFuncWithNameArg(cmd func() error) {
c.CobraCommand.Run = func(_ *cobra.Command, args []string) {
c.NameArg = GetNameArg(args)
run(cmd)
}
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/ctl/cmdutils/cmdutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ func LogPlanModeWarning(plan bool) {
}

// AddApproveFlag adds common `--approve` flag
func AddApproveFlag(fs *pflag.FlagSet, rc *ResourceCmd) {
approve := fs.Bool("approve", !rc.Plan, "Apply the changes")
AddPreRun(rc.Command, func(cmd *cobra.Command, args []string) {
if cmd.Flag("approve").Changed {
rc.Plan = !*approve
func AddApproveFlag(fs *pflag.FlagSet, cmd *Cmd) {
approve := fs.Bool("approve", !cmd.Plan, "Apply the changes")
AddPreRun(cmd.CobraCommand, func(cobraCmd *cobra.Command, args []string) {
if cobraCmd.Flag("approve").Changed {
cmd.Plan = !*approve
}
})
}
Expand Down
36 changes: 18 additions & 18 deletions pkg/ctl/cmdutils/configfile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,18 @@ type ClusterConfigLoader interface {
}

type commonClusterConfigLoader struct {
*ResourceCmd
*Cmd

flagsIncompatibleWithConfigFile, flagsIncompatibleWithoutConfigFile sets.String

validateWithConfigFile, validateWithoutConfigFile func() error
}

func newCommonClusterConfigLoader(rc *ResourceCmd) *commonClusterConfigLoader {
func newCommonClusterConfigLoader(cmd *Cmd) *commonClusterConfigLoader {
nilValidatorFunc := func() error { return nil }

return &commonClusterConfigLoader{
ResourceCmd: rc,
Cmd: cmd,

validateWithConfigFile: nilValidatorFunc,
flagsIncompatibleWithConfigFile: sets.NewString("name", "region", "version"),
Expand All @@ -49,7 +49,7 @@ func (l *commonClusterConfigLoader) Load() error {

if l.ClusterConfigFile == "" {
for f := range l.flagsIncompatibleWithoutConfigFile {
if flag := l.Command.Flag(f); flag != nil && flag.Changed {
if flag := l.CobraCommand.Flag(f); flag != nil && flag.Changed {
return fmt.Errorf("cannot use --%s unless a config file is specified via --config-file/-f", f)
}
}
Expand All @@ -58,9 +58,9 @@ func (l *commonClusterConfigLoader) Load() error {

var err error

// The reference to ResourceCmd.ClusterConfig should only be reassigned if ClusterConfigFile is specified
// The reference to ClusterConfig should only be reassigned if ClusterConfigFile is specified
// because other parts of the code store the pointer locally and access it directly instead of via
// the ResourceCmd reference
// the Cmd reference
if l.ClusterConfig, err = eks.LoadConfigFromFile(l.ClusterConfigFile); err != nil {
return err
}
Expand All @@ -71,7 +71,7 @@ func (l *commonClusterConfigLoader) Load() error {
}

for f := range l.flagsIncompatibleWithConfigFile {
if flag := l.Command.Flag(f); flag != nil && flag.Changed {
if flag := l.CobraCommand.Flag(f); flag != nil && flag.Changed {
return ErrCannotUseWithConfigFile(fmt.Sprintf("--%s", f))
}
}
Expand Down Expand Up @@ -113,17 +113,17 @@ func (l *commonClusterConfigLoader) validateMetadataWithoutConfigFile() error {
// NewMetadataLoader handles loading of clusterConfigFile vs using flags for all commands that require only
// metadata fields, e.g. `eksctl delete cluster` or `eksctl utils update-kube-proxy` and other similar
// commands that do simple operations against existing clusters
func NewMetadataLoader(rc *ResourceCmd) ClusterConfigLoader {
l := newCommonClusterConfigLoader(rc)
func NewMetadataLoader(cmd *Cmd) ClusterConfigLoader {
l := newCommonClusterConfigLoader(cmd)

l.validateWithoutConfigFile = l.validateMetadataWithoutConfigFile

return l
}

// NewCreateClusterLoader will load config or use flags for 'eksctl create cluster'
func NewCreateClusterLoader(rc *ResourceCmd, ngFilter *NodeGroupFilter) ClusterConfigLoader {
l := newCommonClusterConfigLoader(rc)
func NewCreateClusterLoader(cmd *Cmd, ngFilter *NodeGroupFilter) ClusterConfigLoader {
l := newCommonClusterConfigLoader(cmd)

l.flagsIncompatibleWithConfigFile.Insert(
"tags",
Expand Down Expand Up @@ -197,8 +197,8 @@ func NewCreateClusterLoader(rc *ResourceCmd, ngFilter *NodeGroupFilter) ClusterC
}

// NewCreateNodeGroupLoader will load config or use flags for 'eksctl create nodegroup'
func NewCreateNodeGroupLoader(rc *ResourceCmd, ngFilter *NodeGroupFilter) ClusterConfigLoader {
l := newCommonClusterConfigLoader(rc)
func NewCreateNodeGroupLoader(cmd *Cmd, ngFilter *NodeGroupFilter) ClusterConfigLoader {
l := newCommonClusterConfigLoader(cmd)

l.flagsIncompatibleWithConfigFile.Insert(
"cluster",
Expand Down Expand Up @@ -255,7 +255,7 @@ func NewCreateNodeGroupLoader(rc *ResourceCmd, ngFilter *NodeGroupFilter) Cluste
}

func normalizeNodeGroup(ng *api.NodeGroup, l *commonClusterConfigLoader) error {
if l.Command.Flag("ssh-public-key").Changed {
if l.CobraCommand.Flag("ssh-public-key").Changed {
if *ng.SSH.PublicKeyPath == "" {
return fmt.Errorf("--ssh-public-key must be non-empty string")
}
Expand All @@ -272,8 +272,8 @@ func normalizeNodeGroup(ng *api.NodeGroup, l *commonClusterConfigLoader) error {
}

// NewDeleteNodeGroupLoader will load config or use flags for 'eksctl delete nodegroup'
func NewDeleteNodeGroupLoader(rc *ResourceCmd, ng *api.NodeGroup, ngFilter *NodeGroupFilter) ClusterConfigLoader {
l := newCommonClusterConfigLoader(rc)
func NewDeleteNodeGroupLoader(cmd *Cmd, ng *api.NodeGroup, ngFilter *NodeGroupFilter) ClusterConfigLoader {
l := newCommonClusterConfigLoader(cmd)

l.flagsIncompatibleWithConfigFile.Insert(
"cluster",
Expand Down Expand Up @@ -319,8 +319,8 @@ func NewDeleteNodeGroupLoader(rc *ResourceCmd, ng *api.NodeGroup, ngFilter *Node
}

// NewUtilsEnableLoggingLoader will load config or use flags for 'eksctl utils update-cluster-logging'
func NewUtilsEnableLoggingLoader(rc *ResourceCmd) ClusterConfigLoader {
l := newCommonClusterConfigLoader(rc)
func NewUtilsEnableLoggingLoader(cmd *Cmd) ClusterConfigLoader {
l := newCommonClusterConfigLoader(cmd)

l.flagsIncompatibleWithConfigFile.Insert(
"enable-types",
Expand Down
40 changes: 20 additions & 20 deletions pkg/ctl/cmdutils/configfile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,47 +29,47 @@ var _ = Describe("cmdutils configfile", func() {
cfg := api.NewClusterConfig()

{
rc := &ResourceCmd{
cmd := &Cmd{
ClusterConfig: cfg,
NameArg: "foo-1",
}

err := NewMetadataLoader(rc).Load()
err := NewMetadataLoader(cmd).Load()
Expect(err).ToNot(HaveOccurred())
Expect(cfg.Metadata.Name).To(Equal("foo-1"))
}

{
rc := &ResourceCmd{
cmd := &Cmd{
ClusterConfig: cfg,
NameArg: "foo-2",
}

err := NewMetadataLoader(rc).Load()
err := NewMetadataLoader(cmd).Load()
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("--name=foo-1 and argument foo-2 cannot be used at the same time"))
}

{
rc := &ResourceCmd{
cmd := &Cmd{
ClusterConfig: cfg,
NameArg: "foo-3",
Command: newCmd(),
CobraCommand: newCmd(),
ClusterConfigFile: examplesDir + "01-simple-cluster.yaml",
}

err := NewMetadataLoader(rc).Load()
err := NewMetadataLoader(cmd).Load()
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal(ErrCannotUseWithConfigFile(`name argument "foo-3"`).Error()))

fs := rc.Command.Flags()
fs := cmd.CobraCommand.Flags()

fs.StringVar(&cfg.Metadata.Name, "name", "", "")
rc.Command.Flag("name").Changed = true
cmd.CobraCommand.Flag("name").Changed = true

Expect(rc.Command.Flag("name").Changed).To(BeTrue())
Expect(cmd.CobraCommand.Flag("name").Changed).To(BeTrue())

err = NewMetadataLoader(rc).Load()
err = NewMetadataLoader(cmd).Load()

Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal(ErrCannotUseWithConfigFile("--name").Error()))
Expand All @@ -82,20 +82,20 @@ var _ = Describe("cmdutils configfile", func() {

Expect(examples).To(HaveLen(11))
for _, example := range examples {
rc := &ResourceCmd{
Command: newCmd(),
cmd := &Cmd{
CobraCommand: newCmd(),
ClusterConfigFile: example,
ClusterConfig: api.NewClusterConfig(),
ProviderConfig: &api.ProviderConfig{},
}

err := NewMetadataLoader(rc).Load()
err := NewMetadataLoader(cmd).Load()

cfg := rc.ClusterConfig
cfg := cmd.ClusterConfig
Expect(err).ToNot(HaveOccurred())
Expect(cfg.Metadata.Name).ToNot(BeEmpty())
Expect(cfg.Metadata.Region).ToNot(BeEmpty())
Expect(cfg.Metadata.Region).To(Equal(rc.ProviderConfig.Region))
Expect(cfg.Metadata.Region).To(Equal(cmd.ProviderConfig.Region))
Expect(cfg.Metadata.Version).To(BeEmpty())
}
})
Expand All @@ -116,15 +116,15 @@ var _ = Describe("cmdutils configfile", func() {
}

for _, natTest := range natTests {
rc := &ResourceCmd{
Command: newCmd(),
cmd := &Cmd{
CobraCommand: newCmd(),
ClusterConfigFile: filepath.Join(examplesDir, natTest.configFile),
ClusterConfig: api.NewClusterConfig(),
ProviderConfig: &api.ProviderConfig{},
}

Expect(NewCreateClusterLoader(rc, nil).Load()).To(Succeed())
cfg := rc.ClusterConfig
Expect(NewCreateClusterLoader(cmd, nil).Load()).To(Succeed())
cfg := cmd.ClusterConfig
Expect(cfg.VPC.NAT.Gateway).To(Not(BeNil()))
Expect(*cfg.VPC.NAT.Gateway).To(Equal(natTest.expectedGateway))
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/ctl/cmdutils/nodegroup_flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@ import (
)

// AddCommonCreateNodeGroupFlags adds common flags for creating a node group
func AddCommonCreateNodeGroupFlags(fs *pflag.FlagSet, rc *ResourceCmd, ng *api.NodeGroup) {
func AddCommonCreateNodeGroupFlags(fs *pflag.FlagSet, cmd *Cmd, ng *api.NodeGroup) {
fs.StringVarP(&ng.InstanceType, "node-type", "t", api.DefaultNodeType, "node instance type")

desiredCapacity := fs.IntP("nodes", "N", api.DefaultNodeCount, "total number of nodes (for a static ASG)")
minSize := fs.IntP("nodes-min", "m", api.DefaultNodeCount, "minimum nodes in ASG")
maxSize := fs.IntP("nodes-max", "M", api.DefaultNodeCount, "maximum nodes in ASG")

AddPreRun(rc.Command, func(cmd *cobra.Command, args []string) {
if f := cmd.Flag("nodes"); f.Changed {
AddPreRun(cmd.CobraCommand, func(cobraCmd *cobra.Command, args []string) {
if f := cobraCmd.Flag("nodes"); f.Changed {
ng.DesiredCapacity = desiredCapacity
}
if f := cmd.Flag("nodes-min"); f.Changed {
if f := cobraCmd.Flag("nodes-min"); f.Changed {
ng.MinSize = minSize
}
if f := cmd.Flag("nodes-max"); f.Changed {
if f := cobraCmd.Flag("nodes-max"); f.Changed {
ng.MaxSize = maxSize
}
})
Expand Down
Loading