review fix

clusterctl/cmd/validate_cluster.go

@@ -49,19 +49,23 @@ var validateClusterCmd = &cobra.Command{
 func init() {
 	validateClusterCmd.Flags().StringVarP(
 		&vco.Kubeconfig, "kubeconfig", "", "",
-		"The file path of the kubeconfig file for Cluster API management stack. If not specified, $KUBECONFIG environment variable or ${HOME}/.kube/config is used.")
+		"The file path of the kubeconfig file for the cluster to validate.. If not specified, $KUBECONFIG environment variable or ${HOME}/.kube/config is used.")
 	// BindContextFlags will bind the flags cluster, namespace, and user
 	tcmd.BindContextFlags(&do.KubeconfigOverrides.Context, validateClusterCmd.Flags(), tcmd.RecommendedContextOverrideFlags(""))
 	validateCmd.AddCommand(validateClusterCmd)
 }
 
 func RunValidateCluster() error {
-	client, err := clientcmd.NewClusterApiClientForDefaultSearchPath(vco.Kubeconfig, vco.KubeconfigOverrides)
+	clusterApiClient, err := clientcmd.NewClusterApiClientForDefaultSearchPath(vco.Kubeconfig, vco.KubeconfigOverrides)
 	if err != nil {
-		return fmt.Errorf("failed to create client for talking to the apiserver: %v", err)
+		return fmt.Errorf("failed to create cluster API client: %v", err)
+	}
+	k8sClient, err := clientcmd.NewCoreClientSetForDefaultSearchPath(vco.Kubeconfig, vco.KubeconfigOverrides)
+	if err != nil {
+		return fmt.Errorf("failed to create kubernetes client: %v", err)
 	}
 
-	if err = validation.ValidateClusterAPIObjects(os.Stdout, client, vco.KubeconfigOverrides.Context.Cluster, vco.KubeconfigOverrides.Context.Namespace); err != nil {
+	if err = validation.ValidateClusterAPIObjects(os.Stdout, clusterApiClient, k8sClient, vco.KubeconfigOverrides.Context.Cluster, vco.KubeconfigOverrides.Context.Namespace); err != nil {
 		return err
 	}
 

clusterctl/testdata/validate-cluster-no-args-invalid-flag.golden

@@ -5,7 +5,7 @@ Usage:
 Flags:
       --cluster string      The name of the kubeconfig cluster to use
   -h, --help                help for cluster
-      --kubeconfig string   The file path of the kubeconfig file for Cluster API management stack. If not specified, $KUBECONFIG environment variable or ${HOME}/.kube/config is used.
+      --kubeconfig string   The file path of the kubeconfig file for the cluster to validate.. If not specified, $KUBECONFIG environment variable or ${HOME}/.kube/config is used.
   -n, --namespace string    If present, the namespace scope for this CLI request
       --user string         The name of the kubeconfig user to use
 

clusterctl/validation/testdata/fail-to-validate-machine-objects-with-noderef-errors.golden

@@ -0,0 +1,6 @@
+Validating cluster API objects in namespace "validate-cluster-api-object-output"
+Checking cluster object "test-cluster"... PASS
+Checking machine object "test-machine1"... FAIL
+	The referred node test-node-not-ready is not ready.
+Checking machine object "test-machine2"... FAIL
+	Unable to get the referred node test-node-not-exist: nodes "test-node-not-exist" not found

clusterctl/validation/validate_cluster_api_objects.go

@@ -21,43 +21,42 @@ import (
 	"io"
 
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 	"sigs.k8s.io/cluster-api/pkg/apis/cluster/common"
 	"sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
 	"sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset"
+	"sigs.k8s.io/cluster-api/pkg/controller/noderefutil"
 )
 
-func ValidateClusterAPIObjects(w io.Writer, client *clientset.Clientset, clusterName string, namespace string) error {
-	fmt.Fprintf(w,"Validating cluster API objects in namespace %q\n", namespace)
+func ValidateClusterAPIObjects(w io.Writer, clusterApiClient *clientset.Clientset, k8sClient kubernetes.Interface, clusterName string, namespace string) error {
+	fmt.Fprintf(w, "Validating cluster API objects in namespace %q\n", namespace)
 
-	cluster, err := getClusterObject(client, clusterName, namespace)
+	cluster, err := getClusterObject(clusterApiClient, clusterName, namespace)
 	if err != nil {
 		return err
 	}
 	if err := validateClusterObject(w, cluster); err != nil {
 		return err
 	}
 
-	machines, err := client.ClusterV1alpha1().Machines(namespace).List(meta_v1.ListOptions{})
+	machines, err := clusterApiClient.ClusterV1alpha1().Machines(namespace).List(meta_v1.ListOptions{})
 	if err != nil {
 		return fmt.Errorf("failed to get the machines from the apiserver in namespace %q: %v", namespace, err)
 	}
-	if err = validateMachineObjects(w, machines); err != nil {
-		return err
-	}
 
-	return nil
+	return validateMachineObjects(w, machines, k8sClient)
 }
 
-func getClusterObject(client *clientset.Clientset, clusterName string, namespace string) (*v1alpha1.Cluster, error) {
+func getClusterObject(clusterApiClient *clientset.Clientset, clusterName string, namespace string) (*v1alpha1.Cluster, error) {
 	if clusterName != "" {
-		cluster, err := client.ClusterV1alpha1().Clusters(namespace).Get(clusterName, meta_v1.GetOptions{})
+		cluster, err := clusterApiClient.ClusterV1alpha1().Clusters(namespace).Get(clusterName, meta_v1.GetOptions{})
 		if err != nil {
 			return nil, fmt.Errorf("failed to get the cluster %q from the apiserver in namespace %q: %v", clusterName, namespace, err)
 		}
 		return cluster, nil
 	}
 
-	clusters, err := client.ClusterV1alpha1().Clusters(namespace).List(meta_v1.ListOptions{})
+	clusters, err := clusterApiClient.ClusterV1alpha1().Clusters(namespace).List(meta_v1.ListOptions{})
 	if err != nil {
 		return nil, fmt.Errorf("failed to get the clusters from the apiserver in namespace %q: %v", namespace, err)
 	}
@@ -70,18 +69,31 @@ func getClusterObject(client *clientset.Clientset, clusterName string, namespace
 }
 
 func validateClusterObject(w io.Writer, cluster *v1alpha1.Cluster) error {
-	fmt.Fprintf(w,"Checking cluster object %q... ", cluster.Name)
+	fmt.Fprintf(w, "Checking cluster object %q... ", cluster.Name)
 	if cluster.Status.ErrorReason != "" || cluster.Status.ErrorMessage != "" {
-		fmt.Fprintf(w,"FAIL\n")
-		fmt.Fprintf(w,"\t[%v]: %s\n", cluster.Status.ErrorReason, cluster.Status.ErrorMessage)
-		return fmt.Errorf("failed to validate the cluster %q.", cluster.Name)
+		fmt.Fprintf(w, "FAIL\n")
+		fmt.Fprintf(w, "\t[%v]: %s\n", cluster.Status.ErrorReason, cluster.Status.ErrorMessage)
+		return fmt.Errorf("Cluster %q failed the validation.", cluster.Name)
+	}
+	fmt.Fprintf(w, "PASS\n")
+	return nil
+}
+
+func validateMachineObjects(w io.Writer, machines *v1alpha1.MachineList, k8sClient kubernetes.Interface) error {
+	pass := true
+	for _, machine := range machines.Items {
+		if (!validateMachineObject(w, machine, k8sClient)) {
+			pass = false
+		}
+	}
+	if !pass {
+		return fmt.Errorf("Machine objects failed the validation.")
 	}
-	fmt.Fprintf(w,"PASS\n")
 	return nil
 }
 
-func validateMachineObject(w io.Writer, machine v1alpha1.Machine) bool {
-	fmt.Fprintf(w,"Checking machine object %q... ", machine.Name)
+func validateMachineObject(w io.Writer, machine v1alpha1.Machine, k8sClient kubernetes.Interface) bool {
+	fmt.Fprintf(w, "Checking machine object %q... ", machine.Name)
 	if machine.Status.ErrorReason != nil || machine.Status.ErrorMessage != nil {
 		var reason common.MachineStatusError = ""
 		if machine.Status.ErrorReason != nil {
@@ -91,28 +103,32 @@ func validateMachineObject(w io.Writer, machine v1alpha1.Machine) bool {
 		if machine.Status.ErrorMessage != nil {
 			message = *machine.Status.ErrorMessage
 		}
-		fmt.Fprintf(w,"FAIL\n")
-		fmt.Fprintf(w,"\t[%v]: %s\n", reason, message)
+		fmt.Fprintf(w, "FAIL\n")
+		fmt.Fprintf(w, "\t[%v]: %s\n", reason, message)
 		return false
 	}
 	if machine.Status.NodeRef == nil {
-		fmt.Fprintf(w,"FAIL\n")
-		fmt.Fprintf(w,"\tThe corresponding node does not exist.\n")
+		fmt.Fprintf(w, "FAIL\n")
+		fmt.Fprintf(w, "\tThe corresponding node does not exist.\n")
+		return false
+	}
+	err := validateReferredNode(w, machine.Status.NodeRef.Name, k8sClient)
+	if err != nil {
+		fmt.Fprintf(w, "FAIL\n")
+		fmt.Fprintf(w, "\t%v\n", err)
 		return false
 	}
-	fmt.Fprintf(w,"PASS\n")
+	fmt.Fprintf(w, "PASS\n")
 	return true
 }
 
-func validateMachineObjects(w io.Writer, machines *v1alpha1.MachineList) error {
-	pass := true
-	for _, machine := range machines.Items {
-		if (!validateMachineObject(w, machine)) {
-			pass = false
-		}
+func validateReferredNode(w io.Writer, nodeName string, k8sClient kubernetes.Interface) error {
+	node, err := k8sClient.CoreV1().Nodes().Get(nodeName, meta_v1.GetOptions{})
+	if err != nil {
+		return fmt.Errorf("Unable to get the referred node %s: %v", nodeName, err)
 	}
-	if !pass {
-		return fmt.Errorf("failed to validate some machine object.")
+	if !noderefutil.IsNodeReady(node) {
+		return fmt.Errorf("The referred node %s is not ready.", nodeName)
 	}
 	return nil
 }