Refactor cluster status command

This patch moves all cluster status functionality from
cmd level to pkg level as well as unit tests, making code
cleaner and improving actual test coverage.

Change-Id: Ia811887b684b2129ca30dd90b5afc72e726271ff
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>

cmd/cluster/status.go

@@ -15,68 +15,26 @@
 package cluster
 
 import (
-	"fmt"
-
 	"github.com/spf13/cobra"
 
 	"opendev.org/airship/airshipctl/pkg/cluster"
 	"opendev.org/airship/airshipctl/pkg/config"
-	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/k8s/client"
-	"opendev.org/airship/airshipctl/pkg/log"
-	"opendev.org/airship/airshipctl/pkg/util"
 )
 
 // NewStatusCommand creates a command which reports the statuses of a cluster's deployed components.
 func NewStatusCommand(cfgFactory config.Factory, factory client.Factory) *cobra.Command {
+	o := cluster.NewStatusOptions(cfgFactory, factory)
 	cmd := &cobra.Command{
 		Use:   "status",
 		Short: "Retrieve statuses of deployed cluster components",
-		RunE: func(cmd *cobra.Command, args []string) error {
-			conf, err := cfgFactory()
-			if err != nil {
-				return err
-			}
-
-			manifest, err := conf.CurrentContextManifest()
-			if err != nil {
-				return err
-			}
-
-			docBundle, err := document.NewBundleByPath(manifest.TargetPath)
-			if err != nil {
-				return err
-			}
-
-			docs, err := docBundle.GetAllDocuments()
-			if err != nil {
-				return err
-			}
-
-			client, err := factory(conf)
-			if err != nil {
-				return err
-			}
-
-			statusMap, err := cluster.NewStatusMap(client)
-			if err != nil {
-				return err
-			}
-
-			tw := util.NewTabWriter(cmd.OutOrStdout())
-			fmt.Fprintf(tw, "Kind\tName\tStatus\n")
-			for _, doc := range docs {
-				status, err := statusMap.GetStatusForResource(doc)
-				if err != nil {
-					log.Debug(err)
-				} else {
-					fmt.Fprintf(tw, "%s\t%s\t%s\n", doc.GetKind(), doc.GetName(), status)
-				}
-			}
-			tw.Flush()
-			return nil
-		},
+		RunE:  clusterStatusRunE(o),
 	}
-
 	return cmd
 }
+
+func clusterStatusRunE(o cluster.StatusOptions) func(cmd *cobra.Command, args []string) error {
+	return func(cmd *cobra.Command, args []string) error {
+		return cluster.StatusRunner(o, cmd.OutOrStdout())
+	}
+}

cmd/cluster/status_test.go

@@ -17,140 +17,19 @@ package cluster_test
 import (
 	"testing"
 
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
-
 	"opendev.org/airship/airshipctl/cmd/cluster"
-	"opendev.org/airship/airshipctl/pkg/config"
-	"opendev.org/airship/airshipctl/pkg/k8s/client"
-	"opendev.org/airship/airshipctl/pkg/k8s/client/fake"
 	"opendev.org/airship/airshipctl/testutil"
 )
 
-const (
-	fixturesPath = "testdata/statusmap"
-)
-
 func TestNewClusterStatusCmd(t *testing.T) {
-	tests := []struct {
-		cmdTest   *testutil.CmdTest
-		resources []runtime.Object
-		CRDs      []runtime.Object
-	}{
+	tests := []*testutil.CmdTest{
 		{
-			cmdTest: &testutil.CmdTest{
-				Name:    "check-status-no-resources",
-				CmdLine: "",
-			},
-		},
-		{
-			cmdTest: &testutil.CmdTest{
-				Name:    "cluster-status-cmd-with-help",
-				CmdLine: "--help",
-			},
-		},
-		{
-			cmdTest: &testutil.CmdTest{
-				Name:    "check-status-with-resources",
-				CmdLine: "",
-			},
-			resources: []runtime.Object{
-				makeResource("Resource", "stable-resource", "stable"),
-				makeResource("Resource", "pending-resource", "pending"),
-			},
-			CRDs: []runtime.Object{
-				makeResourceCRD(annotationValidStatusCheck()),
-			},
+			Name:    "cluster-status-cmd-with-help",
+			CmdLine: "--help",
+			Cmd:     cluster.NewStatusCommand(nil, nil),
 		},
 	}
-
-	for _, tt := range tests {
-		tt := tt
-		testClientFactory := func(_ *config.Config) (client.Interface, error) {
-			return fake.NewClient(
-				fake.WithDynamicObjects(tt.resources...),
-				fake.WithCRDs(tt.CRDs...),
-			), nil
-		}
-		tt.cmdTest.Cmd = cluster.NewStatusCommand(clusterStatusTestSettings(), testClientFactory)
-		testutil.RunTest(t, tt.cmdTest)
-	}
-}
-
-func clusterStatusTestSettings() config.Factory {
-	return func() (*config.Config, error) {
-		return &config.Config{
-			Contexts: map[string]*config.Context{
-				"testContext": {Manifest: "testManifest"},
-			},
-			Manifests: map[string]*config.Manifest{
-				"testManifest": {TargetPath: fixturesPath},
-			},
-			CurrentContext: "testContext",
-		}, nil
-	}
-}
-
-func makeResource(kind, name, state string) *unstructured.Unstructured {
-	return &unstructured.Unstructured{
-		Object: map[string]interface{}{
-			"apiVersion": "example.com/v1",
-			"kind":       kind,
-			"metadata": map[string]interface{}{
-				"name":      name,
-				"namespace": "default",
-			},
-			"status": map[string]interface{}{
-				"state": state,
-			},
-		},
-	}
-}
-
-func annotationValidStatusCheck() map[string]string {
-	return map[string]string{
-		"airshipit.org/status-check": `
-[
-  {
-    "status": "Stable",
-    "condition": "@.status.state==\"stable\""
-  },
-  {
-    "status": "Pending",
-    "condition": "@.status.state==\"pending\""
-  }
-]`,
-	}
-}
-
-func makeResourceCRD(annotations map[string]string) *apiextensionsv1.CustomResourceDefinition {
-	return &apiextensionsv1.CustomResourceDefinition{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "CustomResourceDefinition",
-			APIVersion: "apiextensions.k8s.io/v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:        "resources.example.com",
-			Annotations: annotations,
-		},
-		Spec: apiextensionsv1.CustomResourceDefinitionSpec{
-			Group: "example.com",
-			Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
-				{
-					Name:    "v1",
-					Served:  true,
-					Storage: true,
-				},
-			},
-			// omitting the openAPIV3Schema for brevity
-			Scope: "Namespaced",
-			Names: apiextensionsv1.CustomResourceDefinitionNames{
-				Kind:     "Resource",
-				Plural:   "resources",
-				Singular: "resource",
-			},
-		},
+	for _, testcase := range tests {
+		testutil.RunTest(t, testcase)
 	}
 }

cmd/cluster/testdata/TestNewClusterStatusCmdGoldenOutput/check-status-no-resources.golden

@@ -1 +0,0 @@
-Kind   Name   Status

cmd/cluster/testdata/TestNewClusterStatusCmdGoldenOutput/check-status-with-resources.golden

@@ -1,3 +0,0 @@
-Kind       Name               Status
-Resource   pending-resource   Pending
-Resource   stable-resource    Stable

cmd/cluster/testdata/statusmap/crd.yaml

@@ -1,40 +0,0 @@
-# this CRD defines a type whose status can be checked using the condition in
-# the annotations
-apiVersion: apiextensions.k8s.io/v1
-kind: CustomResourceDefinition
-metadata:
-  name: resources.example.com
-  annotations:
-    airshipit.org/status-check: |
-      [
-        {
-          "status": "Stable",
-          "condition": "@.status.state==\"stable\""
-        },
-        {
-          "status": "Pending",
-          "condition": "@.status.state==\"pending\""
-        }
-      ]
-spec:
-  group: example.com
-  versions:
-    - name: v1
-      served: true
-      storage: true
-      schema:
-        openAPIV3Schema:
-          type: object
-          properties:
-            status:
-              type: object
-              properties:
-                state:
-                  type: string
-  scope: Namespaced
-  names:
-    plural: resources
-    singular: resource
-    kind: Resource
-    shortNames:
-    - rsc

cmd/cluster/testdata/statusmap/kustomization.yaml

@@ -1,8 +0,0 @@
-resources:
-  - crd.yaml
-  - stable-resource.yaml
-  - pending-resource.yaml
-  - missing.yaml
-  - unknown.yaml
-  - legacy-crd.yaml
-  - legacy-resource.yaml

cmd/cluster/testdata/statusmap/legacy-crd.yaml

@@ -1,42 +0,0 @@
-# this is a legacy CRD which defines a type whose status can be checked using
-# the condition in the annotations
-# It is included in tests to assure backward compatibility
-apiVersion: apiextensions.k8s.io/v1beta1
-kind: CustomResourceDefinition
-metadata:
-  name: legacies.example.com
-  annotations:
-    airshipit.org/status-check: |
-      [
-        {
-          "status": "Stable",
-          "condition": "@.status.state==\"stable\""
-        },
-        {
-          "status": "Pending",
-          "condition": "@.status.state==\"pending\""
-        }
-      ]
-spec:
-  group: example.com
-  versions:
-    - name: v1
-      served: true
-      storage: true
-  scope: Namespaced
-  names:
-    plural: legacies
-    singular: legacy
-    kind: Legacy
-    shortNames:
-    - lgc
-  preserveUnknownFields: false
-  validation:
-    openAPIV3Schema:
-      type: object
-      properties:
-        status:
-          type: object
-          properties:
-            state:
-              type: string

cmd/cluster/testdata/statusmap/legacy-resource.yaml

@@ -1,7 +0,0 @@
-# this legacy-resource is stable because the fake version in the cluster will
-# have .status.state == "stable"
-apiVersion: "example.com/v1"
-kind: Legacy
-metadata:
-  name: stable-legacy
-  namespace: default

cmd/cluster/testdata/statusmap/missing.yaml

@@ -1,7 +0,0 @@
-# This resource doesn't have a status-check defined by its CRD (which is also
-# missing for brevity). Requesting its status is an error
-apiVersion: "example.com/v1"
-kind: Missing
-metadata:
-  name: missing-resource
-  namespace: default

cmd/cluster/testdata/statusmap/pending-resource.yaml

@@ -1,7 +0,0 @@
-# this resource is pending because the fake version in the cluster will
-# have .status.state == "pending"
-apiVersion: "example.com/v1"
-kind: Resource
-metadata:
-  name: pending-resource
-  namespace: default

cmd/cluster/testdata/statusmap/stable-resource.yaml

@@ -1,7 +0,0 @@
-# this resource is stable because the fake version in the cluster will have
-# .status.state == "stable"
-apiVersion: "example.com/v1"
-kind: Resource
-metadata:
-  name: stable-resource
-  namespace: default

cmd/cluster/testdata/statusmap/unknown.yaml

@@ -1,8 +0,0 @@
-# this resource is in an unknown state because the fake version in the cluster
-# will have .status.state == "unknown", which does not correlate to any of the
-# status checks in the CRD.
-apiVersion: "example.com/v1"
-kind: Resource
-metadata:
-  name: unknown
-  namespace: default

pkg/cluster/command.go

@@ -0,0 +1,52 @@
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+     https://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cluster
+
+import (
+	"fmt"
+	"io"
+
+	"opendev.org/airship/airshipctl/pkg/log"
+	"opendev.org/airship/airshipctl/pkg/util"
+)
+
+// StatusRunner runs internal logic of cluster status command
+func StatusRunner(o StatusOptions, w io.Writer) error {
+	statusMap, docs, err := o.GetStatusMapDocs()
+	if err != nil {
+		return err
+	}
+
+	var errors []error
+	tw := util.NewTabWriter(w)
+	fmt.Fprintf(tw, "Kind\tName\tStatus\n")
+	for _, doc := range docs {
+		status, err := statusMap.GetStatusForResource(doc)
+		if err != nil {
+			errors = append(errors, err)
+		} else {
+			fmt.Fprintf(tw, "%s\t%s\t%s\n", doc.GetKind(), doc.GetName(), status)
+		}
+	}
+	tw.Flush()
+
+	if len(errors) > 0 {
+		log.Debug("The following errors occurred while requesting the status:")
+		for _, statusErr := range errors {
+			log.Debug(statusErr)
+		}
+	}
+	return nil
+}

pkg/cluster/command_test.go

@@ -0,0 +1,62 @@
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+     https://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cluster_test
+
+import (
+	"bytes"
+	"fmt"
+	"regexp"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"opendev.org/airship/airshipctl/pkg/cluster"
+	"opendev.org/airship/airshipctl/pkg/document"
+	"opendev.org/airship/airshipctl/pkg/k8s/client/fake"
+)
+
+type mockStatusOptions struct{}
+
+func (o mockStatusOptions) GetStatusMapDocs() (*cluster.StatusMap, []document.Document, error) {
+	fakeClient := fake.NewClient(
+		fake.WithCRDs(makeResourceCRD(annotationValidStatusCheck())),
+		fake.WithDynamicObjects(makeResource("stable-resource", "stable")))
+	fakeSM, err := cluster.NewStatusMap(fakeClient)
+	if err != nil {
+		return nil, nil, err
+	}
+	fakeDocBundle, err := document.NewBundleByPath("testdata/statusmap")
+	if err != nil {
+		return nil, nil, err
+	}
+
+	fakeDocs, err := fakeDocBundle.GetAllDocuments()
+	if err != nil {
+		return nil, nil, err
+	}
+	return fakeSM, fakeDocs, nil
+}
+
+func TestStatusRunner(t *testing.T) {
+	statusOptions := mockStatusOptions{}
+	b := bytes.NewBuffer(nil)
+	err := cluster.StatusRunner(statusOptions, b)
+	require.NoError(t, err)
+	expectedOutput := fmt.Sprintf("Kind Name Status Resource stable-resource Stable ")
+	space := regexp.MustCompile(`\s+`)
+	str := space.ReplaceAllString(b.String(), " ")
+	assert.Equal(t, expectedOutput, str)
+}

pkg/cluster/status.go

@@ -29,10 +29,64 @@ import (
 	"sigs.k8s.io/cli-utils/pkg/kstatus/status"
 	"sigs.k8s.io/cli-utils/pkg/object"
 
+	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/k8s/client"
 )
 
+// A Status represents a kubernetes resource's state.
+type Status string
+
+// StatusOptions provides a way to get status map within all the documents in the bundle
+type StatusOptions interface {
+	GetStatusMapDocs() (*StatusMap, []document.Document, error)
+}
+
+type statusOptions struct {
+	ConfigFactory config.Factory
+	ClientFactory client.Factory
+}
+
+// NewStatusOptions constructs a new StatusOptions interface based on inner struct
+func NewStatusOptions(cfgFactory config.Factory, clientFactory client.Factory) StatusOptions {
+	return &statusOptions{ConfigFactory: cfgFactory, ClientFactory: clientFactory}
+}
+
+// GetStatusMapDocs returns status map within all the documents in the bundle
+func (o *statusOptions) GetStatusMapDocs() (*StatusMap, []document.Document, error) {
+	conf, err := o.ConfigFactory()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	manifest, err := conf.CurrentContextManifest()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	docBundle, err := document.NewBundleByPath(manifest.TargetPath)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	docs, err := docBundle.GetAllDocuments()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	client, err := o.ClientFactory(conf)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	statusMap, err := NewStatusMap(client)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return statusMap, docs, nil
+}
+
 // StatusMap holds a mapping of schema.GroupVersionResource to various statuses
 // a resource may be in, as well as the Expression used to check for that
 // status.

pkg/cluster/status_test.go

@@ -23,16 +23,78 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/cli-utils/pkg/kstatus/status"
 	"sigs.k8s.io/cli-utils/pkg/object"
 
 	"opendev.org/airship/airshipctl/pkg/cluster"
+	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/document"
+	"opendev.org/airship/airshipctl/pkg/k8s/client"
 	"opendev.org/airship/airshipctl/pkg/k8s/client/fake"
 	"opendev.org/airship/airshipctl/testutil"
 )
 
+func TestGetStatusMapDocs(t *testing.T) {
+	tests := []struct {
+		name      string
+		resources []runtime.Object
+		CRDs      []runtime.Object
+	}{
+		{
+			name: "get-status-map-docs-no-resources",
+		},
+		{
+			name: "get-status-map-docs-with-resources",
+			resources: []runtime.Object{
+				makeResource("stable-resource", "stable"),
+				makeResource("pending-resource", "pending"),
+			},
+			CRDs: []runtime.Object{
+				makeResourceCRD(annotationValidStatusCheck()),
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		settings := clusterStatusTestSettings()
+		fakeClient := fake.NewClient(
+			fake.WithDynamicObjects(tt.resources...),
+			fake.WithCRDs(tt.CRDs...))
+		statusOptions := cluster.NewStatusOptions(func() (*config.Config, error) {
+			return settings, nil
+		}, func(_ *config.Config) (client.Interface, error) {
+			return fakeClient, nil
+		})
+
+		expectedSM, err := cluster.NewStatusMap(fakeClient)
+		require.NoError(t, err)
+		docBundle, err := document.NewBundleByPath(settings.Manifests["testManifest"].TargetPath)
+		require.NoError(t, err)
+		expectedDocs, err := docBundle.GetAllDocuments()
+		require.NoError(t, err)
+
+		sm, docs, err := statusOptions.GetStatusMapDocs()
+		require.NoError(t, err)
+		assert.Equal(t, expectedSM, sm)
+		assert.Equal(t, expectedDocs, docs)
+	}
+}
+
+func clusterStatusTestSettings() *config.Config {
+	return &config.Config{
+		Contexts: map[string]*config.Context{
+			"testContext": {Manifest: "testManifest"},
+		},
+		Manifests: map[string]*config.Manifest{
+			"testManifest": {TargetPath: "testdata/statusmap"},
+		},
+		CurrentContext: "testContext",
+	}
+}
+
 func TestNewStatusMap(t *testing.T) {
 	tests := []struct {
 		name   string