From 2bbc95d9e8624929758c1b9af37a6f25e6cf7926 Mon Sep 17 00:00:00 2001 From: chrislovecnm Date: Mon, 28 Nov 2016 18:54:57 -0700 Subject: [PATCH] changes from code review --- cmd/kops/validate.go | 6 ++---- cmd/kops/validate_cluster.go | 9 ++++----- docs/cli/kops.md | 2 +- docs/cli/kops_validate.md | 10 ++-------- docs/cli/kops_validate_cluster.md | 3 +-- pkg/apis/kops/node_api_adapter.go | 3 +-- pkg/apis/kops/validate_cluster.go | 14 +++++--------- pkg/apis/kops/validate_cluster_test.go | 8 +++----- 8 files changed, 19 insertions(+), 36 deletions(-) diff --git a/cmd/kops/validate.go b/cmd/kops/validate.go index c26b0daeb3..861ecf7833 100644 --- a/cmd/kops/validate.go +++ b/cmd/kops/validate.go @@ -31,8 +31,8 @@ var validateCmd = ValidateCmd{ cobraCommand: &cobra.Command{ Use: "validate", SuggestFor: []string{"list"}, - Short: "validate a kubernetes cluster", - Long: `validate a kubernetes cluster`, + Short: "Validate Cluster", + Long: `Validate a Kubernetes Cluster`, }, } @@ -40,6 +40,4 @@ func init() { cmd := validateCmd.cobraCommand rootCommand.AddCommand(cmd) - - cmd.PersistentFlags().StringVarP(&validateCmd.output, "output", "o", OutputTable, "output format. One of: table, yaml") } diff --git a/cmd/kops/validate_cluster.go b/cmd/kops/validate_cluster.go index 52fa5bd5d2..aca332a98f 100644 --- a/cmd/kops/validate_cluster.go +++ b/cmd/kops/validate_cluster.go @@ -59,7 +59,7 @@ func (c *ValidateClusterCmd) Run(args []string) error { err := rootCommand.ProcessArgs(args) if err != nil { - return fmt.Errorf("Process args filed %v", err) + return fmt.Errorf("Process args failed %v", err) } cluster, err := rootCommand.Cluster() @@ -80,9 +80,8 @@ func (c *ValidateClusterCmd) Run(args []string) error { fmt.Printf("Validating cluster %v\n\n", cluster.Name) var instanceGroups []*api.InstanceGroup - for i := range list.Items { - ig := &list.Items[i] - instanceGroups = append(instanceGroups, ig) + for _, ig := range list.Items { + instanceGroups = append(instanceGroups, &ig) } if len(instanceGroups) == 0 { @@ -158,7 +157,7 @@ func (c *ValidateClusterCmd) Run(args []string) error { fmt.Printf("mastersCount %v, mastersReady %v", validationCluster.MastersCount, len(validationCluster.MastersReadyArray)) fmt.Printf("nodesNotReady %v", len(validationCluster.NodesNotReadyArray)) fmt.Printf("nodesCount %v, nodesReady %v", validationCluster.NodesCount, len(validationCluster.NodesReadyArray)) - return fmt.Errorf("\nYou cluster %s is NOT ready.", cluster.Name) + return fmt.Errorf("\nYour cluster %s is NOT ready.", cluster.Name) } } diff --git a/docs/cli/kops.md b/docs/cli/kops.md index 66f235b93f..0a0d230976 100644 --- a/docs/cli/kops.md +++ b/docs/cli/kops.md @@ -37,6 +37,6 @@ It allows you to create, destroy, upgrade and maintain clusters. * [kops toolbox](kops_toolbox.md) - Misc infrequently used commands * [kops update](kops_update.md) - update clusters * [kops upgrade](kops_upgrade.md) - upgrade clusters -* [kops validate](kops_validate.md) - validate a kubernetes cluster +* [kops validate](kops_validate.md) - Validate Cluster * [kops version](kops_version.md) - Print the client version information diff --git a/docs/cli/kops_validate.md b/docs/cli/kops_validate.md index 888c18fece..db2f64695c 100644 --- a/docs/cli/kops_validate.md +++ b/docs/cli/kops_validate.md @@ -1,17 +1,11 @@ ## kops validate -validate a kubernetes cluster +Validate Cluster ### Synopsis -validate a kubernetes cluster - -### Options - -``` - -o, --output string output format. One of: table, yaml (default "table") -``` +Validate a Kubernetes Cluster ### Options inherited from parent commands diff --git a/docs/cli/kops_validate_cluster.md b/docs/cli/kops_validate_cluster.md index 8df5ea833d..c4899edcc1 100644 --- a/docs/cli/kops_validate_cluster.md +++ b/docs/cli/kops_validate_cluster.md @@ -20,7 +20,6 @@ kops validate cluster --log_dir string If non-empty, write log files in this directory --logtostderr log to standard error instead of files (default false) --name string Name of cluster - -o, --output string output format. One of: table, yaml (default "table") --state string Location of state storage --stderrthreshold severity logs at or above this threshold go to stderr (default 2) -v, --v Level log level for V logs @@ -28,5 +27,5 @@ kops validate cluster ``` ### SEE ALSO -* [kops validate](kops_validate.md) - validate a kubernetes cluster +* [kops validate](kops_validate.md) - Validate Cluster diff --git a/pkg/apis/kops/node_api_adapter.go b/pkg/apis/kops/node_api_adapter.go index 53bdf9d451..105ef00232 100644 --- a/pkg/apis/kops/node_api_adapter.go +++ b/pkg/apis/kops/node_api_adapter.go @@ -163,8 +163,7 @@ func (nodeAA *NodeAPIAdapter) WaitForNodeToBe(conditionType v1.NodeConditionType glog.V(4).Infof("Couldn't get node %s", nodeAA.nodeName) continue } - var iSet bool - iSet, err = IsNodeConditionSetAsExpected(node, conditionType, wantTrue) + iSet, err := IsNodeConditionSetAsExpected(node, conditionType, wantTrue) if err != nil { glog.V(4).Infof("IsNodeConditionSetAsExpected failed for node %s, %v", nodeAA.nodeName, err) diff --git a/pkg/apis/kops/validate_cluster.go b/pkg/apis/kops/validate_cluster.go index 01844b6720..024532a5a1 100644 --- a/pkg/apis/kops/validate_cluster.go +++ b/pkg/apis/kops/validate_cluster.go @@ -17,7 +17,6 @@ limitations under the License. package kops import ( - "errors" "fmt" "time" @@ -69,7 +68,7 @@ func ValidateCluster(clusterName string, instanceGroupList *InstanceGroupList) ( } if len(instanceGroups) == 0 { - return validationCluster, errors.New("No InstanceGroup objects found\n") + return validationCluster, fmt.Errorf("No InstanceGroup objects found\n") } nodeAA := &NodeAPIAdapter{} @@ -88,18 +87,15 @@ func ValidateCluster(clusterName string, instanceGroupList *InstanceGroupList) ( return nil, fmt.Errorf("Cannot get nodes for %q: %v", clusterName, err) } - return validateTheNodes(clusterName,validationCluster) + return validateTheNodes(clusterName, validationCluster) } - - - -func validateTheNodes(clusterName string, validationCluster *ValidationCluster) (*ValidationCluster, error) { +func validateTheNodes(clusterName string, validationCluster *ValidationCluster) (*ValidationCluster, error) { nodes := validationCluster.NodeList if nodes == nil || nodes.Items == nil { - return validationCluster, errors.New("No nodes found in validationCluster") + return validationCluster, fmt.Errorf("No nodes found in validationCluster") } for _, node := range nodes.Items { @@ -152,6 +148,6 @@ func validateTheNodes(clusterName string, validationCluster *ValidationCluster) if validationCluster.MastersReady && validationCluster.NodesReady { return validationCluster, nil } else { - return validationCluster, fmt.Errorf("You cluster is NOT ready %s", clusterName) + return validationCluster, fmt.Errorf("Your cluster is NOT ready %s", clusterName) } } diff --git a/pkg/apis/kops/validate_cluster_test.go b/pkg/apis/kops/validate_cluster_test.go index 97ba58e6bc..9aac6676a2 100644 --- a/pkg/apis/kops/validate_cluster_test.go +++ b/pkg/apis/kops/validate_cluster_test.go @@ -210,11 +210,9 @@ func dummyNode(nodeMap map[string]string) v1.Node { // MakeNodeList constructs api.NodeList from list of node names and a NodeResource. func makeNodeList(nodes []map[string]string) *v1.NodeList { - list := v1.NodeList{ - Items: make([]v1.Node, len(nodes)), - } - for i := range nodes { - list.Items[i] = dummyNode(nodes[i]) + var list v1.NodeList + for _, node := range nodes { + list.Items = append(list.Items, dummyNode(node)) } return &list }