Refactor of codebase addressing IDE errors

Goland is complaining of a number of bad practices, and unused code.
This patchset addresses some of those complaints, such as:
- struct initialization without field names
- error string should not be capitalized or end with punctuation
- name starts with package name
- duplicate yaml keys
- snake case
- redundant parentheses

For the http.Transport redundant parentheses, the lint target fails
without the use of //nolint:errcheck so it has been added as
(currently) there is no potential for there to be an error returned
see [0] for example.

[0] https://golang.org/src/net/http/transport.go#L42

Change-Id: Ib1b20639c9b6e9be097ef1f158ecd7472f578488
Signed-off-by: Alexander Hughes <Alexander.Hughes@pm.me>

cmd/cluster/cluster.go

@@ -20,15 +20,10 @@ import (
 	"opendev.org/airship/airshipctl/pkg/environment"
 )
 
-var (
-	// ClusterUse subcommand string
-	ClusterUse = "cluster"
-)
-
 // NewClusterCommand returns cobra command object of the airshipctl cluster and adds it's subcommands.
 func NewClusterCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command {
 	clusterRootCmd := &cobra.Command{
-		Use: ClusterUse,
+		Use: "cluster",
 		// TODO: (kkalynovskyi) Add more description when more subcommands are added
 		Short: "Control Kubernetes cluster",
 		Long:  "Interactions with Kubernetes cluster, such as get status, deploy initial infrastructure",

cmd/config/get_authinfo_test.go

@@ -69,7 +69,7 @@ func TestGetAuthInfoCmd(t *testing.T) {
 			CmdLine: fmt.Sprintf("%s", missingAuthInfo),
 			Cmd:     cmd.NewCmdConfigGetAuthInfo(settings),
 			Error: fmt.Errorf("user %s information was not "+
-				"found in the configuration.", missingAuthInfo),
+				"found in the configuration", missingAuthInfo),
 		},
 	}
 

cmd/config/get_cluster_test.go

@@ -110,7 +110,7 @@ func TestGetClusterCmd(t *testing.T) {
 			CmdLine: fmt.Sprintf("%s %s", targetFlag, missingCluster),
 			Cmd:     cmd.NewCmdConfigGetCluster(settings),
 			Error: fmt.Errorf("cluster clustermissing information was not " +
-				"found in the configuration."),
+				"found in the configuration"),
 		},
 	}
 

pkg/cluster/status_test.go

@@ -24,7 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/dynamic"
-	dynamic_fake "k8s.io/client-go/dynamic/fake"
+	dynamicFake "k8s.io/client-go/dynamic/fake"
 
 	"opendev.org/airship/airshipctl/pkg/cluster"
 	"opendev.org/airship/airshipctl/pkg/document"
@@ -149,7 +149,7 @@ func TestGetStatusForResource(t *testing.T) {
 				ByGvk("example.com", "v1", "Missing").
 				ByName("missing-resource"),
 			testClient: makeTestClient(),
-			err:        cluster.ErrResourceNotFound{"missing-resource"},
+			err:        cluster.ErrResourceNotFound{Resource: "missing-resource"},
 		},
 	}
 
@@ -180,7 +180,7 @@ func TestGetStatusForResource(t *testing.T) {
 func makeTestClient(obj ...runtime.Object) fake.Client {
 	testClient := fake.Client{
 		MockDynamicClient: func() dynamic.Interface {
-			return dynamic_fake.NewSimpleDynamicClient(runtime.NewScheme(), obj...)
+			return dynamicFake.NewSimpleDynamicClient(runtime.NewScheme(), obj...)
 		},
 	}
 	return testClient

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

@@ -3,7 +3,6 @@
 # It is included in tests to assure backward compatibility
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
-metadata:
 metadata:
   name: legacies.example.com
   annotations:

pkg/container/container_docker_test.go

@@ -393,7 +393,7 @@ func TestRunCommand(t *testing.T) {
 					return resC, nil
 				},
 				containerLogs: func() (io.ReadCloser, error) {
-					return nil, fmt.Errorf("Logs error")
+					return nil, fmt.Errorf("logs error")
 				},
 			},
 			expectedErr: ErrRunContainerCommand{Cmd: "docker logs testID"},

pkg/document/filesystem.go

@@ -32,17 +32,17 @@ type FileSystem interface {
 	TempFile(string, string) (File, error)
 }
 
-// DocumentFs is adaptor to TempFile
-type DocumentFs struct {
+// Fs is adaptor to TempFile
+type Fs struct {
 	fs.FileSystem
 }
 
-// NewDocumentFs returns an instalce of DocumentFs
+// NewDocumentFs returns an instance of Fs
 func NewDocumentFs() FileSystem {
-	return &DocumentFs{FileSystem: fs.MakeFsOnDisk()}
+	return &Fs{FileSystem: fs.MakeFsOnDisk()}
 }
 
 // TempFile creates file in temporary filesystem, at default os.TempDir
-func (dfs DocumentFs) TempFile(tmpDir string, prefix string) (File, error) {
+func (dfs Fs) TempFile(tmpDir string, prefix string) (File, error) {
 	return ioutil.TempFile(tmpDir, prefix)
 }

pkg/remote/redfish/client.go

@@ -203,7 +203,7 @@ func NewClient(ephemeralNodeID string,
 	// We clone the default transport to ensure when we customize the transport
 	// that we are providing it sane timeouts and other defaults that we would
 	// normally get when not overriding the transport
-	defaultTransportCopy := (http.DefaultTransport.(*http.Transport))
+	defaultTransportCopy := http.DefaultTransport.(*http.Transport) //nolint:errcheck
 	transport := defaultTransportCopy.Clone()
 
 	if insecure {

pkg/remote/redfish/utils.go

@@ -26,7 +26,7 @@ import (
 )
 
 const (
-	RedfishURLSchemeSeparator = "+"
+	URLSchemeSeparator = "+"
 )
 
 // GetResourceIDFromURL returns a parsed Redfish resource ID

pkg/remote/remote_direct.go

@@ -25,10 +25,6 @@ import (
 	"opendev.org/airship/airshipctl/pkg/remote/redfish"
 )
 
-const (
-	AirshipHostKind string = "BareMetalHost"
-)
-
 // Adapter bridges the gap between out-of-band clients. It can hold any type of OOB client, e.g. Redfish.
 type Adapter struct {
 	OOBClient    Client
@@ -51,7 +47,7 @@ func (a *Adapter) configureClient(remoteURL string) error {
 		}
 
 		baseURL := fmt.Sprintf("%s://%s", rfURL.Scheme, rfURL.Host)
-		schemeSplit := strings.Split(rfURL.Scheme, redfish.RedfishURLSchemeSeparator)
+		schemeSplit := strings.Split(rfURL.Scheme, redfish.URLSchemeSeparator)
 		if len(schemeSplit) > 1 {
 			baseURL = fmt.Sprintf("%s://%s", schemeSplit[len(schemeSplit)-1], rfURL.Host)
 		}

pkg/remote/remote_direct_test.go

@@ -113,7 +113,7 @@ func TestDoRemoteDirectRedfish(t *testing.T) {
 	a, err := NewAdapter(settings)
 	assert.NoError(t, err)
 
-	ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, false, false, "admin", "password")
+	ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, "admin", "password")
 	assert.NoError(t, err)
 
 	rMock.On("SetVirtualMedia", a.Context, isoURL).Times(1).Return(nil)
@@ -140,7 +140,7 @@ func TestDoRemoteDirectRedfishVirtualMediaError(t *testing.T) {
 	a, err := NewAdapter(settings)
 	assert.NoError(t, err)
 
-	ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, false, false, "admin", "password")
+	ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, "admin", "password")
 	assert.NoError(t, err)
 
 	expectedErr := redfish.ErrRedfishClient{Message: "Unable to set virtual media."}
@@ -169,7 +169,7 @@ func TestDoRemoteDirectRedfishBootSourceError(t *testing.T) {
 	a, err := NewAdapter(settings)
 	assert.NoError(t, err)
 
-	ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, false, false, "admin", "password")
+	ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, "admin", "password")
 	assert.NoError(t, err)
 
 	rMock.On("SetVirtualMedia", a.Context, isoURL).Times(1).Return(nil)
@@ -199,7 +199,7 @@ func TestDoRemoteDirectRedfishRebootError(t *testing.T) {
 	a, err := NewAdapter(settings)
 	assert.NoError(t, err)
 
-	ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, false, false, "admin", "password")
+	ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, "admin", "password")
 	assert.NoError(t, err)
 
 	rMock.On("SetVirtualMedia", a.Context, isoURL).Times(1).Return(nil)

testutil/redfishutils/mock_client.go

@@ -110,8 +110,8 @@ func (m *MockClient) SystemPowerStatus(ctx context.Context, systemID string) (st
 
 // NewClient returns a mocked Redfish client in order to test functions that use the Redfish client without making any
 // Redfish API calls.
-func NewClient(ephemeralNodeID string, isoPath string, redfishURL string, insecure bool,
-	proxy bool, username string, password string) (context.Context, *MockClient, error) {
+func NewClient(ephemeralNodeID string, isoPath string, redfishURL string, username string,
+	password string) (context.Context, *MockClient, error) {
 	var ctx context.Context
 	if username != "" && password != "" {
 		ctx = context.WithValue(

testutil/testdatafs.go

@@ -20,7 +20,7 @@ import (
 func SetupTestFs(t *testing.T, fixtureDir string) document.FileSystem {
 	t.Helper()
 
-	x := &document.DocumentFs{FileSystem: fs.MakeFsInMemory()}
+	x := &document.Fs{FileSystem: fs.MakeFsInMemory()}
 
 	files, err := ioutil.ReadDir(fixtureDir)
 	require.NoErrorf(t, err, "Failed to read fixture directory %s", fixtureDir)