Skip to content

Commit

Permalink
Merge pull request #103 from fromanirh/reusable-commands
Browse files Browse the repository at this point in the history
pkg: commands: refactor commands to make them reusable
  • Loading branch information
ffromani authored Jul 4, 2022
2 parents 137afff + 9a7cf0a commit b6335be
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 49 deletions.
20 changes: 10 additions & 10 deletions pkg/commands/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ import (
"github.com/spf13/cobra"
)

type deployOptions struct {
type DeployOptions struct {
clusterPlatform platform.Platform
waitCompletion bool
}

func NewDeployCommand(commonOpts *CommonOptions) *cobra.Command {
opts := &deployOptions{}
opts := &DeployOptions{}
deploy := &cobra.Command{
Use: "deploy",
Short: "deploy the components and configurations needed for topology-aware-scheduling",
Expand All @@ -51,7 +51,7 @@ func NewDeployCommand(commonOpts *CommonOptions) *cobra.Command {
}

func NewRemoveCommand(commonOpts *CommonOptions) *cobra.Command {
opts := &deployOptions{}
opts := &DeployOptions{}
remove := &cobra.Command{
Use: "remove",
Short: "remove the components and configurations needed for topology-aware-scheduling",
Expand Down Expand Up @@ -102,7 +102,7 @@ func NewRemoveCommand(commonOpts *CommonOptions) *cobra.Command {
return remove
}

func NewDeployAPICommand(commonOpts *CommonOptions, opts *deployOptions) *cobra.Command {
func NewDeployAPICommand(commonOpts *CommonOptions, opts *DeployOptions) *cobra.Command {
deploy := &cobra.Command{
Use: "api",
Short: "deploy the APIs needed for topology-aware-scheduling",
Expand All @@ -123,7 +123,7 @@ func NewDeployAPICommand(commonOpts *CommonOptions, opts *deployOptions) *cobra.
return deploy
}

func NewDeploySchedulerPluginCommand(commonOpts *CommonOptions, opts *deployOptions) *cobra.Command {
func NewDeploySchedulerPluginCommand(commonOpts *CommonOptions, opts *DeployOptions) *cobra.Command {
deploy := &cobra.Command{
Use: "scheduler-plugin",
Short: "deploy the scheduler plugin needed for topology-aware-scheduling",
Expand All @@ -146,7 +146,7 @@ func NewDeploySchedulerPluginCommand(commonOpts *CommonOptions, opts *deployOpti
return deploy
}

func NewDeployTopologyUpdaterCommand(commonOpts *CommonOptions, opts *deployOptions) *cobra.Command {
func NewDeployTopologyUpdaterCommand(commonOpts *CommonOptions, opts *DeployOptions) *cobra.Command {
deploy := &cobra.Command{
Use: "topology-updater",
Short: "deploy the topology updater needed for topology-aware-scheduling",
Expand All @@ -169,7 +169,7 @@ func NewDeployTopologyUpdaterCommand(commonOpts *CommonOptions, opts *deployOpti
return deploy
}

func NewRemoveAPICommand(commonOpts *CommonOptions, opts *deployOptions) *cobra.Command {
func NewRemoveAPICommand(commonOpts *CommonOptions, opts *DeployOptions) *cobra.Command {
remove := &cobra.Command{
Use: "api",
Short: "remove the APIs needed for topology-aware-scheduling",
Expand All @@ -191,7 +191,7 @@ func NewRemoveAPICommand(commonOpts *CommonOptions, opts *deployOptions) *cobra.
return remove
}

func NewRemoveSchedulerPluginCommand(commonOpts *CommonOptions, opts *deployOptions) *cobra.Command {
func NewRemoveSchedulerPluginCommand(commonOpts *CommonOptions, opts *DeployOptions) *cobra.Command {
remove := &cobra.Command{
Use: "scheduler-plugin",
Short: "remove the scheduler plugin needed for topology-aware-scheduling",
Expand All @@ -214,7 +214,7 @@ func NewRemoveSchedulerPluginCommand(commonOpts *CommonOptions, opts *deployOpti
return remove
}

func NewRemoveTopologyUpdaterCommand(commonOpts *CommonOptions, opts *deployOptions) *cobra.Command {
func NewRemoveTopologyUpdaterCommand(commonOpts *CommonOptions, opts *DeployOptions) *cobra.Command {
remove := &cobra.Command{
Use: "topology-updater",
Short: "remove the topology updater needed for topology-aware-scheduling",
Expand All @@ -237,7 +237,7 @@ func NewRemoveTopologyUpdaterCommand(commonOpts *CommonOptions, opts *deployOpti
return remove
}

func deployOnCluster(commonOpts *CommonOptions, opts *deployOptions) error {
func deployOnCluster(commonOpts *CommonOptions, opts *DeployOptions) error {
la := tlog.NewLogAdapter(commonOpts.Log, commonOpts.DebugLog)
platDetect := detectPlatform(commonOpts.DebugLog, commonOpts.UserPlatform)
opts.clusterPlatform = platDetect.Discovered
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ import (
"github.com/k8stopologyawareschedwg/deployer/pkg/images"
)

type imagesOptions struct {
type ImagesOptions struct {
jsonOutput bool
rawOutput bool
}

func NewImagesCommand(commonOpts *CommonOptions) *cobra.Command {
opts := &imagesOptions{}
opts := &ImagesOptions{}
images := &cobra.Command{
Use: "images",
Short: "dump the container images used to deploy",
Expand Down
14 changes: 7 additions & 7 deletions pkg/commands/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,18 @@ import (
"github.com/k8stopologyawareschedwg/deployer/pkg/tlog"
)

type renderOptions struct{}
type RenderOptions struct{}

func NewRenderCommand(commonOpts *CommonOptions) *cobra.Command {
opts := &renderOptions{}
opts := &RenderOptions{}
render := &cobra.Command{
Use: "render",
Short: "render all the manifests",
RunE: func(cmd *cobra.Command, args []string) error {
if commonOpts.UserPlatform == platform.Unknown {
return fmt.Errorf("must explicitely select a cluster platform")
}
return renderManifests(cmd, commonOpts, opts, args)
return RenderManifests(commonOpts)
},
Args: cobra.NoArgs,
}
Expand All @@ -52,7 +52,7 @@ func NewRenderCommand(commonOpts *CommonOptions) *cobra.Command {
return render
}

func NewRenderAPICommand(commonOpts *CommonOptions, opts *renderOptions) *cobra.Command {
func NewRenderAPICommand(commonOpts *CommonOptions, opts *RenderOptions) *cobra.Command {
render := &cobra.Command{
Use: "api",
Short: "render the APIs needed for topology-aware-scheduling",
Expand All @@ -71,7 +71,7 @@ func NewRenderAPICommand(commonOpts *CommonOptions, opts *renderOptions) *cobra.
return render
}

func NewRenderSchedulerPluginCommand(commonOpts *CommonOptions, opts *renderOptions) *cobra.Command {
func NewRenderSchedulerPluginCommand(commonOpts *CommonOptions, opts *RenderOptions) *cobra.Command {
render := &cobra.Command{
Use: "scheduler-plugin",
Short: "render the scheduler plugin needed for topology-aware-scheduling",
Expand Down Expand Up @@ -102,7 +102,7 @@ func NewRenderSchedulerPluginCommand(commonOpts *CommonOptions, opts *renderOpti
return render
}

func NewRenderTopologyUpdaterCommand(commonOpts *CommonOptions, opts *renderOptions) *cobra.Command {
func NewRenderTopologyUpdaterCommand(commonOpts *CommonOptions, opts *RenderOptions) *cobra.Command {
render := &cobra.Command{
Use: "topology-updater",
Short: "render the topology updater needed for topology-aware-scheduling",
Expand Down Expand Up @@ -140,7 +140,7 @@ func makeUpdaterObjects(commonOpts *CommonOptions) ([]client.Object, string, err
return append([]client.Object{ns}, objs...), namespace, nil
}

func renderManifests(cmd *cobra.Command, commonOpts *CommonOptions, opts *renderOptions, args []string) error {
func RenderManifests(commonOpts *CommonOptions) error {
var objs []client.Object

apiManifests, err := api.GetManifests(commonOpts.UserPlatform)
Expand Down
67 changes: 38 additions & 29 deletions pkg/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"os"

"github.com/spf13/cobra"
"github.com/spf13/pflag"

"github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform"
"github.com/k8stopologyawareschedwg/deployer/pkg/deployer/updaters"
Expand Down Expand Up @@ -57,29 +58,7 @@ func NewRootCommand(extraCmds ...NewCommandFunc) *cobra.Command {
Short: "deployer helps setting up all the topology-aware-scheduling components on a kubernetes cluster",

PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
if commonOpts.Debug {
commonOpts.DebugLog = log.New(os.Stderr, "", log.LstdFlags)
} else {
commonOpts.DebugLog = log.New(ioutil.Discard, "", 0)
}
// we abuse the logger to have a common interface and the timestamps
commonOpts.Log = log.New(os.Stdout, "", log.LstdFlags)

// if it is unknown, it's fine
commonOpts.UserPlatform, _ = platform.FromString(commonOpts.plat)

if commonOpts.rteConfigFile != "" {
data, err := os.ReadFile(commonOpts.rteConfigFile)
if err != nil {
return err
}
commonOpts.RTEConfigData = string(data)
commonOpts.DebugLog.Printf("RTE config: read %d bytes", len(commonOpts.RTEConfigData))
}
if err := validateUpdaterType(commonOpts.UpdaterType); err != nil {
return err
}
return nil
return PostSetupOptions(commonOpts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return ShowHelp(cmd, args)
Expand All @@ -88,12 +67,7 @@ func NewRootCommand(extraCmds ...NewCommandFunc) *cobra.Command {
SilenceErrors: true,
}

root.PersistentFlags().BoolVarP(&commonOpts.Debug, "debug", "D", false, "enable debug log")
root.PersistentFlags().StringVarP(&commonOpts.plat, "platform", "P", "", "platform to deploy on")
root.PersistentFlags().IntVarP(&commonOpts.Replicas, "replicas", "R", 1, "set the replica value - where relevant.")
root.PersistentFlags().BoolVar(&commonOpts.PullIfNotPresent, "pull-if-not-present", false, "force pull policies to IfNotPresent.")
root.PersistentFlags().StringVar(&commonOpts.rteConfigFile, "rte-config-file", "", "inject rte configuration reading from this file.")
root.PersistentFlags().StringVar(&commonOpts.UpdaterType, "updater-type", "RTE", "type of updater to deploy - RTE or NFD")
InitFlags(root.PersistentFlags(), commonOpts)

root.AddCommand(
NewRenderCommand(commonOpts),
Expand All @@ -111,6 +85,41 @@ func NewRootCommand(extraCmds ...NewCommandFunc) *cobra.Command {
return root
}

func InitFlags(flags *pflag.FlagSet, commonOpts *CommonOptions) {
flags.BoolVarP(&commonOpts.Debug, "debug", "D", false, "enable debug log")
flags.StringVarP(&commonOpts.plat, "platform", "P", "", "platform to deploy on")
flags.IntVarP(&commonOpts.Replicas, "replicas", "R", 1, "set the replica value - where relevant.")
flags.BoolVar(&commonOpts.PullIfNotPresent, "pull-if-not-present", false, "force pull policies to IfNotPresent.")
flags.StringVar(&commonOpts.rteConfigFile, "rte-config-file", "", "inject rte configuration reading from this file.")
flags.StringVar(&commonOpts.UpdaterType, "updater-type", "RTE", "type of updater to deploy - RTE or NFD")
}

func PostSetupOptions(commonOpts *CommonOptions) error {
if commonOpts.Debug {
commonOpts.DebugLog = log.New(os.Stderr, "", log.LstdFlags)
} else {
commonOpts.DebugLog = log.New(ioutil.Discard, "", 0)
}
// we abuse the logger to have a common interface and the timestamps
commonOpts.Log = log.New(os.Stdout, "", log.LstdFlags)

// if it is unknown, it's fine
commonOpts.UserPlatform, _ = platform.FromString(commonOpts.plat)

if commonOpts.rteConfigFile != "" {
data, err := os.ReadFile(commonOpts.rteConfigFile)
if err != nil {
return err
}
commonOpts.RTEConfigData = string(data)
commonOpts.DebugLog.Printf("RTE config: read %d bytes", len(commonOpts.RTEConfigData))
}
if err := validateUpdaterType(commonOpts.UpdaterType); err != nil {
return err
}
return nil
}

func validateUpdaterType(updaterType string) error {
if updaterType != updaters.RTE && updaterType != updaters.NFD {
return fmt.Errorf("%q is invalid updater type", updaterType)
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

func NewSetupCommand(commonOpts *CommonOptions) *cobra.Command {
depOpts := &deployOptions{}
depOpts := &DeployOptions{}
valOpts := &validateOptions{}
setup := &cobra.Command{
Use: "setup",
Expand Down
8 changes: 8 additions & 0 deletions test/e2e/negative.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,11 @@ var _ = ginkgo.Describe("[NegativeFlow] Deployer validation", func() {
})
})
})

var _ = ginkgo.Describe("[NegativeFlow] Deployer option validation", func() {
ginkgo.It("It should fail with invalid --updater-type", func() {
updaterType := "LOCAL"
err := deploy(updaterType)
gomega.Expect(err).To(gomega.HaveOccurred(), "deployed succesfully with unknown updater type %s", updaterType)
})
})

0 comments on commit b6335be

Please sign in to comment.