From 995538829e756a9e22e843380b0083a3f70e2e7c Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Wed, 15 Apr 2020 21:01:36 +0000 Subject: [PATCH] Add management support to non-ephemeral hosts Management support is only available to the ephemeral host. This change extends support of management features to hosts not labeled as the ephemeral host. Change-Id: I0ac917f2633f659e56e67cafc52d2db9d967754f Signed-off-by: Drew Walters --- cmd/bootstrap/bootstrap_remotedirect.go | 13 +- cmd/remote/remote.go | 29 +- cmd/remote/remote_power_off.go | 45 +-- cmd/remote/remote_power_status.go | 48 ++-- cmd/remote/remote_reboot.go | 45 +-- pkg/config/types.go | 11 - pkg/config/utils.go | 3 +- pkg/remote/errors.go | 22 ++ pkg/remote/management.go | 191 +++++++++++++ pkg/remote/management_test.go | 137 +++++++++ pkg/remote/redfish/client.go | 77 +++-- pkg/remote/redfish/client_test.go | 142 ++++++---- pkg/remote/redfish/utils.go | 26 +- pkg/remote/redfish/vendors/dell/client.go | 13 +- .../redfish/vendors/dell/client_test.go | 38 +-- pkg/remote/remote_direct.go | 185 ++---------- pkg/remote/remote_direct_test.go | 265 +++++++++--------- .../ephemeral/bootstrap/baremetal.yaml | 51 +++- pkg/remote/types.go | 39 --- .../templates/airshipconfig.j2 | 7 + testutil/redfishutils/mock_client.go | 48 ++-- 21 files changed, 872 insertions(+), 563 deletions(-) create mode 100644 pkg/remote/management.go create mode 100644 pkg/remote/management_test.go delete mode 100644 pkg/remote/types.go diff --git a/cmd/bootstrap/bootstrap_remotedirect.go b/cmd/bootstrap/bootstrap_remotedirect.go index acd5c6eeb..cd83149cd 100644 --- a/cmd/bootstrap/bootstrap_remotedirect.go +++ b/cmd/bootstrap/bootstrap_remotedirect.go @@ -17,6 +17,8 @@ package bootstrap import ( "github.com/spf13/cobra" + "opendev.org/airship/airshipctl/pkg/config" + "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) @@ -27,12 +29,19 @@ func NewRemoteDirectCommand(rootSettings *environment.AirshipCTLSettings) *cobra Use: "remotedirect", Short: "Bootstrap ephemeral node", RunE: func(cmd *cobra.Command, args []string) error { - a, err := remote.NewAdapter(rootSettings) + manager, err := remote.NewManager(rootSettings, + config.BootstrapPhase, + remote.ByLabel(document.EphemeralHostSelector)) if err != nil { return err } - return a.DoRemoteDirect() + if len(manager.Hosts) != 1 { + return remote.NewRemoteDirectErrorf("more than one node defined as the ephemeral node") + } + + ephemeralHost := manager.Hosts[0] + return ephemeralHost.DoRemoteDirect(rootSettings) }, } diff --git a/cmd/remote/remote.go b/cmd/remote/remote.go index f172f98ef..396378d42 100644 --- a/cmd/remote/remote.go +++ b/cmd/remote/remote.go @@ -1,14 +1,16 @@ -// 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. +/* +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 @@ -18,6 +20,11 @@ import ( "opendev.org/airship/airshipctl/pkg/environment" ) +const ( + flagPhase = "phase" + flagPhaseDescription = "airshipctl phase that contains the desired baremetal host document(s)" +) + // NewRemoteCommand creates a new command that provides functionality to control remote entities. func NewRemoteCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { remoteRootCmd := &cobra.Command{ diff --git a/cmd/remote/remote_power_off.go b/cmd/remote/remote_power_off.go index 2521bcec6..c11dd3650 100644 --- a/cmd/remote/remote_power_off.go +++ b/cmd/remote/remote_power_off.go @@ -1,14 +1,16 @@ -// 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. +/* + 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 @@ -17,31 +19,38 @@ import ( "github.com/spf13/cobra" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewPowerOffCommand provides a command to shutdown a remote host. func NewPowerOffCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { - powerOffCmd := &cobra.Command{ + var phase string + cmd := &cobra.Command{ Use: "poweroff SYSTEM_ID", Short: "Shutdown a host", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - a, err := remote.NewAdapter(rootSettings) + m, err := remote.NewManager(rootSettings, phase, remote.ByName(args[0])) if err != nil { return err } - if err := a.OOBClient.SystemPowerOff(a.Context, args[0]); err != nil { - return err - } + for _, host := range m.Hosts { + if err := host.SystemPowerOff(host.Context); err != nil { + return err + } - fmt.Fprintf(cmd.OutOrStdout(), "Remote host %s powered off\n", args[0]) + fmt.Fprintf(cmd.OutOrStdout(), "Remote host %s powered off\n", args[0]) + } return nil }, } - return powerOffCmd + flags := cmd.Flags() + flags.StringVar(&phase, flagPhase, config.BootstrapPhase, flagPhaseDescription) + + return cmd } diff --git a/cmd/remote/remote_power_status.go b/cmd/remote/remote_power_status.go index 6eaac55e0..f6ec2c754 100644 --- a/cmd/remote/remote_power_status.go +++ b/cmd/remote/remote_power_status.go @@ -1,14 +1,16 @@ -// 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. +/* + 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 @@ -17,32 +19,40 @@ import ( "github.com/spf13/cobra" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewPowerStatusCommand provides a command to retrieve the power status of a remote host. func NewPowerStatusCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { - powerStatusCmd := &cobra.Command{ + var phase string + cmd := &cobra.Command{ Use: "powerstatus SYSTEM_ID", Short: "Retrieve the power status of a host", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - a, err := remote.NewAdapter(rootSettings) + m, err := remote.NewManager(rootSettings, phase, remote.ByName(args[0])) if err != nil { return err } - powerStatus, err := a.OOBClient.SystemPowerStatus(a.Context, args[0]) - if err != nil { - return err - } + for _, host := range m.Hosts { + powerStatus, err := host.SystemPowerStatus(host.Context) + if err != nil { + return err + } - fmt.Fprintf(cmd.OutOrStdout(), "Remote host %s has power status: %s\n", args[0], powerStatus) + fmt.Fprintf(cmd.OutOrStdout(), "Remote host %s has power status: %s\n", args[0], + powerStatus) + } return nil }, } - return powerStatusCmd + flags := cmd.Flags() + flags.StringVar(&phase, flagPhase, config.BootstrapPhase, flagPhaseDescription) + + return cmd } diff --git a/cmd/remote/remote_reboot.go b/cmd/remote/remote_reboot.go index 2acd0ec4b..d2cd0506a 100644 --- a/cmd/remote/remote_reboot.go +++ b/cmd/remote/remote_reboot.go @@ -1,14 +1,16 @@ -// 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. +/* +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 @@ -17,31 +19,38 @@ import ( "github.com/spf13/cobra" + "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/remote" ) // NewRebootCommand provides a command with the capability to reboot hosts. func NewRebootCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { - rebootCmd := &cobra.Command{ + var phase string + cmd := &cobra.Command{ Use: "reboot SYSTEM_ID", Short: "Reboot a host", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - a, err := remote.NewAdapter(rootSettings) + m, err := remote.NewManager(rootSettings, phase, remote.ByName(args[0])) if err != nil { return err } - if err = a.OOBClient.RebootSystem(a.Context, args[0]); err != nil { - return err - } + for _, host := range m.Hosts { + if err := host.RebootSystem(host.Context); err != nil { + return err + } - fmt.Fprintf(cmd.OutOrStdout(), "Rebooted remote host %s\n", args[0]) + fmt.Fprintf(cmd.OutOrStdout(), "Rebooted remote host %s\n", args[0]) + } return nil }, } - return rebootCmd + flags := cmd.Flags() + flags.StringVar(&phase, flagPhase, config.BootstrapPhase, flagPhaseDescription) + + return cmd } diff --git a/pkg/config/types.go b/pkg/config/types.go index aff3d1470..77365b60e 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -193,17 +193,6 @@ type Builder struct { // RemoteDirect configuration options type RemoteDirect struct { - // RemoteType specifies type of epehemeral node managfement (e.g redfish, - // smash e.t.c.) - RemoteType string `json:"remoteType,omitempty"` // IsoURL specifies url to download ISO image for epehemeral node IsoURL string `json:"isoUrl,omitempty"` - // Ignore SSL certificate check. This options is useful for remote APIs - // with non-trusted or self-signed SSL certificates - Insecure bool `json:"insecure,omitempty"` - // Allow remotedirect requests to be proxied. This defaults to false - // because in general, most users will want to communicate directly - // with redfish and other bmc urls directly even if the environment - // has a proxy set - UseProxy bool `json:"useproxy,omitempty"` } diff --git a/pkg/config/utils.go b/pkg/config/utils.go index ed69b9a28..55554231e 100644 --- a/pkg/config/utils.go +++ b/pkg/config/utils.go @@ -42,8 +42,7 @@ func NewConfig() *Config { OutputMetadataFileName: "output-metadata.yaml", }, RemoteDirect: &RemoteDirect{ - RemoteType: AirshipDefaultRemoteType, - IsoURL: AirshipDefaultIsoURL, + IsoURL: AirshipDefaultIsoURL, }, }, }, diff --git a/pkg/remote/errors.go b/pkg/remote/errors.go index 5b1bc69c7..918742acf 100644 --- a/pkg/remote/errors.go +++ b/pkg/remote/errors.go @@ -29,3 +29,25 @@ func NewRemoteDirectErrorf(format string, v ...interface{}) error { e.Message = fmt.Sprintf(format, v...) return e } + +// 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 { + aerror.AirshipError + Type string +} + +func (e ErrUnknownManagementType) Error() string { + return fmt.Sprintf("unknown management type: %s", e.Type) +} + +// ErrMissingBootstrapInfoOption is an error that indicates a bootstrap option is missing in the airshipctl +// bootstrapInfo configuration. +type ErrMissingBootstrapInfoOption struct { + aerror.AirshipError + What string +} + +func (e ErrMissingBootstrapInfoOption) Error() string { + return fmt.Sprintf("missing bootstrapInfo option: %s", e.What) +} diff --git a/pkg/remote/management.go b/pkg/remote/management.go new file mode 100644 index 000000000..8c67cc9bc --- /dev/null +++ b/pkg/remote/management.go @@ -0,0 +1,191 @@ +/* + 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 ( + "context" + + "opendev.org/airship/airshipctl/pkg/config" + "opendev.org/airship/airshipctl/pkg/document" + "opendev.org/airship/airshipctl/pkg/environment" + "opendev.org/airship/airshipctl/pkg/remote/redfish" +) + +// Client is a set of functions that clients created for out-of-band power management and control should implement. The +// functions within client are used by power management commands and remote direct functionality. +type Client interface { + RebootSystem(context.Context) error + + SystemPowerOff(context.Context) error + + // TODO(drewwalters96): Should this be a string forever? We may want to define our own custom type, as the + // string format will be client dependent when we add new clients. + SystemPowerStatus(context.Context) (string, error) + + NodeID() string + + SetBootSourceByType(context.Context) error + + // TODO(drewwalters96): This function is tightly coupled to Redfish. It should be combined with the + // SetBootSource operation and removed from the client interface. + SetVirtualMedia(context.Context, string) error +} + +// 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 { + Client + Context context.Context + BMCAddress 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} + } + + for _, doc := range docs { + host, err := newBaremetalHost(mgmtCfg, doc, docBundle) + if err != nil { + return err + } + + a.Hosts = append(a.Hosts, host) + } + + 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 = append(a.Hosts, host) + + 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(settings *environment.AirshipCTLSettings, phase string, hosts ...HostSelector) (*Manager, error) { + configContext, err := settings.Config.GetCurrentContext() + if err != nil { + return nil, err + } + + managementCfg, err := settings.Config.CurrentContextManagementConfig() + if err != nil { + return nil, err + } + + clusterType := configContext.ClusterType() + entrypoint, err := settings.Config.CurrentContextEntryPoint(clusterType, phase) + if err != nil { + return nil, err + } + + docBundle, err := document.NewBundleByPath(entrypoint) + 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 + } + } + + 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) { + var host baremetalHost + + address, err := document.GetBMHBMCAddress(hostDoc) + if err != nil { + return host, err + } + + username, password, err := document.GetBMHBMCCredentials(hostDoc, docBundle) + if err != nil { + return host, err + } + + // Select the client that corresponds to the management type specified in the airshipctl config. + switch mgmtCfg.Type { + case redfish.ClientType: + ctx, client, err := redfish.NewClient( + address, + mgmtCfg.Insecure, + mgmtCfg.UseProxy, + username, + password) + + if err != nil { + return host, err + } + + host = baremetalHost{client, ctx, address, username, password} + default: + return host, ErrUnknownManagementType{Type: mgmtCfg.Type} + } + + return host, nil +} diff --git a/pkg/remote/management_test.go b/pkg/remote/management_test.go new file mode 100644 index 000000000..5f20d3c69 --- /dev/null +++ b/pkg/remote/management_test.go @@ -0,0 +1,137 @@ +/* + 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/environment" + "opendev.org/airship/airshipctl/testutil" +) + +type Configuration func(*environment.AirshipCTLSettings) + +// 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) *environment.AirshipCTLSettings { + t.Helper() + + settings := &environment.AirshipCTLSettings{Config: testutil.DummyConfig()} + + for _, cfg := range configs { + cfg(settings) + } + + return settings +} + +// withManagementConfig initializes the management config when used as an argument to initSettings. +func withManagementConfig(cfg *config.ManagementConfiguration) Configuration { + return func(settings *environment.AirshipCTLSettings) { + settings.Config.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 *environment.AirshipCTLSettings) { + manifest, err := settings.Config.CurrentContextManifest() + if err != nil { + panic(fmt.Sprintf("Unable to initialize management tests. Current Context error %q", err)) + } + + manifest.TargetPath = fmt.Sprintf("testdata/%s", path) + } +} + +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 TestNewManagerByNameNoHostFound(t *testing.T) { + settings := initSettings(t, withTestDataPath("base")) + + _, err := NewManager(settings, config.BootstrapPhase, ByName("bad-name")) + 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 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/redfish/client.go b/pkg/remote/redfish/client.go index b70ddb2cb..8efb69c76 100644 --- a/pkg/remote/redfish/client.go +++ b/pkg/remote/redfish/client.go @@ -17,7 +17,6 @@ import ( "crypto/tls" "fmt" "net/http" - "net/url" "strings" "time" @@ -36,20 +35,18 @@ const ( // Client holds details about a Redfish out-of-band system required for out-of-band management. type Client struct { - ephemeralNodeID string - isoPath string - redfishURL url.URL - RedfishAPI redfishAPI.RedfishAPI - RedfishCFG *redfishClient.Configuration + nodeID string + RedfishAPI redfishAPI.RedfishAPI + RedfishCFG *redfishClient.Configuration } -// EphemeralNodeID retrieves the ephemeral node ID. -func (c *Client) EphemeralNodeID() string { - return c.ephemeralNodeID +// NodeID retrieves the ephemeral node ID. +func (c *Client) NodeID() string { + return c.nodeID } // RebootSystem power cycles a host by sending a shutdown signal followed by a power on signal. -func (c *Client) RebootSystem(ctx context.Context, systemID string) error { +func (c *Client) RebootSystem(ctx context.Context) error { waitForPowerState := func(desiredState redfishClient.PowerState) error { // Check if number of retries is defined in context totalRetries, ok := ctx.Value("numRetries").(int) @@ -57,8 +54,8 @@ func (c *Client) RebootSystem(ctx context.Context, systemID string) error { totalRetries = systemActionRetries } - for retry := 0; retry < totalRetries; retry++ { - system, httpResp, err := c.RedfishAPI.GetSystem(ctx, systemID) + for retry := 0; retry <= totalRetries; retry++ { + system, httpResp, err := c.RedfishAPI.GetSystem(ctx, c.nodeID) if err = ScreenRedfishError(httpResp, err); err != nil { return err } @@ -68,7 +65,7 @@ func (c *Client) RebootSystem(ctx context.Context, systemID string) error { time.Sleep(systemRebootDelay) } return ErrOperationRetriesExceeded{ - What: fmt.Sprintf("reboot system %s", c.EphemeralNodeID()), + What: fmt.Sprintf("reboot system %s", c.nodeID), Retries: totalRetries, } } @@ -77,7 +74,7 @@ func (c *Client) RebootSystem(ctx context.Context, systemID string) error { // Send PowerOff request resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF - _, httpResp, err := c.RedfishAPI.ResetSystem(ctx, systemID, resetReq) + _, httpResp, err := c.RedfishAPI.ResetSystem(ctx, c.nodeID, resetReq) if err = ScreenRedfishError(httpResp, err); err != nil { return err } @@ -89,7 +86,7 @@ func (c *Client) RebootSystem(ctx context.Context, systemID string) error { // Send PowerOn request resetReq.ResetType = redfishClient.RESETTYPE_ON - _, httpResp, err = c.RedfishAPI.ResetSystem(ctx, systemID, resetReq) + _, httpResp, err = c.RedfishAPI.ResetSystem(ctx, c.nodeID, resetReq) if err = ScreenRedfishError(httpResp, err); err != nil { return err } @@ -98,18 +95,18 @@ func (c *Client) RebootSystem(ctx context.Context, systemID string) error { return waitForPowerState(redfishClient.POWERSTATE_ON) } -// SetEphemeralBootSourceByType sets the boot source of the ephemeral node to one that's compatible with the boot +// SetBootSourceByType sets the boot source of the ephemeral node to one that's compatible with the boot // source type. -func (c *Client) SetEphemeralBootSourceByType(ctx context.Context) error { - _, vMediaType, err := GetVirtualMediaID(ctx, c.RedfishAPI, c.ephemeralNodeID) +func (c *Client) SetBootSourceByType(ctx context.Context) error { + _, vMediaType, err := GetVirtualMediaID(ctx, c.RedfishAPI, c.nodeID) if err != nil { return err } // Retrieve system information, containing available boot sources - system, _, err := c.RedfishAPI.GetSystem(ctx, c.ephemeralNodeID) + system, _, err := c.RedfishAPI.GetSystem(ctx, c.nodeID) if err != nil { - return ErrRedfishClient{Message: fmt.Sprintf("Get System[%s] failed with err: %v", c.ephemeralNodeID, err)} + return ErrRedfishClient{Message: fmt.Sprintf("Get System[%s] failed with err: %v", c.nodeID, err)} } allowableValues := system.Boot.BootSourceOverrideTargetRedfishAllowableValues @@ -118,12 +115,12 @@ func (c *Client) SetEphemeralBootSourceByType(ctx context.Context) error { /* set boot source */ systemReq := redfishClient.ComputerSystem{} systemReq.Boot.BootSourceOverrideTarget = bootSource - _, httpResp, err := c.RedfishAPI.SetSystem(ctx, c.ephemeralNodeID, systemReq) + _, httpResp, err := c.RedfishAPI.SetSystem(ctx, c.nodeID, systemReq) return ScreenRedfishError(httpResp, err) } } - return ErrRedfishClient{Message: fmt.Sprintf("failed to set system[%s] boot source", c.ephemeralNodeID)} + return ErrRedfishClient{Message: fmt.Sprintf("failed to set system[%s] boot source", c.nodeID)} } // SetVirtualMedia injects a virtual media device to an established virtual media ID. This assumes that isoPath is @@ -151,16 +148,16 @@ func (c *Client) SetVirtualMedia(ctx context.Context, isoPath string) error { return ErrOperationRetriesExceeded{What: fmt.Sprintf("eject media %s", vMediaID), Retries: totalRetries} } - log.Debugf("Ephemeral Node System ID: '%s'", c.ephemeralNodeID) + log.Debugf("Setting virtual media for node: '%s'", c.nodeID) - managerID, err := GetManagerID(ctx, c.RedfishAPI, c.ephemeralNodeID) + managerID, err := getManagerID(ctx, c.RedfishAPI, c.nodeID) if err != nil { return err } log.Debugf("Ephemeral node managerID: '%s'", managerID) - vMediaID, _, err := GetVirtualMediaID(ctx, c.RedfishAPI, c.ephemeralNodeID) + vMediaID, _, err := GetVirtualMediaID(ctx, c.RedfishAPI, c.nodeID) if err != nil { return err } @@ -194,18 +191,18 @@ func (c *Client) SetVirtualMedia(ctx context.Context, isoPath string) error { } // SystemPowerOff shuts down a host. -func (c *Client) SystemPowerOff(ctx context.Context, systemID string) error { +func (c *Client) SystemPowerOff(ctx context.Context) error { resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF - _, httpResp, err := c.RedfishAPI.ResetSystem(ctx, systemID, resetReq) + _, httpResp, err := c.RedfishAPI.ResetSystem(ctx, c.nodeID, resetReq) return ScreenRedfishError(httpResp, err) } // SystemPowerStatus retrieves the power status of a host as a human-readable string. -func (c *Client) SystemPowerStatus(ctx context.Context, systemID string) (string, error) { - computerSystem, httpResp, err := c.RedfishAPI.GetSystem(ctx, systemID) +func (c *Client) SystemPowerStatus(ctx context.Context) (string, error) { + computerSystem, httpResp, err := c.RedfishAPI.GetSystem(ctx, c.nodeID) if err = ScreenRedfishError(httpResp, err); err != nil { return "", err } @@ -214,9 +211,7 @@ func (c *Client) SystemPowerStatus(ctx context.Context, systemID string) (string } // NewClient returns a client with the capability to make Redfish requests. -func NewClient(ephemeralNodeID string, - isoPath string, - redfishURL string, +func NewClient(redfishURL string, insecure bool, useProxy bool, username string, @@ -236,13 +231,13 @@ func NewClient(ephemeralNodeID string, return ctx, nil, ErrRedfishMissingConfig{What: "Redfish URL"} } - parsedURL, err := url.Parse(redfishURL) + basePath, err := getBasePath(redfishURL) if err != nil { return ctx, nil, err } cfg := &redfishClient.Configuration{ - BasePath: redfishURL, + BasePath: basePath, DefaultHeader: make(map[string]string), UserAgent: headerUserAgent, } @@ -268,12 +263,16 @@ func NewClient(ephemeralNodeID string, Transport: transport, } + // Retrieve system ID from end of Redfish URL + systemID := GetResourceIDFromURL(redfishURL) + if len(systemID) == 0 { + return ctx, nil, ErrRedfishMissingConfig{What: "management URL system ID"} + } + c := &Client{ - ephemeralNodeID: ephemeralNodeID, - isoPath: isoPath, - redfishURL: *parsedURL, - RedfishAPI: redfishClient.NewAPIClient(cfg).DefaultApi, - RedfishCFG: cfg, + nodeID: systemID, + RedfishAPI: redfishClient.NewAPIClient(cfg).DefaultApi, + RedfishCFG: cfg, } return ctx, c, nil diff --git a/pkg/remote/redfish/client_test.go b/pkg/remote/redfish/client_test.go index 89c9bfaf3..7205d5d6d 100644 --- a/pkg/remote/redfish/client_test.go +++ b/pkg/remote/redfish/client_test.go @@ -29,18 +29,33 @@ import ( ) const ( - ephemeralNodeID = "ephemeral-node-id" - isoPath = "https://localhost:8080/debian.iso" - redfishURL = "https://localhost:1234" + nodeID = "System.Embedded.1" + isoPath = "https://localhost:8080/debian.iso" + redfishURL = "redfish+https://localhost:2224/Systems/System.Embedded.1" ) func TestNewClient(t *testing.T) { - _, _, err := NewClient(ephemeralNodeID, isoPath, redfishURL, false, false, "", "") + _, _, err := NewClient(redfishURL, false, false, "", "") + assert.NoError(t, err) +} + +func TestNewClientMissingSystemID(t *testing.T) { + badURL := "redfish+https://localhost:2224" + + _, _, err := NewClient(badURL, false, false, "", "") + _, ok := err.(ErrRedfishMissingConfig) + assert.True(t, ok) +} + +func TestNewClientNoRedfishMarking(t *testing.T) { + url := "https://localhost:2224/Systems/System.Embedded.1" + + _, _, err := NewClient(url, false, false, "", "") assert.NoError(t, err) } func TestNewClientAuth(t *testing.T) { - ctx, _, err := NewClient(ephemeralNodeID, isoPath, redfishURL, false, false, "username", "password") + ctx, _, err := NewClient(redfishURL, false, false, "username", "password") assert.NoError(t, err) cAuth := ctx.Value(redfishClient.ContextBasicAuth) @@ -50,7 +65,7 @@ func TestNewClientAuth(t *testing.T) { func TestNewClientEmptyRedfishURL(t *testing.T) { // Redfish URL cannot be empty when creating a client. - _, _, err := NewClient(ephemeralNodeID, isoPath, "", false, false, "", "") + _, _, err := NewClient("", false, false, "", "") assert.Error(t, err) } @@ -58,29 +73,31 @@ func TestRebootSystem(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(ephemeralNodeID, isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + // Mock redfish shutdown and status requests resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF httpResp := &http.Response{StatusCode: 200} - m.On("ResetSystem", ctx, ephemeralNodeID, resetReq).Times(1).Return(redfishClient.RedfishError{}, httpResp, nil) + m.On("ResetSystem", ctx, client.nodeID, resetReq).Times(1).Return(redfishClient.RedfishError{}, httpResp, nil) - m.On("GetSystem", ctx, ephemeralNodeID).Times(1).Return( + m.On("GetSystem", ctx, client.nodeID).Times(1).Return( redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_OFF}, httpResp, nil) // Mock redfish startup and status requests resetReq.ResetType = redfishClient.RESETTYPE_ON - m.On("ResetSystem", ctx, ephemeralNodeID, resetReq).Times(1).Return(redfishClient.RedfishError{}, httpResp, nil) + m.On("ResetSystem", ctx, client.nodeID, resetReq).Times(1).Return(redfishClient.RedfishError{}, httpResp, nil) - m.On("GetSystem", ctx, ephemeralNodeID).Times(1). + m.On("GetSystem", ctx, client.nodeID).Times(1). Return(redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_ON}, httpResp, nil) // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.RebootSystem(ctx, ephemeralNodeID) + err = client.RebootSystem(ctx) assert.NoError(t, err) } @@ -88,20 +105,22 @@ func TestRebootSystemShutdownError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(ephemeralNodeID, isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF // Mock redfish shutdown request for failure - m.On("ResetSystem", ctx, ephemeralNodeID, resetReq).Times(1).Return(redfishClient.RedfishError{}, + m.On("ResetSystem", ctx, client.nodeID, resetReq).Times(1).Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 401}, redfishClient.GenericOpenAPIError{}) // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.RebootSystem(ctx, ephemeralNodeID) + err = client.RebootSystem(ctx) _, ok := err.(ErrRedfishClient) assert.True(t, ok) } @@ -110,18 +129,19 @@ func TestRebootSystemStartupError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(ephemeralNodeID, isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF // Mock redfish shutdown request - systemID := ephemeralNodeID - m.On("ResetSystem", ctx, systemID, resetReq).Times(1).Return(redfishClient.RedfishError{}, + m.On("ResetSystem", ctx, client.nodeID, resetReq).Times(1).Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil) - m.On("GetSystem", ctx, systemID).Times(1).Return( + m.On("GetSystem", ctx, client.nodeID).Times(1).Return( redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_OFF}, &http.Response{StatusCode: 200}, nil) @@ -129,13 +149,13 @@ func TestRebootSystemStartupError(t *testing.T) { resetOnReq.ResetType = redfishClient.RESETTYPE_ON // Mock redfish startup request for failure - m.On("ResetSystem", ctx, systemID, resetOnReq).Times(1).Return(redfishClient.RedfishError{}, + m.On("ResetSystem", ctx, client.nodeID, resetOnReq).Times(1).Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 401}, redfishClient.GenericOpenAPIError{}) // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.RebootSystem(ctx, systemID) + err = client.RebootSystem(ctx) _, ok := err.(ErrRedfishClient) assert.True(t, ok) } @@ -144,76 +164,83 @@ func TestRebootSystemTimeout(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(ephemeralNodeID, isoPath, redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + ctx := context.WithValue(context.Background(), "numRetries", 1) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF - systemID := ephemeralNodeID - m.On("ResetSystem", ctx, systemID, resetReq). + m.On("ResetSystem", ctx, client.nodeID, resetReq). Times(1). Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil) - m.On("GetSystem", ctx, systemID). + m.On("GetSystem", ctx, client.nodeID). Return(redfishClient.ComputerSystem{}, &http.Response{StatusCode: 200}, nil) // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.RebootSystem(ctx, systemID) - assert.Equal(t, ErrOperationRetriesExceeded{What: "reboot system ephemeral-node-id", Retries: 1}, err) + err = client.RebootSystem(ctx) + assert.Equal(t, ErrOperationRetriesExceeded{What: "reboot system System.Embedded.1", Retries: 1}, err) } -func TestSetEphemeralBootSourceByTypeGetSystemError(t *testing.T) { +func TestSetBootSourceByTypeGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient("invalid-server", isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + // Mock redfish get system request - m.On("GetSystem", ctx, client.ephemeralNodeID).Times(1).Return(redfishClient.ComputerSystem{}, + m.On("GetSystem", ctx, client.NodeID()).Times(1).Return(redfishClient.ComputerSystem{}, &http.Response{StatusCode: 500}, redfishClient.GenericOpenAPIError{}) // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.SetEphemeralBootSourceByType(ctx) + err = client.SetBootSourceByType(ctx) assert.Error(t, err) } -func TestSetEphemeralBootSourceByTypeSetSystemError(t *testing.T) { +func TestSetBootSourceByTypeSetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient("invalid-server", isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + httpResp := &http.Response{StatusCode: 200} - m.On("GetSystem", ctx, client.ephemeralNodeID).Return(testutil.GetTestSystem(), httpResp, nil) + m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). Return(testutil.GetVirtualMedia([]string{"CD"}), httpResp, nil) - m.On("SetSystem", ctx, client.ephemeralNodeID, mock.Anything).Times(1).Return( + m.On("SetSystem", ctx, client.nodeID, mock.Anything).Times(1).Return( redfishClient.ComputerSystem{}, &http.Response{StatusCode: 401}, redfishClient.GenericOpenAPIError{}) // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.SetEphemeralBootSourceByType(ctx) + err = client.SetBootSourceByType(ctx) assert.Error(t, err) } -func TestSetEphemeralBootSourceByTypeBootSourceUnavailable(t *testing.T) { +func TestSetBootSourceByTypeBootSourceUnavailable(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient("invalid-server", isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + invalidSystem := testutil.GetTestSystem() invalidSystem.Boot.BootSourceOverrideTargetRedfishAllowableValues = []redfishClient.BootSource{ redfishClient.BOOTSOURCE_HDD, @@ -221,7 +248,7 @@ func TestSetEphemeralBootSourceByTypeBootSourceUnavailable(t *testing.T) { } httpResp := &http.Response{StatusCode: 200} - m.On("GetSystem", ctx, client.ephemeralNodeID).Return(invalidSystem, httpResp, nil) + m.On("GetSystem", ctx, client.nodeID).Return(invalidSystem, httpResp, nil) m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). @@ -230,7 +257,7 @@ func TestSetEphemeralBootSourceByTypeBootSourceUnavailable(t *testing.T) { // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.SetEphemeralBootSourceByType(ctx) + err = client.SetBootSourceByType(ctx) _, ok := err.(ErrRedfishClient) assert.True(t, ok) } @@ -239,10 +266,11 @@ func TestSetVirtualMediaEjectVirtualMedia(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - systemID := ephemeralNodeID - _, client, err := NewClient(systemID, isoPath, redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + // Normal retries are 30. Limit them here for test time. ctx := context.WithValue(context.Background(), "numRetries", 1) @@ -252,7 +280,7 @@ func TestSetVirtualMediaEjectVirtualMedia(t *testing.T) { testMedia.Inserted = &inserted httpResp := &http.Response{StatusCode: 200} - m.On("GetSystem", ctx, client.ephemeralNodeID).Return(testutil.GetTestSystem(), httpResp, nil) + m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). @@ -271,7 +299,7 @@ func TestSetVirtualMediaEjectVirtualMedia(t *testing.T) { // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.SetVirtualMedia(ctx, client.isoPath) + err = client.SetVirtualMedia(ctx, isoPath) assert.NoError(t, err) } @@ -279,17 +307,19 @@ func TestSetVirtualMediaGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient("invalid-server", isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + // Mock redfish get system request - m.On("GetSystem", ctx, client.ephemeralNodeID).Times(1).Return(redfishClient.ComputerSystem{}, + m.On("GetSystem", ctx, client.nodeID).Times(1).Return(redfishClient.ComputerSystem{}, nil, redfishClient.GenericOpenAPIError{}) // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.SetVirtualMedia(ctx, client.isoPath) + err = client.SetVirtualMedia(ctx, isoPath) assert.Error(t, err) } @@ -297,10 +327,11 @@ func TestSetVirtualMediaEjectVirtualMediaRetriesExceeded(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - systemID := ephemeralNodeID - _, client, err := NewClient(systemID, isoPath, redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + ctx := context.WithValue(context.Background(), "numRetries", 1) // Mark test media as inserted @@ -309,7 +340,7 @@ func TestSetVirtualMediaEjectVirtualMediaRetriesExceeded(t *testing.T) { testMedia.Inserted = &inserted httpResp := &http.Response{StatusCode: 200} - m.On("GetSystem", ctx, client.ephemeralNodeID).Return(testutil.GetTestSystem(), httpResp, nil) + m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). @@ -328,7 +359,7 @@ func TestSetVirtualMediaEjectVirtualMediaRetriesExceeded(t *testing.T) { // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.SetVirtualMedia(ctx, client.isoPath) + err = client.SetVirtualMedia(ctx, isoPath) _, ok := err.(ErrOperationRetriesExceeded) assert.True(t, ok) } @@ -337,12 +368,13 @@ func TestSetVirtualMediaInsertVirtualMediaError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - systemID := ephemeralNodeID - ctx, client, err := NewClient(systemID, isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) + client.nodeID = nodeID + httpResp := &http.Response{StatusCode: 200} - m.On("GetSystem", ctx, client.ephemeralNodeID).Return(testutil.GetTestSystem(), httpResp, nil) + m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). @@ -355,7 +387,7 @@ func TestSetVirtualMediaInsertVirtualMediaError(t *testing.T) { // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.SetVirtualMedia(ctx, client.isoPath) + err = client.SetVirtualMedia(ctx, isoPath) _, ok := err.(ErrRedfishClient) assert.True(t, ok) } diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index 37832bb19..0c5298a73 100644 --- a/pkg/remote/redfish/utils.go +++ b/pkg/remote/redfish/utils.go @@ -27,7 +27,7 @@ import ( ) const ( - URLSchemeSeparator = "+" + redfishURLSchemeSeparator = "+" ) // DecodeRawError decodes a raw Redfish HTTP response and retrieves the extended information and available resolutions @@ -220,3 +220,27 @@ func ScreenRedfishError(httpResp *http.Response, clientErr error) error { return finalError } + +func getManagerID(ctx context.Context, api redfishAPI.RedfishAPI, systemID string) (string, error) { + system, _, err := api.GetSystem(ctx, systemID) + if err != nil { + return "", err + } + + return GetResourceIDFromURL(system.Links.ManagedBy[0].OdataId), nil +} + +func getBasePath(redfishURL string) (string, error) { + parsedURL, err := url.Parse(redfishURL) + if err != nil { + return "", ErrRedfishClient{Message: fmt.Sprintf("Redfish URL malformed %s", err.Error())} + } + + baseURL := fmt.Sprintf("%s://%s", parsedURL.Scheme, parsedURL.Host) + schemeSplit := strings.Split(parsedURL.Scheme, redfishURLSchemeSeparator) + if len(schemeSplit) > 1 { + baseURL = fmt.Sprintf("%s://%s", schemeSplit[len(schemeSplit)-1], parsedURL.Host) + } + + return baseURL, nil +} diff --git a/pkg/remote/redfish/vendors/dell/client.go b/pkg/remote/redfish/vendors/dell/client.go index a9a91c036..18ecd13af 100644 --- a/pkg/remote/redfish/vendors/dell/client.go +++ b/pkg/remote/redfish/vendors/dell/client.go @@ -69,9 +69,9 @@ type iDRACAPIExtendedInfo struct { Resolution string `json:"Resolution,omitempty"` } -// SetEphemeralBootSourceByType sets the boot source of the ephemeral node to a virtual CD, "VCD-DVD". -func (c *Client) SetEphemeralBootSourceByType(ctx context.Context) error { - managerID, err := redfish.GetManagerID(ctx, c.RedfishAPI, c.EphemeralNodeID()) +// SetBootSourceByType sets the boot source of the ephemeral node to a virtual CD, "VCD-DVD". +func (c *Client) SetBootSourceByType(ctx context.Context) error { + managerID, err := redfish.GetManagerID(ctx, c.RedfishAPI, c.NodeID()) if err != nil { return err } @@ -120,15 +120,12 @@ func (c *Client) SetEphemeralBootSourceByType(ctx context.Context) error { } // NewClient returns a client with the capability to make Redfish requests. -func NewClient(ephemeralNodeID string, - isoPath string, - redfishURL string, +func NewClient(redfishURL string, insecure bool, useProxy bool, username string, password string) (context.Context, *Client, error) { - ctx, genericClient, err := redfish.NewClient( - ephemeralNodeID, isoPath, redfishURL, insecure, useProxy, username, password) + ctx, genericClient, err := redfish.NewClient(redfishURL, insecure, useProxy, username, password) if err != nil { return ctx, nil, err } diff --git a/pkg/remote/redfish/vendors/dell/client_test.go b/pkg/remote/redfish/vendors/dell/client_test.go index 8a63b8a4a..2bbd3bb64 100644 --- a/pkg/remote/redfish/vendors/dell/client_test.go +++ b/pkg/remote/redfish/vendors/dell/client_test.go @@ -1,14 +1,16 @@ -// 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. +/* + 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 dell @@ -23,30 +25,28 @@ import ( ) const ( - ephemeralNodeID = "System.Embedded.1" - isoPath = "https://localhost:8080/debian.iso" - redfishURL = "redfish+https://localhost/Systems/System.Embedded.1" + redfishURL = "redfish+https://localhost/Systems/System.Embedded.1" ) func TestNewClient(t *testing.T) { - _, _, err := NewClient(ephemeralNodeID, isoPath, redfishURL, false, false, "username", "password") + _, _, err := NewClient(redfishURL, false, false, "username", "password") assert.NoError(t, err) } -func TestSetEphemeralBootSourceByTypeGetSystemError(t *testing.T) { +func TestSetBootSourceByTypeGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient("invalid-server", isoPath, redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "") assert.NoError(t, err) // Mock redfish get system request - m.On("GetSystem", ctx, client.EphemeralNodeID()).Times(1).Return(redfishClient.ComputerSystem{}, + m.On("GetSystem", ctx, client.NodeID()).Times(1).Return(redfishClient.ComputerSystem{}, &http.Response{StatusCode: 500}, redfishClient.GenericOpenAPIError{}) // Replace normal API client with mocked API client client.RedfishAPI = m - err = client.SetEphemeralBootSourceByType(ctx) + err = client.SetBootSourceByType(ctx) assert.Error(t, err) } diff --git a/pkg/remote/remote_direct.go b/pkg/remote/remote_direct.go index f0ebe9575..77172a17d 100644 --- a/pkg/remote/remote_direct.go +++ b/pkg/remote/remote_direct.go @@ -1,186 +1,63 @@ -// 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. +/* + 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" - "fmt" - "net/url" - "strings" - "opendev.org/airship/airshipctl/pkg/config" - "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/environment" - alog "opendev.org/airship/airshipctl/pkg/log" - "opendev.org/airship/airshipctl/pkg/remote/redfish" - redfishDell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell" + "opendev.org/airship/airshipctl/pkg/log" ) -// 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 - Context context.Context - remoteConfig *config.RemoteDirect - remoteURL string - username string - password string -} - -// configureClient retrieves a client for remoteDirect requests based on the RemoteType in the Airship config file. -func (a *Adapter) configureClient(remoteURL string) error { - switch a.remoteConfig.RemoteType { - case redfish.ClientType, redfishDell.ClientType: - rfURL, err := url.Parse(remoteURL) - if err != nil { - return err - } - - baseURL := fmt.Sprintf("%s://%s", rfURL.Scheme, rfURL.Host) - schemeSplit := strings.Split(rfURL.Scheme, redfish.URLSchemeSeparator) - if len(schemeSplit) > 1 { - baseURL = fmt.Sprintf("%s://%s", schemeSplit[len(schemeSplit)-1], rfURL.Host) - } - - urlPath := strings.Split(rfURL.Path, "/") - nodeID := urlPath[len(urlPath)-1] - if nodeID == "" { - return redfish.ErrRedfishMissingConfig{ - What: "redfish ephemeral node id empty", - } - } - - if a.remoteConfig.IsoURL == "" { - return redfish.ErrRedfishMissingConfig{ - What: "redfish ephemeral node iso Path empty", - } - } - - if a.remoteConfig.RemoteType == redfishDell.ClientType { - alog.Debug("Remote type: Redfish for Integrated Dell Remote Access Controller (iDrac) systems") - a.Context, a.OOBClient, err = redfishDell.NewClient( - nodeID, - a.remoteConfig.IsoURL, - baseURL, - a.remoteConfig.Insecure, - a.remoteConfig.UseProxy, - a.username, - a.password) - } else { - alog.Debug("Remote type: Redfish") - a.Context, a.OOBClient, err = redfish.NewClient( - nodeID, - a.remoteConfig.IsoURL, - baseURL, - a.remoteConfig.Insecure, - a.remoteConfig.UseProxy, - a.username, - a.password) - } - - if err != nil { - return err - } - default: - return NewRemoteDirectErrorf("invalid remote type") - } - - return nil -} - -// initializeAdapter retrieves the remote direct configuration defined in the Airship configuration file. -func (a *Adapter) initializeAdapter(settings *environment.AirshipCTLSettings) error { +// DoRemoteDirect bootstraps the ephemeral node. +func (b baremetalHost) DoRemoteDirect(settings *environment.AirshipCTLSettings) error { cfg := settings.Config bootstrapSettings, err := cfg.CurrentContextBootstrapInfo() if err != nil { return err } - a.remoteConfig = bootstrapSettings.RemoteDirect - if a.remoteConfig == nil { + remoteConfig := bootstrapSettings.RemoteDirect + if remoteConfig == nil { return config.ErrMissingConfig{What: "RemoteDirect options not defined in bootstrap config"} } - bundlePath, err := cfg.CurrentContextEntryPoint(config.Ephemeral, config.BootstrapPhase) + log.Debugf("Using ephemeral node %s with BMCAddress %s", b.NodeID(), b.BMCAddress) + + // Perform remote direct operations + if remoteConfig.IsoURL == "" { + return ErrMissingBootstrapInfoOption{What: "isoURL"} + } + + err = b.SetVirtualMedia(b.Context, remoteConfig.IsoURL) if err != nil { return err } - docBundle, err := document.NewBundleByPath(bundlePath) + log.Debugf("Successfully loaded virtual media: %q", remoteConfig.IsoURL) + + err = b.SetBootSourceByType(b.Context) if err != nil { return err } - selector := document.NewEphemeralBMHSelector() - doc, err := docBundle.SelectOne(selector) + err = b.RebootSystem(b.Context) if err != nil { return err } - a.remoteURL, err = document.GetBMHBMCAddress(doc) - if err != nil { - return err - } - - a.username, a.password, err = document.GetBMHBMCCredentials(doc, docBundle) - if err != nil { - return err - } + log.Debugf("Restarted ephemeral host %s", b.NodeID()) return nil } - -// DoRemoteDirect executes remote direct based on remote type. -func (a *Adapter) DoRemoteDirect() error { - alog.Debugf("Using Remote Endpoint: %q", a.remoteURL) - - /* Load ISO in manager's virtual media */ - err := a.OOBClient.SetVirtualMedia(a.Context, a.remoteConfig.IsoURL) - if err != nil { - return err - } - - alog.Debugf("Successfully loaded virtual media: %q", a.remoteConfig.IsoURL) - - /* Set system's bootsource to selected media */ - err = a.OOBClient.SetEphemeralBootSourceByType(a.Context) - if err != nil { - return err - } - - /* Reboot system */ - err = a.OOBClient.RebootSystem(a.Context, a.OOBClient.EphemeralNodeID()) - if err != nil { - return err - } - - alog.Debug("Restarted ephemeral host") - - return nil -} - -// NewAdapter provides an adapter that exposes the capability to perform remote direct functionality with any -// out-of-band client. -func NewAdapter(settings *environment.AirshipCTLSettings) (*Adapter, error) { - a := &Adapter{} - a.Context = context.Background() - err := a.initializeAdapter(settings) - if err != nil { - return a, err - } - - if err := a.configureClient(a.remoteURL); err != nil { - return a, err - } - - return a, nil -} diff --git a/pkg/remote/remote_direct_test.go b/pkg/remote/remote_direct_test.go index 6cdd3f013..1320b6335 100644 --- a/pkg/remote/remote_direct_test.go +++ b/pkg/remote/remote_direct_test.go @@ -15,205 +15,192 @@ 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/environment" "opendev.org/airship/airshipctl/pkg/remote/redfish" - "opendev.org/airship/airshipctl/testutil" "opendev.org/airship/airshipctl/testutil/redfishutils" ) const ( - systemID = "server-100" + systemID = "System.Embedded.1" isoURL = "https://localhost:8080/ubuntu.iso" - redfishURL = "https://redfish.local" + redfishURL = "redfish+https://localhost:2344/Systems/System.Embedded.1" + username = "admin" + password = "password" ) -func initSettings(t *testing.T, rd *config.RemoteDirect, testdata string) *environment.AirshipCTLSettings { - t.Helper() - settings := &environment.AirshipCTLSettings{Config: testutil.DummyConfig()} - bi, err := settings.Config.CurrentContextBootstrapInfo() - require.NoError(t, err) - bi.RemoteDirect = rd - cm, err := settings.Config.CurrentContextManifest() - require.NoError(t, err) - cm.TargetPath = "testdata/" + testdata - return settings +// withRemoteDirectConfig initializes the remote direct settings when used as an argument to "initSettings". +func withRemoteDirectConfig(cfg *config.RemoteDirect) Configuration { + return func(settings *environment.AirshipCTLSettings) { + bootstrapInfo, err := settings.Config.CurrentContextBootstrapInfo() + if err != nil { + panic(fmt.Sprintf("Unable to initialize remote direct tests. Current Context error %q", err)) + } + + bootstrapInfo.RemoteDirect = cfg + } } -func TestUnknownRemoteType(t *testing.T) { - s := initSettings( - t, - &config.RemoteDirect{ - RemoteType: "new-remote", - IsoURL: "/test.iso", - }, - "base", - ) +func TestDoRemoteDirectMissingConfigOpts(t *testing.T) { + ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + assert.NoError(t, err) - _, err := NewAdapter(s) - _, ok := err.(*GenericError) - assert.True(t, ok) + ephemeralHost := baremetalHost{ + rMock, + ctx, + redfishURL, + username, + password, + } + + settings := initSettings(t, withRemoteDirectConfig(nil), withTestDataPath("base")) + err = ephemeralHost.DoRemoteDirect(settings) + assert.Error(t, err) } -func TestRedfishRemoteDirectWithEmptyURL(t *testing.T) { - s := initSettings( - t, - &config.RemoteDirect{ - RemoteType: "redfish", - IsoURL: "/test.iso", - }, - "emptyurl", - ) +func TestDoRemoteDirectMissingISOURL(t *testing.T) { + ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + assert.NoError(t, err) - _, err := NewAdapter(s) - _, ok := err.(redfish.ErrRedfishMissingConfig) - assert.True(t, ok) -} + rMock.On("NodeID").Times(1).Return(systemID) -func TestRedfishRemoteDirectWithEmptyIsoPath(t *testing.T) { - s := initSettings( - t, - &config.RemoteDirect{ - RemoteType: "redfish", - IsoURL: "", - }, - "base", - ) + ephemeralHost := baremetalHost{ + rMock, + ctx, + redfishURL, + username, + password, + } - _, err := NewAdapter(s) - _, ok := err.(redfish.ErrRedfishMissingConfig) - assert.True(t, ok) -} + cfg := &config.RemoteDirect{} -func TestBootstrapRemoteDirectMissingConfigOpts(t *testing.T) { - s := initSettings( - t, - nil, - "base", - ) - - _, err := NewAdapter(s) - _, ok := err.(config.ErrMissingConfig) - assert.True(t, ok) + settings := initSettings(t, withRemoteDirectConfig(cfg), withTestDataPath("base")) + err = ephemeralHost.DoRemoteDirect(settings) + assert.Error(t, err) } func TestDoRemoteDirectRedfish(t *testing.T) { - cfg := &config.RemoteDirect{ - RemoteType: redfish.ClientType, - IsoURL: isoURL, + ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + assert.NoError(t, err) + + rMock.On("NodeID").Times(1).Return(systemID) + rMock.On("SetVirtualMedia", ctx, isoURL).Times(1).Return(nil) + rMock.On("SetBootSourceByType", ctx).Times(1).Return(nil) + rMock.On("NodeID").Times(1).Return(systemID) + rMock.On("RebootSystem", ctx).Times(1).Return(nil) + + ephemeralHost := baremetalHost{ + rMock, + ctx, + redfishURL, + username, + password, } - // Initialize a remote direct adapter - settings := initSettings(t, cfg, "base") - a, err := NewAdapter(settings) - assert.NoError(t, err) + cfg := &config.RemoteDirect{ + IsoURL: isoURL, + } - ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, "admin", "password") - assert.NoError(t, err) - - rMock.On("SetVirtualMedia", a.Context, isoURL).Times(1).Return(nil) - rMock.On("SetEphemeralBootSourceByType", a.Context).Times(1).Return(nil) - rMock.On("EphemeralNodeID").Times(1).Return(systemID) - rMock.On("RebootSystem", a.Context, systemID).Times(1).Return(nil) - - // Swap the redfish client initialized by the remote direct adapter with the above mocked client - a.Context = ctx - a.OOBClient = rMock - - err = a.DoRemoteDirect() + settings := initSettings(t, withRemoteDirectConfig(cfg), withTestDataPath("base")) + err = ephemeralHost.DoRemoteDirect(settings) assert.NoError(t, err) } func TestDoRemoteDirectRedfishVirtualMediaError(t *testing.T) { - cfg := &config.RemoteDirect{ - RemoteType: redfish.ClientType, - IsoURL: isoURL, - } - - // Initialize a remote direct adapter - settings := initSettings(t, cfg, "base") - a, err := NewAdapter(settings) - assert.NoError(t, err) - - ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, "admin", "password") + ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) expectedErr := redfish.ErrRedfishClient{Message: "Unable to set virtual media."} - rMock.On("SetVirtualMedia", a.Context, isoURL).Times(1).Return(expectedErr) - rMock.On("SetEphemeralBootSourceByType", a.Context).Times(1).Return(nil) - rMock.On("EphemeralNodeID").Times(1).Return(systemID) - rMock.On("RebootSystem", a.Context, systemID).Times(1).Return(nil) - // Swap the redfish client initialized by the remote direct adapter with the above mocked client - a.Context = ctx - a.OOBClient = rMock + rMock.On("NodeID").Times(1).Return(systemID) + rMock.On("SetVirtualMedia", ctx, isoURL).Times(1).Return(expectedErr) + rMock.On("SetBootSourceByType", ctx).Times(1).Return(nil) + rMock.On("NodeID").Times(1).Return(systemID) + rMock.On("RebootSystem", ctx).Times(1).Return(nil) - err = a.DoRemoteDirect() + ephemeralHost := baremetalHost{ + rMock, + ctx, + redfishURL, + username, + password, + } + + cfg := &config.RemoteDirect{ + IsoURL: isoURL, + } + + settings := initSettings(t, withRemoteDirectConfig(cfg), withTestDataPath("base")) + + err = ephemeralHost.DoRemoteDirect(settings) _, ok := err.(redfish.ErrRedfishClient) assert.True(t, ok) } func TestDoRemoteDirectRedfishBootSourceError(t *testing.T) { - cfg := &config.RemoteDirect{ - RemoteType: redfish.ClientType, - IsoURL: isoURL, - } - - // Initialize a remote direct adapter - settings := initSettings(t, cfg, "base") - a, err := NewAdapter(settings) + ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) - ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, "admin", "password") - assert.NoError(t, err) - - rMock.On("SetVirtualMedia", a.Context, isoURL).Times(1).Return(nil) + rMock.On("NodeID").Times(1).Return(systemID) + rMock.On("SetVirtualMedia", ctx, isoURL).Times(1).Return(nil) expectedErr := redfish.ErrRedfishClient{Message: "Unable to set boot source."} - rMock.On("SetEphemeralBootSourceByType", a.Context).Times(1).Return(expectedErr) - rMock.On("EphemeralNodeID").Times(1).Return(systemID) - rMock.On("RebootSystem", a.Context, systemID).Times(1).Return(nil) + rMock.On("SetBootSourceByType", ctx).Times(1).Return(expectedErr) + rMock.On("NodeID").Times(1).Return(systemID) + rMock.On("RebootSystem", ctx).Times(1).Return(nil) - // Swap the redfish client initialized by the remote direct adapter with the above mocked client - a.Context = ctx - a.OOBClient = rMock + ephemeralHost := baremetalHost{ + rMock, + ctx, + redfishURL, + username, + password, + } - err = a.DoRemoteDirect() + cfg := &config.RemoteDirect{ + IsoURL: isoURL, + } + + settings := initSettings(t, withRemoteDirectConfig(cfg), withTestDataPath("base")) + + err = ephemeralHost.DoRemoteDirect(settings) _, ok := err.(redfish.ErrRedfishClient) assert.True(t, ok) } func TestDoRemoteDirectRedfishRebootError(t *testing.T) { - cfg := &config.RemoteDirect{ - RemoteType: redfish.ClientType, - IsoURL: isoURL, - } - - // Initialize a remote direct adapter - settings := initSettings(t, cfg, "base") - a, err := NewAdapter(settings) + ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) - ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, "admin", "password") - assert.NoError(t, err) - - rMock.On("SetVirtualMedia", a.Context, isoURL).Times(1).Return(nil) - rMock.On("SetEphemeralBootSourceByType", a.Context).Times(1).Return(nil) - rMock.On("EphemeralNodeID").Times(1).Return(systemID) + rMock.On("NodeID").Times(1).Return(systemID) + rMock.On("SetVirtualMedia", ctx, isoURL).Times(1).Return(nil) + rMock.On("SetVirtualMedia", ctx, isoURL).Times(1).Return(nil) + rMock.On("SetBootSourceByType", ctx).Times(1).Return(nil) + rMock.On("NodeID").Times(1).Return(systemID) expectedErr := redfish.ErrRedfishClient{Message: "Unable to set boot source."} - rMock.On("RebootSystem", a.Context, systemID).Times(1).Return(expectedErr) + rMock.On("RebootSystem", ctx).Times(1).Return(expectedErr) - // Swap the redfish client initialized by the remote direct adapter with the above mocked client - a.Context = ctx - a.OOBClient = rMock + ephemeralHost := baremetalHost{ + rMock, + ctx, + redfishURL, + username, + password, + } - err = a.DoRemoteDirect() + cfg := &config.RemoteDirect{ + IsoURL: isoURL, + } + + settings := initSettings(t, withRemoteDirectConfig(cfg), withTestDataPath("base")) + + err = ephemeralHost.DoRemoteDirect(settings) _, ok := err.(redfish.ErrRedfishClient) assert.True(t, ok) } diff --git a/pkg/remote/testdata/base/manifests/site/test-site/ephemeral/bootstrap/baremetal.yaml b/pkg/remote/testdata/base/manifests/site/test-site/ephemeral/bootstrap/baremetal.yaml index bc5dbb5a6..3d26b2cd6 100644 --- a/pkg/remote/testdata/base/manifests/site/test-site/ephemeral/bootstrap/baremetal.yaml +++ b/pkg/remote/testdata/base/manifests/site/test-site/ephemeral/bootstrap/baremetal.yaml @@ -9,7 +9,7 @@ spec: online: true bootMACAddress: 00:3b:8b:0c:ec:8b bmc: - address: redfish+http://nolocalhost:8888/redfish/v1/Systems/test-node + address: redfish+http://nolocalhost:8888/redfish/v1/Systems/ephemeral credentialsName: master-0-bmc-secret --- apiVersion: v1 @@ -23,3 +23,52 @@ data: username: YWRtaW4= password: cGFzc3dvcmQ= ... +--- +apiVersion: metal3.io/v1alpha1 +kind: BareMetalHost +metadata: + labels: + airshipit.org/test-node: "true" + name: master-1 +spec: + online: true + bootMACAddress: 00:3b:8b:0c:ec:8b + bmc: + address: redfish+http://nolocalhost:8888/redfish/v1/Systems/node-master-1 + credentialsName: master-1-bmc-secret +--- +apiVersion: metal3.io/v1alpha1 +kind: BareMetalHost +metadata: + labels: + airshipit.org/test-node: "true" + name: master-2 +spec: + online: true + bootMACAddress: 00:3b:8b:0c:ec:8b + bmc: + address: redfish+http://nolocalhost:8888/redfish/v1/Systems/node-master-2 + credentialsName: master-1-bmc-secret +--- +apiVersion: v1 +kind: Secret +metadata: + labels: + airshipit.org/ephemeral-node: "true" + name: master-1-bmc-secret +type: Opaque +data: + username: YWRtaW4= + password: cGFzc3dvcmQ= +... +--- +apiVersion: metal3.io/v1alpha1 +kind: BareMetalHost +metadata: + name: no-creds +spec: + online: true + bootMACAddress: 00:3b:8b:0c:ec:8b + bmc: + address: redfish+http://nolocalhost:8888/redfish/v1/Systems/test-node +... diff --git a/pkg/remote/types.go b/pkg/remote/types.go deleted file mode 100644 index f516c190d..000000000 --- a/pkg/remote/types.go +++ /dev/null @@ -1,39 +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" -) - -// Client is a set of functions that clients created for out-of-band power management and control should implement. The -// functions within client are used by power management commands and remote direct functionality. -type Client interface { - RebootSystem(context.Context, string) error - - SystemPowerOff(context.Context, string) error - - // TODO(drewwalters96): Should this be a string forever? We may want to define our own custom type, as the - // string format will be client dependent when we add new clients. - SystemPowerStatus(context.Context, string) (string, error) - - EphemeralNodeID() string - - // TODO(drewwalters96): This function may be too tightly coupled to remoteDirect operations. This could probably - // be combined with SetVirtualMedia. - SetEphemeralBootSourceByType(context.Context) error - - // TODO(drewwalters96): This function is tightly coupled to Redfish. It should be combined with the - // SetBootSource operation and removed from the client interface. - SetVirtualMedia(context.Context, string) error -} diff --git a/roles/airshipctl-test-configs/templates/airshipconfig.j2 b/roles/airshipctl-test-configs/templates/airshipconfig.j2 index bba8aedfc..519125341 100644 --- a/roles/airshipctl-test-configs/templates/airshipconfig.j2 +++ b/roles/airshipctl-test-configs/templates/airshipconfig.j2 @@ -14,12 +14,19 @@ bootstrapInfo: userDataFileName: user-data networkConfigFileName: network-config outputMetadataFileName: output-metadata.yaml +managementConfiguration: + dummy_management_config: + type: {{ remote_type }} + insecure: {{ remote_insecure }} + useproxy: {{ remote_proxy }} + clusters: dummycluster: clusterType: ephemeral: bootstrapInfo: dummy_bootstrap_config clusterKubeconf: dummycluster_ephemeral + managementConfiguration: dummy_management_config contexts: dummy_cluster: contextKubeconf: dummy_cluster diff --git a/testutil/redfishutils/mock_client.go b/testutil/redfishutils/mock_client.go index f57d02396..c62f66949 100644 --- a/testutil/redfishutils/mock_client.go +++ b/testutil/redfishutils/mock_client.go @@ -14,7 +14,6 @@ package redfishutils import ( "context" - "net/url" "github.com/stretchr/testify/mock" redfishClient "opendev.org/airship/go-redfish/client" @@ -25,20 +24,18 @@ import ( // MockClient is a fake Redfish client for unit testing. type MockClient struct { mock.Mock - ephemeralNodeID string - isoPath string - redfishURL url.URL + nodeID string } -// EphemeralNodeID provides a stubbed method that can be mocked to test functions that use the Redfish client without +// NodeID provides a stubbed method that can be mocked to test functions that use the Redfish client without // making any Redfish API calls or requiring the appropriate Redfish client settings. // // Example usage: // client := redfishutils.NewClient() -// client.On("GetEphemeralNodeID").Return() +// client.On("NodeID").Return() // -// err := client.GetEphemeralNodeID() -func (m *MockClient) EphemeralNodeID() string { +// err := client.NodeID() +func (m *MockClient) NodeID() string { args := m.Called() return args.String(0) } @@ -51,20 +48,20 @@ func (m *MockClient) EphemeralNodeID() string { // client.On("RebootSystem").Return() // // err := client.RebootSystem() -func (m *MockClient) RebootSystem(ctx context.Context, systemID string) error { - args := m.Called(ctx, systemID) +func (m *MockClient) RebootSystem(ctx context.Context) error { + args := m.Called(ctx) return args.Error(0) } -// SetEphemeralBootSourceByType provides a stubbed method that can be mocked to test functions that use the +// SetBootSourceByType provides a stubbed method that can be mocked to test functions that use the // Redfish client without making any Redfish API calls or requiring the appropriate Redfish client settings. // // Example usage: // client := redfishutils.NewClient() -// client.On("SetEphemeralBootSourceByType").Return() +// client.On("SetBootSourceByType").Return() // -// err := client.setEphemeralBootSourceByType() -func (m *MockClient) SetEphemeralBootSourceByType(ctx context.Context) error { +// err := client.SetBootSourceByType() +func (m *MockClient) SetBootSourceByType(ctx context.Context) error { args := m.Called(ctx) return args.Error(0) } @@ -90,8 +87,8 @@ func (m *MockClient) SetVirtualMedia(ctx context.Context, isoPath string) error // client.On("SystemPowerOff").Return() // // err := client.SystemPowerOff() -func (m *MockClient) SystemPowerOff(ctx context.Context, systemID string) error { - args := m.Called(ctx, systemID) +func (m *MockClient) SystemPowerOff(ctx context.Context) error { + args := m.Called(ctx) return args.Error(0) } @@ -103,14 +100,14 @@ func (m *MockClient) SystemPowerOff(ctx context.Context, systemID string) error // client.On("SystemPowerStatus").Return() // // err := client.SystemPowerStatus() -func (m *MockClient) SystemPowerStatus(ctx context.Context, systemID string) (string, error) { - args := m.Called(ctx, systemID) +func (m *MockClient) SystemPowerStatus(ctx context.Context) (string, error) { + args := m.Called(ctx) return args.String(0), args.Error(1) } // 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, username string, +func NewClient(redfishURL string, insecure bool, useProxy bool, username string, password string) (context.Context, *MockClient, error) { var ctx context.Context if username != "" && password != "" { @@ -127,16 +124,13 @@ func NewClient(ephemeralNodeID string, isoPath string, redfishURL string, userna return ctx, nil, redfish.ErrRedfishMissingConfig{What: "Redfish URL"} } - parsedURL, err := url.Parse(redfishURL) - if err != nil { - return ctx, nil, err + // Retrieve system ID from end of Redfish URL + systemID := redfish.GetResourceIDFromURL(redfishURL) + if len(systemID) == 0 { + return ctx, nil, redfish.ErrRedfishMissingConfig{What: "management URL system ID"} } - m := &MockClient{ - ephemeralNodeID: ephemeralNodeID, - isoPath: isoPath, - redfishURL: *parsedURL, - } + m := &MockClient{nodeID: systemID} return ctx, m, nil }