From 0c3eefe036f5b859a4b0efa12b885ec63a60c853 Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Wed, 1 Apr 2020 19:12:31 +0000 Subject: [PATCH] Add remote host power status command This change introduces a command that allows the user to retrieve the power status of a remote host. Relates-To: #5 Change-Id: I4d3ded6667a5427ad6814c3d000da3becaec50a1 Signed-off-by: Drew Walters --- cmd/remote/remote.go | 7 ++- cmd/remote/remote_power_status.go | 48 +++++++++++++++++++ cmd/remote/remote_test.go | 35 -------------- .../remote-error-not-implemented.golden | 7 --- pkg/remote/redfish/client.go | 10 ++++ pkg/remote/remote_direct.go | 12 ++--- pkg/remote/remote_direct_test.go | 32 ++++++------- pkg/remote/types.go | 4 ++ testutil/redfishutils/mock_client.go | 13 +++++ 9 files changed, 100 insertions(+), 68 deletions(-) create mode 100644 cmd/remote/remote_power_status.go delete mode 100644 cmd/remote/remote_test.go delete mode 100644 cmd/remote/testdata/TestRemoteCommandGoldenOutput/remote-error-not-implemented.golden diff --git a/cmd/remote/remote.go b/cmd/remote/remote.go index eb5c67429..0e0a27654 100644 --- a/cmd/remote/remote.go +++ b/cmd/remote/remote.go @@ -16,7 +16,6 @@ import ( "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/environment" - "opendev.org/airship/airshipctl/pkg/errors" ) // NewRemoteCommand creates a new command that provides functionality to control remote entities. @@ -24,10 +23,10 @@ func NewRemoteCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Comma remoteRootCmd := &cobra.Command{ Use: "remote", Short: "Control remote entities, i.e. hosts.", - RunE: func(cmd *cobra.Command, args []string) error { - return errors.ErrNotImplemented{} - }, } + powerStatusCmd := NewPowerStatusCommand(rootSettings) + remoteRootCmd.AddCommand(powerStatusCmd) + return remoteRootCmd } diff --git a/cmd/remote/remote_power_status.go b/cmd/remote/remote_power_status.go new file mode 100644 index 000000000..6eaac55e0 --- /dev/null +++ b/cmd/remote/remote_power_status.go @@ -0,0 +1,48 @@ +// 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" + + "github.com/spf13/cobra" + + "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{ + 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) + if err != nil { + return err + } + + powerStatus, err := a.OOBClient.SystemPowerStatus(a.Context, args[0]) + if err != nil { + return err + } + + fmt.Fprintf(cmd.OutOrStdout(), "Remote host %s has power status: %s\n", args[0], powerStatus) + + return nil + }, + } + + return powerStatusCmd +} diff --git a/cmd/remote/remote_test.go b/cmd/remote/remote_test.go deleted file mode 100644 index 50bc40ff9..000000000 --- a/cmd/remote/remote_test.go +++ /dev/null @@ -1,35 +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 ( - "testing" - - "opendev.org/airship/airshipctl/pkg/errors" - "opendev.org/airship/airshipctl/testutil" -) - -func TestRemoteCommand(t *testing.T) { - tests := []*testutil.CmdTest{ - { - Name: "remote-error-not-implemented", - CmdLine: "", - Cmd: NewRemoteCommand(nil), - Error: errors.ErrNotImplemented{}, - }, - } - - for _, tt := range tests { - testutil.RunTest(t, tt) - } -} diff --git a/cmd/remote/testdata/TestRemoteCommandGoldenOutput/remote-error-not-implemented.golden b/cmd/remote/testdata/TestRemoteCommandGoldenOutput/remote-error-not-implemented.golden deleted file mode 100644 index 992df392f..000000000 --- a/cmd/remote/testdata/TestRemoteCommandGoldenOutput/remote-error-not-implemented.golden +++ /dev/null @@ -1,7 +0,0 @@ -Error: Not implemented -Usage: - remote [flags] - -Flags: - -h, --help help for remote - diff --git a/pkg/remote/redfish/client.go b/pkg/remote/redfish/client.go index a3467b6b1..a77abba53 100644 --- a/pkg/remote/redfish/client.go +++ b/pkg/remote/redfish/client.go @@ -146,6 +146,16 @@ func (c *Client) SetVirtualMedia(ctx context.Context, isoPath string) error { 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) + if err = ScreenRedfishError(httpResp, err); err != nil { + return "", err + } + + return string(computerSystem.PowerState), nil +} + // NewClient returns a client with the capability to make Redfish requests. func NewClient(ephemeralNodeID string, isoPath string, diff --git a/pkg/remote/remote_direct.go b/pkg/remote/remote_direct.go index 3b57c83df..5b6a015b2 100644 --- a/pkg/remote/remote_direct.go +++ b/pkg/remote/remote_direct.go @@ -32,7 +32,7 @@ const ( // 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 + Context context.Context remoteConfig *config.RemoteDirect remoteURL string username string @@ -70,7 +70,7 @@ func (a *Adapter) configureClient(remoteURL string) error { } } - a.context, a.OOBClient, err = redfish.NewClient( + a.Context, a.OOBClient, err = redfish.NewClient( nodeID, a.remoteConfig.IsoURL, baseURL, @@ -136,7 +136,7 @@ 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) + err := a.OOBClient.SetVirtualMedia(a.Context, a.remoteConfig.IsoURL) if err != nil { return err } @@ -144,13 +144,13 @@ func (a *Adapter) DoRemoteDirect() error { alog.Debugf("Successfully loaded virtual media: %q", a.remoteConfig.IsoURL) /* Set system's bootsource to selected media */ - err = a.OOBClient.SetEphemeralBootSourceByType(a.context) + err = a.OOBClient.SetEphemeralBootSourceByType(a.Context) if err != nil { return err } /* Reboot system */ - err = a.OOBClient.RebootSystem(a.context, a.OOBClient.EphemeralNodeID()) + err = a.OOBClient.RebootSystem(a.Context, a.OOBClient.EphemeralNodeID()) if err != nil { return err } @@ -164,7 +164,7 @@ func (a *Adapter) DoRemoteDirect() error { // out-of-band client. func NewAdapter(settings *environment.AirshipCTLSettings) (*Adapter, error) { a := &Adapter{} - a.context = context.Background() + a.Context = context.Background() err := a.initializeAdapter(settings) if err != nil { return a, err diff --git a/pkg/remote/remote_direct_test.go b/pkg/remote/remote_direct_test.go index a501215e4..0df7a2989 100644 --- a/pkg/remote/remote_direct_test.go +++ b/pkg/remote/remote_direct_test.go @@ -103,13 +103,13 @@ func TestDoRemoteDirectRedfish(t *testing.T) { ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, false, false, "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("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) + 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.Context = ctx a.OOBClient = rMock err = a.DoRemoteDirect() @@ -131,13 +131,13 @@ func TestDoRemoteDirectRedfishVirtualMediaError(t *testing.T) { 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("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) + 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.Context = ctx a.OOBClient = rMock err = a.DoRemoteDirect() @@ -159,15 +159,15 @@ func TestDoRemoteDirectRedfishBootSourceError(t *testing.T) { ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, false, false, "admin", "password") assert.NoError(t, err) - rMock.On("SetVirtualMedia", a.context, isoURL).Times(1).Return(nil) + rMock.On("SetVirtualMedia", a.Context, 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("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("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.Context = ctx a.OOBClient = rMock err = a.DoRemoteDirect() @@ -189,15 +189,15 @@ func TestDoRemoteDirectRedfishRebootError(t *testing.T) { ctx, rMock, err := redfishutils.NewClient(systemID, isoURL, redfishURL, false, false, "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("SetVirtualMedia", a.Context, isoURL).Times(1).Return(nil) + rMock.On("SetEphemeralBootSourceByType", a.Context).Times(1).Return(nil) rMock.On("EphemeralNodeID").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", a.Context, systemID).Times(1).Return(expectedErr) // Swap the redfish client initialized by the remote direct adapter with the above mocked client - a.context = ctx + a.Context = ctx a.OOBClient = rMock err = a.DoRemoteDirect() diff --git a/pkg/remote/types.go b/pkg/remote/types.go index 2fab92264..6b4b8dc4a 100644 --- a/pkg/remote/types.go +++ b/pkg/remote/types.go @@ -20,6 +20,10 @@ import ( // functions within client are used by power management commands and remote direct functionality. type Client interface { RebootSystem(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 diff --git a/testutil/redfishutils/mock_client.go b/testutil/redfishutils/mock_client.go index b2ca622fb..c8b58c32d 100644 --- a/testutil/redfishutils/mock_client.go +++ b/testutil/redfishutils/mock_client.go @@ -82,6 +82,19 @@ func (m *MockClient) SetVirtualMedia(ctx context.Context, isoPath string) error return args.Error(0) } +// SystemPowerStatus 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("SystemPowerStatus").Return() +// +// err := client.SystemPowerStatus() +func (m *MockClient) SystemPowerStatus(ctx context.Context, systemID string) (string, error) { + args := m.Called(ctx, systemID) + 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, insecure bool,