From 129fd7ffa62eef8ebeea265dcfc8970e095807c2 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Fri, 22 Jan 2021 20:23:08 +0000 Subject: [PATCH] Cleanup unused remote pkg code Change-Id: Ie879b6c8947148ae15b1c7cb5e15a46e998ec05c --- pkg/remote/errors.go | 62 -------- pkg/remote/management.go | 243 ------------------------------- pkg/remote/management_test.go | 185 ----------------------- pkg/remote/remote_direct.go | 66 --------- pkg/remote/remote_direct_test.go | 91 ------------ 5 files changed, 647 deletions(-) delete mode 100644 pkg/remote/errors.go delete mode 100644 pkg/remote/management.go delete mode 100644 pkg/remote/management_test.go delete mode 100644 pkg/remote/remote_direct.go delete mode 100644 pkg/remote/remote_direct_test.go diff --git a/pkg/remote/errors.go b/pkg/remote/errors.go deleted file mode 100644 index cd62aad8d..000000000 --- a/pkg/remote/errors.go +++ /dev/null @@ -1,62 +0,0 @@ -/* - 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 remote - -import ( - "fmt" -) - -// TODO: This need to be refactored to match the error format used elsewhere in airshipctl -// Usage of this error should be deprecated as it doesn't provide meaningful feedback to the user. - -// GenericError provides general feedback about an error that occurred in a remote operation -type GenericError struct { - Message string -} - -// NewRemoteDirectErrorf returns formatted remote direct errors -func NewRemoteDirectErrorf(format string, v ...interface{}) error { - return &GenericError{Message: fmt.Sprintf(format, v...)} -} - -func (e GenericError) Error() string { - return e.Message -} - -// ErrUnknownManagementType is an error that indicates the remote type specified in the airshipctl management -// configuration (e.g. redfish, redfish-dell) is not supported. -type ErrUnknownManagementType struct { - Type string -} - -func (e ErrUnknownManagementType) Error() string { - return fmt.Sprintf("unknown management type: %s", e.Type) -} - -// ErrMissingOption is an error that indicates a remote direct config option is missing -type ErrMissingOption struct { - What string -} - -func (e ErrMissingOption) Error() string { - return fmt.Sprintf("missing option: %s", e.What) -} - -// ErrNoHostsFound is an error that indicates that no hosts matched the selection criteria passed to a manager. -type ErrNoHostsFound struct{} - -func (e ErrNoHostsFound) Error() string { - return "no hosts selected" -} diff --git a/pkg/remote/management.go b/pkg/remote/management.go deleted file mode 100644 index 5fc70ad31..000000000 --- a/pkg/remote/management.go +++ /dev/null @@ -1,243 +0,0 @@ -/* - 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 remote manages baremetal hosts. -package remote - -import ( - "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/log" - "opendev.org/airship/airshipctl/pkg/phase" - "opendev.org/airship/airshipctl/pkg/phase/ifc" - remoteifc "opendev.org/airship/airshipctl/pkg/remote/ifc" - "opendev.org/airship/airshipctl/pkg/remote/redfish" - redfishdell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell" -) - -// Manager orchestrates a grouping of baremetal hosts. When a manager is created using its convenience function, the -// manager contains a list of hosts ready for out-of-band management. Iterate over the Hosts property to invoke actions -// on each host. -type Manager struct { - Config config.ManagementConfiguration - Hosts []baremetalHost -} - -// baremetalHost is an airshipctl representation of a baremetal host, defined by a baremetal host document, that embeds -// actions an out-of-band client can perform. Once instantiated, actions can be performed on a baremetal host. -type baremetalHost struct { - remoteifc.Client - BMCAddress string - HostName string - username string - password string -} - -// HostSelector populates baremetal hosts within a manager when supplied with selection criteria. -type HostSelector func(*Manager, config.ManagementConfiguration, document.Bundle) error - -// ByLabel adds all hosts to a manager whose documents match a supplied label selector. -func ByLabel(label string) HostSelector { - return func(a *Manager, mgmtCfg config.ManagementConfiguration, docBundle document.Bundle) error { - selector := document.NewSelector().ByKind(document.BareMetalHostKind).ByLabel(label) - docs, err := docBundle.Select(selector) - if err != nil { - return err - } - - if len(docs) == 0 { - return document.ErrDocNotFound{Selector: selector} - } - - var matchingHosts []baremetalHost - for _, doc := range docs { - host, err := newBaremetalHost(mgmtCfg, doc, docBundle) - if err != nil { - return err - } - - matchingHosts = append(matchingHosts, host) - } - - a.Hosts = reconcileHosts(a.Hosts, matchingHosts...) - - return nil - } -} - -// ByName adds the host to a manager whose document meets the specified name. -func ByName(name string) HostSelector { - return func(a *Manager, mgmtCfg config.ManagementConfiguration, docBundle document.Bundle) error { - selector := document.NewSelector().ByKind(document.BareMetalHostKind).ByName(name) - doc, err := docBundle.SelectOne(selector) - if err != nil { - return err - } - - host, err := newBaremetalHost(mgmtCfg, doc, docBundle) - if err != nil { - return err - } - - a.Hosts = reconcileHosts(a.Hosts, host) - - return nil - } -} - -// All adds the host to a manager whose documents match the BareMetalHostKind. -func All() HostSelector { - return func(a *Manager, mgmtCfg config.ManagementConfiguration, docBundle document.Bundle) error { - selector := document.NewSelector().ByKind(document.BareMetalHostKind) - docs, err := docBundle.Select(selector) - if err != nil { - return err - } - - if len(docs) == 0 { - return document.ErrDocNotFound{Selector: selector} - } - - var matchingHosts []baremetalHost - for _, doc := range docs { - host, err := newBaremetalHost(mgmtCfg, doc, docBundle) - if err != nil { - return err - } - - matchingHosts = append(matchingHosts, host) - } - - a.Hosts = reconcileHosts(a.Hosts, matchingHosts...) - - return nil - } -} - -// NewManager provides a manager that exposes the capability to perform remote direct functionality and other -// out-of-band management on multiple hosts. -func NewManager(cfg *config.Config, phaseName string, hosts ...HostSelector) (*Manager, error) { - managementCfg, err := cfg.CurrentContextManagementConfig() - if err != nil { - return nil, err - } - - if err = managementCfg.Validate(); err != nil { - return nil, err - } - - helper, err := phase.NewHelper(cfg) - if err != nil { - return nil, err - } - - phaseClient := phase.NewClient(helper) - phase, err := phaseClient.PhaseByID(ifc.ID{Name: phaseName}) - if err != nil { - return nil, err - } - - docRoot, err := phase.DocumentRoot() - if err != nil { - return nil, err - } - - docBundle, err := document.NewBundleByPath(docRoot) - if err != nil { - return nil, err - } - - manager := &Manager{ - Config: *managementCfg, - Hosts: []baremetalHost{}, - } - - // Each function in hosts modifies the list of hosts for the new manager based on selection criteria provided - // by CLI arguments and airshipctl settings. - for _, addHost := range hosts { - if err := addHost(manager, *managementCfg, docBundle); err != nil { - return nil, err - } - } - - if len(manager.Hosts) == 0 { - return manager, ErrNoHostsFound{} - } - - return manager, nil -} - -// newBaremetalHost creates a representation of a baremetal host that is configured to perform management actions by -// invoking its client methods (provided by the remote.Client interface). -func newBaremetalHost(mgmtCfg config.ManagementConfiguration, - hostDoc document.Document, - docBundle document.Bundle) (baremetalHost, error) { - address, err := document.GetBMHBMCAddress(hostDoc) - if err != nil { - return baremetalHost{}, err - } - - username, password, err := document.GetBMHBMCCredentials(hostDoc, docBundle) - if err != nil { - return baremetalHost{}, err - } - - var clientFactory remoteifc.ClientFactory - // Select the client that corresponds to the management type specified in the airshipctl config. - switch mgmtCfg.Type { - case redfish.ClientType: - log.Debug("Remote type: Redfish") - clientFactory = redfish.ClientFactory - case redfishdell.ClientType: - clientFactory = redfishdell.ClientFactory - default: - return baremetalHost{}, ErrUnknownManagementType{Type: mgmtCfg.Type} - } - - client, err := clientFactory( - address, - mgmtCfg.Insecure, - mgmtCfg.UseProxy, - username, - password, mgmtCfg.SystemActionRetries, - mgmtCfg.SystemRebootDelay) - - if err != nil { - return baremetalHost{}, err - } - - return baremetalHost{client, address, hostDoc.GetName(), username, password}, nil -} - -// reconcileHosts produces the intersection of two baremetal host arrays. -func reconcileHosts(existingHosts []baremetalHost, newHosts ...baremetalHost) []baremetalHost { - if len(existingHosts) == 0 { - return newHosts - } - - // Create a map of host document names for efficient filtering - hostMap := make(map[string]bool) - for _, host := range existingHosts { - hostMap[host.HostName] = true - } - - var reconciledHosts []baremetalHost - for _, host := range newHosts { - if _, exists := hostMap[host.HostName]; exists { - reconciledHosts = append(reconciledHosts, host) - } - } - - return reconciledHosts -} diff --git a/pkg/remote/management_test.go b/pkg/remote/management_test.go deleted file mode 100644 index 64ab99a66..000000000 --- a/pkg/remote/management_test.go +++ /dev/null @@ -1,185 +0,0 @@ -/* - 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 remote - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/remote/redfish" - redfishdell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell" - "opendev.org/airship/airshipctl/testutil" -) - -type Configuration func(*config.Config) - -// initSettings initializes the global airshipctl settings with test data by accepting functions as arguments that -// manipulate configuration sections. -func initSettings(t *testing.T, configs ...Configuration) *config.Config { - t.Helper() - - config := testutil.DummyConfig() - for _, cfg := range configs { - cfg(config) - } - return config -} - -// withManagementConfig initializes the management config when used as an argument to initSettings. -func withManagementConfig(cfg *config.ManagementConfiguration) Configuration { - return func(settings *config.Config) { - settings.ManagementConfiguration["dummy_management_config"] = cfg - } -} - -// withTestDataPath sets the test data path when used as an argument to initSettings. -func withTestDataPath(path string) Configuration { - return func(settings *config.Config) { - manifest, err := settings.CurrentContextManifest() - if err != nil { - panic(fmt.Sprintf("Unable to initialize management tests. Current Context error %q", err)) - } - manifest.TargetPath = "testdata" - manifest.MetadataPath = "metadata.yaml" - dummyRepo := testutil.DummyRepository() - dummyRepo.URLString = fmt.Sprintf("http://dummy.url.com/%s.git", path) - manifest.Repositories = map[string]*config.Repository{manifest.PhaseRepositoryName: dummyRepo} - } -} - -func TestNewManagerEphemeralHost(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - manager, err := NewManager(settings, config.BootstrapPhase, ByLabel(document.EphemeralHostSelector)) - require.NoError(t, err) - require.Equal(t, 1, len(manager.Hosts)) - - assert.Equal(t, "ephemeral", manager.Hosts[0].NodeID()) -} - -func TestNewManagerByName(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - manager, err := NewManager(settings, config.BootstrapPhase, ByName("master-1")) - require.NoError(t, err) - require.Equal(t, 1, len(manager.Hosts)) - - assert.Equal(t, "node-master-1", manager.Hosts[0].NodeID()) -} - -func TestNewManagerMultipleNodes(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - manager, err := NewManager(settings, config.BootstrapPhase, ByLabel("airshipit.org/test-node=true")) - require.NoError(t, err) - require.Equal(t, 2, len(manager.Hosts)) - - assert.Equal(t, "node-master-1", manager.Hosts[0].NodeID()) - assert.Equal(t, "node-master-2", manager.Hosts[1].NodeID()) -} - -func TestNewManagerMultipleSelectors(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - manager, err := NewManager(settings, config.BootstrapPhase, ByName("master-1"), - ByLabel("airshipit.org/test-node=true")) - require.NoError(t, err) - require.Equal(t, 1, len(manager.Hosts)) - - assert.Equal(t, "node-master-1", manager.Hosts[0].NodeID()) -} - -func TestNewManagerMultipleSelectorsNoMatch(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - manager, err := NewManager(settings, config.BootstrapPhase, ByName("master-2"), - ByLabel(document.EphemeralHostSelector)) - - // Must return ErrNoHostsFound here, without check for specific error, test can panic - require.Equal(t, ErrNoHostsFound{}, err) - require.Equal(t, 0, len(manager.Hosts)) - assert.Error(t, err) -} - -func TestNewManagerByNameNoHostFound(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - _, err := NewManager(settings, config.BootstrapPhase, ByName("bad-name")) - assert.Error(t, err) -} - -func TestNewManagerNoSelectors(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - _, err := NewManager(settings, config.BootstrapPhase) - assert.Error(t, err) -} - -func TestNewManagerByLabelNoHostsFound(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - _, err := NewManager(settings, config.BootstrapPhase, ByLabel("bad-label=true")) - assert.Error(t, err) -} - -func TestNewManagerRedfish(t *testing.T) { - cfg := &config.ManagementConfiguration{Type: redfish.ClientType} - settings := initSettings(t, withManagementConfig(cfg), withTestDataPath("base")) - - _, err := NewManager(settings, config.BootstrapPhase, ByLabel(document.EphemeralHostSelector)) - assert.NoError(t, err) -} - -func TestNewManagerRedfishDell(t *testing.T) { - cfg := &config.ManagementConfiguration{Type: redfishdell.ClientType} - settings := initSettings(t, withManagementConfig(cfg), withTestDataPath("base")) - - _, err := NewManager(settings, config.BootstrapPhase, ByLabel(document.EphemeralHostSelector)) - assert.NoError(t, err) -} - -func TestNewManagerUnknownRemoteType(t *testing.T) { - badCfg := &config.ManagementConfiguration{Type: "bad-remote-type"} - settings := initSettings(t, withManagementConfig(badCfg), withTestDataPath("base")) - - _, err := NewManager(settings, config.BootstrapPhase, ByLabel(document.EphemeralHostSelector)) - assert.Error(t, err) -} - -func TestNewManagerMissingBMCAddress(t *testing.T) { - settings := initSettings(t, withTestDataPath("emptyurl")) - - _, err := NewManager(settings, config.BootstrapPhase, ByLabel(document.EphemeralHostSelector)) - assert.Error(t, err) -} - -func TestNewManagerMissingCredentials(t *testing.T) { - settings := initSettings(t, withTestDataPath("emptyurl")) - - _, err := NewManager(settings, config.BootstrapPhase, ByName("no-creds")) - assert.Error(t, err) -} - -func TestNewManagerBadPhase(t *testing.T) { - settings := initSettings(t, withTestDataPath("base")) - - _, err := NewManager(settings, "bad-phase", ByLabel(document.EphemeralHostSelector)) - assert.Error(t, err) -} diff --git a/pkg/remote/remote_direct.go b/pkg/remote/remote_direct.go deleted file mode 100644 index ecce0c8d8..000000000 --- a/pkg/remote/remote_direct.go +++ /dev/null @@ -1,66 +0,0 @@ -/* - 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 remote - -import ( - "context" - - api "opendev.org/airship/airshipctl/pkg/api/v1alpha1" - "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/document" - "opendev.org/airship/airshipctl/pkg/phase" - "opendev.org/airship/airshipctl/pkg/phase/ifc" -) - -// DoRemoteDirect bootstraps the ephemeral node. -func (b baremetalHost) DoRemoteDirect(cfg *config.Config) error { - helper, err := phase.NewHelper(cfg) - if err != nil { - return err - } - - phaseClient := phase.NewClient(helper) - phase, err := phaseClient.PhaseByID(ifc.ID{Name: config.BootstrapPhase}) - if err != nil { - return err - } - - docRoot, err := phase.DocumentRoot() - if err != nil { - return err - } - - docBundle, err := document.NewBundleByPath(docRoot) - if err != nil { - return err - } - - remoteDirectConfiguration := &api.RemoteDirectConfiguration{} - selector, err := document.NewSelector().ByObject(remoteDirectConfiguration, api.Scheme) - if err != nil { - return err - } - doc, err := docBundle.SelectOne(selector) - if err != nil { - return err - } - - err = doc.ToAPIObject(remoteDirectConfiguration, api.Scheme) - if err != nil { - return err - } - - return b.RemoteDirect(context.Background(), remoteDirectConfiguration.IsoURL) -} diff --git a/pkg/remote/remote_direct_test.go b/pkg/remote/remote_direct_test.go deleted file mode 100644 index 768a67753..000000000 --- a/pkg/remote/remote_direct_test.go +++ /dev/null @@ -1,91 +0,0 @@ -/* - 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 remote - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "opendev.org/airship/airshipctl/testutil/redfishutils" -) - -const ( - redfishURL = "redfish+https://localhost:2344/Systems/System.Embedded.1" - username = "admin" - password = "password" -) - -func TestDoRemoteDirectMissingConfigOpts(t *testing.T) { - rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) - assert.NoError(t, err) - - ephemeralHost := baremetalHost{ - rMock, - redfishURL, - "doc-name", - username, - password, - } - - settings := initSettings(t, withTestDataPath("noremote")) - // there must be document.ErrDocNotFound - err = ephemeralHost.DoRemoteDirect(settings) - expectedErrorMessage := `document filtered by selector [Group="airshipit.org", Version="v1alpha1", ` + - `Kind="RemoteDirectConfiguration"] found no documents` - assert.Equal(t, expectedErrorMessage, fmt.Sprintf("%s", err)) - assert.Error(t, err) -} - -func TestDoRemoteDirectRedfish(t *testing.T) { - rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) - require.NoError(t, err) - - rMock.On("RemoteDirect").Times(1).Return(nil) - - ephemeralHost := baremetalHost{ - rMock, - redfishURL, - "doc-name", - username, - password, - } - - settings := initSettings(t, withTestDataPath("base")) - err = ephemeralHost.DoRemoteDirect(settings) - assert.NoError(t, err) -} - -func TestDoRemoteDirectError(t *testing.T) { - rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) - require.NoError(t, err) - - expectedErr := fmt.Errorf("remote direct error") - rMock.On("RemoteDirect").Times(1).Return(expectedErr) - - ephemeralHost := baremetalHost{ - rMock, - redfishURL, - "doc-name", - username, - password, - } - - settings := initSettings(t, withTestDataPath("base")) - actualErr := ephemeralHost.DoRemoteDirect(settings) - assert.Equal(t, expectedErr, actualErr) -}