Merge pull request #80 from aweiteka/cleanup-cost-output

Clean up cost output

cmd/cost/cost.go

@@ -1,14 +1,12 @@
 package cost
 
 import (
-	"fmt"
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/organizations"
 	"github.com/openshift/osd-utils-cli/cmd/common"
 	awsprovider "github.com/openshift/osd-utils-cli/pkg/provider/aws"
 	"github.com/spf13/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
-	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 	"log"
 )
 
@@ -20,9 +18,6 @@ func NewCmdCost(streams genericclioptions.IOStreams) *cobra.Command {
 		Short: "Cost Management related utilities",
 		Long: `The cost command allows for cost management on the AWS platform (other 
 platforms may be added in the future)`,
-		PersistentPreRun: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(opsCost.complete(cmd, args))
-		},
 	}
 
 	//Set flags
@@ -61,16 +56,6 @@ func newCostOptions(streams genericclioptions.IOStreams) *costOptions {
 	}
 }
 
-func (opsCost *costOptions) complete(cmd *cobra.Command, _ []string) error {
-	if opsCost.accessKeyID == "" && opsCost.secretAccessKey == "" {
-		fmt.Fprintln(opsCost.Out, "AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY are not provided, reading credentials from config file or env vars.")
-	} else if opsCost.accessKeyID == "" || opsCost.secretAccessKey == "" {
-		return cmdutil.UsageErrorf(cmd, "The flag aws-access-key-id and aws-secret-access-key should be set or not set at the same time")
-	}
-
-	return nil
-}
-
 //Initiate AWS clients for Organizations and Cost Explorer services using, if given, credentials in flags, else, credentials in the environment
 func (opsCost *costOptions) initAWSClients() (awsprovider.Client, error) {
 	//Initialize AWS clients

cmd/cost/get.go

@@ -21,7 +21,7 @@ func newCmdGet(streams genericclioptions.IOStreams) *cobra.Command {
 	ops := newGetOptions(streams)
 	getCmd := &cobra.Command{
 		Use:   "get",
-		Short: "Get total cost of a given OU. If no OU given, then gets total cost of v4 OU.",
+		Short: "Get total cost of a given OU",
 		Run: func(cmd *cobra.Command, args []string) {
 
 			awsClient, err := opsCost.initAWSClients()
@@ -47,12 +47,15 @@ func newCmdGet(streams genericclioptions.IOStreams) *cobra.Command {
 	}
 	getCmd.Flags().StringVar(&ops.ou, "ou", "", "get OU ID")
 	getCmd.Flags().BoolVarP(&ops.recursive, "recursive", "r", false, "recurse through OUs")
-	getCmd.Flags().StringVarP(&ops.time, "time", "t", "", "set time")
+	getCmd.Flags().StringVarP(&ops.time, "time", "t", "", "set time. One of 'LM', 'MTD', 'TYD', '3M', '6M', '1Y'")
 	getCmd.Flags().BoolVar(&ops.csv, "csv", false, "output result as csv")
 
 	if err := getCmd.MarkFlagRequired("ou"); err != nil {
 		log.Fatalln("OU flag:", err)
 	}
+	if err := getCmd.MarkFlagRequired("time"); err != nil {
+		log.Fatalln("time flag:", err)
+	}
 
 	return getCmd
 }

cmd/cost/list.go

@@ -30,12 +30,17 @@ func newCmdList(streams genericclioptions.IOStreams) *cobra.Command {
 		},
 	}
 	listCmd.Flags().StringVar(&ops.ou, "ou", "", "get OU ID")
-	listCmd.Flags().StringVarP(&ops.time, "time", "t", "", "set time")
+	// list supported time args
+	listCmd.Flags().StringVarP(&ops.time, "time", "t", "", "set time. One of 'LM', 'MTD', 'TYD', '3M', '6M', '1Y'")
 	listCmd.Flags().BoolVar(&ops.csv, "csv", false, "output result as csv")
 
 	if err := listCmd.MarkFlagRequired("ou"); err != nil {
 		log.Fatalln("OU flag:", err)
 	}
+	// require explicit time set
+	if err := listCmd.MarkFlagRequired("time"); err != nil {
+		log.Fatalln("time flag:", err)
+	}
 
 	return listCmd
 }
@@ -90,16 +95,16 @@ func listCostsUnderOU(OU *organizations.OrganizationalUnit, awsClient awsprovide
 func printCostList(cost float64, unit string, OU *organizations.OrganizationalUnit, ops *listOptions, isChildNode bool) {
 	if !isChildNode {
 		if ops.csv {
-			fmt.Printf("\nOU,Cost(%s)\n%v,%f\n", unit, *OU.Name, cost)
+			fmt.Printf("OU,Name,Cost (%s)\n", unit)
 		} else {
-			fmt.Printf("\nListing costs of OU (%s, %s) and all its child OUs:\n\n", *OU.Id, *OU.Name)
-			fmt.Printf("%-30s%-30s%-30s%-30s\n", "OU ID", "OU Name", "Cost", "Unit")
+			fmt.Printf("Costs of OU %s '%s' and its child OUs:\n\n", *OU.Id, *OU.Name)
+			fmt.Printf("%-20s%-30s%-20s\n", "OU ID", "OU Name", "Cost")
 		}
 	}
 
 	if ops.csv {
-		fmt.Printf("%v,%f\n", *OU.Name, cost)
+		fmt.Printf("%v,%v,%.2f\n", *OU.Id, *OU.Name, cost)
 	} else {
-		fmt.Printf("%-30s%-30s%-30f%-30s\n", *OU.Id, *OU.Name, cost, unit)
+		fmt.Printf("%-20s%-30s%.2f\n", *OU.Id, *OU.Name, cost)
 	}
 }

docs/command/osdctl_cost.md

@@ -40,7 +40,7 @@ platforms may be added in the future)
 
 * [osdctl](osdctl.md)	 - OSD CLI
 * [osdctl cost create](osdctl_cost_create.md)	 - Create a cost category for the given OU
-* [osdctl cost get](osdctl_cost_get.md)	 - Get total cost of a given OU. If no OU given, then gets total cost of v4 OU.
+* [osdctl cost get](osdctl_cost_get.md)	 - Get total cost of a given OU
 * [osdctl cost list](osdctl_cost_list.md)	 - List the cost of each OU under given OU
 * [osdctl cost reconcile](osdctl_cost_reconcile.md)	 - Checks if there's a cost category for every OU. If an OU is missing a cost category, creates the cost category
 

docs/command/osdctl_cost_get.md

@@ -1,10 +1,10 @@
 ## osdctl cost get
 
-Get total cost of a given OU. If no OU given, then gets total cost of v4 OU.
+Get total cost of a given OU
 
 ### Synopsis
 
-Get total cost of a given OU. If no OU given, then gets total cost of v4 OU.
+Get total cost of a given OU
 
 ```
 osdctl cost get [flags]
@@ -17,7 +17,7 @@ osdctl cost get [flags]
   -h, --help          help for get
       --ou string     get OU ID
   -r, --recursive     recurse through OUs
-  -t, --time string   set time
+  -t, --time string   set time. One of 'LM', 'MTD', 'TYD', '3M', '6M', '1Y'
 ```
 
 ### Options inherited from parent commands

docs/command/osdctl_cost_list.md

@@ -16,7 +16,7 @@ osdctl cost list [flags]
       --csv           output result as csv
   -h, --help          help for list
       --ou string     get OU ID
-  -t, --time string   set time
+  -t, --time string   set time. One of 'LM', 'MTD', 'TYD', '3M', '6M', '1Y'
 ```
 
 ### Options inherited from parent commands

pkg/provider/aws/client.go

@@ -97,11 +97,7 @@ func NewAwsClient(profile, region, configFile string) (Client, error) {
 
 	if profile == "" && configFile == "" {
 		fmt.Println("Config file and profile are not provided. Reading from env vars.")
-	} else if profile == "" {
-		fmt.Println("No profile provided. Reading from env vars, otherwise, please provide profile to use with config file.")
-	} else if configFile == "" {
-		fmt.Println("No config file provided. Reading from env vars, otherwise, please provide config file to use for profile.")
-	} else { // only set config file if it is not empty
+	} else if configFile != "" { // only set config file if it is not empty
 		absCfgPath, err := filepath.Abs(configFile)
 		if err != nil {
 			return nil, err