From d38b9b5f0bb4518aee5d229732e21d735b7043cc Mon Sep 17 00:00:00 2001 From: shon phand Date: Mon, 18 Oct 2021 18:26:40 +0000 Subject: [PATCH] Enhance baremetal subcommand to list hosts * added NodeName field in the remote Client interface * added new subcommand 'list-hosts' to list hosts from host-inventory from site manifests Closes: #359 Signed-off-by: Shon Phand Change-Id: I560a8117b1d55cad2a634df0d05a4aaedae2a873 --- cmd/baremetal/baremetal.go | 1 + cmd/baremetal/baremetal_test.go | 5 + cmd/baremetal/listhosts.go | 58 ++++++++++++ .../baremetal-list-hosts-with-help.golden | 20 ++++ .../baremetal-with-help.golden | 1 + .../cli/baremetal/airshipctl_baremetal.rst | 1 + .../airshipctl_baremetal_list-hosts.rst | 54 +++++++++++ docs/source/cli/baremetal/index.rst | 1 + .../airshipit.org_baremetalmanagers.yaml | 2 + .../airshipit.org_clusterctls.yaml | 93 +++---------------- .../airshipit.org_hosts.yaml | 51 ++++++++++ .../airshipit.org_isoconfigurations.yaml | 16 ++++ .../airshipit.org_kubeconfigs.yaml | 33 +++++++ .../airshipit.org_kubernetesapplies.yaml | 39 ++++++++ .../airshipit.org_phases.yaml | 26 +++++- ...airshipit.org_replacementtransformers.yaml | 6 ++ .../airshipit.org_versionscatalogues.yaml | 6 +- pkg/api/v1alpha1/host_types.go | 37 ++++++++ pkg/api/v1alpha1/zz_generated.deepcopy.go | 26 ++++++ pkg/inventory/baremetal/baremetal.go | 4 +- pkg/inventory/command.go | 61 ++++++++++++ pkg/inventory/command_test.go | 55 +++++++++++ pkg/inventory/ifc/interface.go | 2 + pkg/inventory/testdata/hosts.yaml | 3 +- pkg/remote/ifc/ifc.go | 3 +- pkg/remote/redfish/client.go | 13 ++- pkg/remote/redfish/client_test.go | 71 +++++++------- pkg/remote/redfish/vendors/dell/client.go | 8 +- .../redfish/vendors/dell/client_test.go | 9 +- pkg/util/tableprinter.go | 2 + testutil/redfishutils/mock_client.go | 20 +++- 31 files changed, 592 insertions(+), 135 deletions(-) create mode 100644 cmd/baremetal/listhosts.go create mode 100644 cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-list-hosts-with-help.golden create mode 100644 docs/source/cli/baremetal/airshipctl_baremetal_list-hosts.rst create mode 100644 manifests/function/airshipctl-schemas/airshipit.org_hosts.yaml create mode 100644 pkg/api/v1alpha1/host_types.go diff --git a/cmd/baremetal/baremetal.go b/cmd/baremetal/baremetal.go index 9f71d28a8..cecdd6523 100644 --- a/cmd/baremetal/baremetal.go +++ b/cmd/baremetal/baremetal.go @@ -85,6 +85,7 @@ func NewBaremetalCommand(cfgFactory config.Factory) *cobra.Command { baremetalRootCmd.AddCommand(NewPowerStatusCommand(cfgFactory, options)) baremetalRootCmd.AddCommand(NewRebootCommand(cfgFactory, options)) baremetalRootCmd.AddCommand(NewRemoteDirectCommand(cfgFactory, options)) + baremetalRootCmd.AddCommand(NewListHostsCommand(cfgFactory, options)) return baremetalRootCmd } diff --git a/cmd/baremetal/baremetal_test.go b/cmd/baremetal/baremetal_test.go index c13f72e6b..4a142f845 100644 --- a/cmd/baremetal/baremetal_test.go +++ b/cmd/baremetal/baremetal_test.go @@ -59,6 +59,11 @@ func TestBaremetal(t *testing.T) { CmdLine: "-h", Cmd: baremetal.NewRemoteDirectCommand(nil, &inventory.CommandOptions{}), }, + { + Name: "baremetal-list-hosts-with-help", + CmdLine: "-h", + Cmd: baremetal.NewListHostsCommand(nil, &inventory.CommandOptions{}), + }, } for _, tt := range tests { diff --git a/cmd/baremetal/listhosts.go b/cmd/baremetal/listhosts.go new file mode 100644 index 000000000..67ae9ade9 --- /dev/null +++ b/cmd/baremetal/listhosts.go @@ -0,0 +1,58 @@ +/* + 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 baremetal + +import ( + "time" + + "github.com/spf13/cobra" + + "opendev.org/airship/airshipctl/pkg/config" + "opendev.org/airship/airshipctl/pkg/inventory" +) + +var ( + listHostsCommand = "list-hosts" + listLong = "List bare metal host(s)." + listExample = ` + Retrieve list of baremetal hosts, default output option is 'table' + # airshipctl baremetal list-hosts + # airshipctl baremetal list-hosts --namespace default + # airshipctl baremetal list-hosts --namespace default --output table + # airshipctl baremetal list-hosts --output yaml +` +) + +// NewListHostsCommand provides a command to list a remote host. +func NewListHostsCommand(cfgFactory config.Factory, options *inventory.CommandOptions) *cobra.Command { + l := &inventory.ListHostsCommand{Options: options} + cmd := &cobra.Command{ + Use: listHostsCommand, + Short: "Airshipctl command to list bare metal host(s)", + Long: listLong, + Example: listExample, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + l.Writer = cmd.OutOrStdout() + return l.RunE() + }, + } + flags := cmd.Flags() + flags.StringVarP(&l.OutputFormat, "output", "o", "table", "output formats. Supported options are 'table' and 'yaml'") + flags.StringVarP(&options.Namespace, flagNamespace, flagNamespaceSort, "", flagNamespaceDescription) + flags.StringVarP(&options.Labels, flagLabel, flagLabelShort, "", flagLabelDescription) + flags.DurationVar(&options.Timeout, flagTimeout, 10*time.Minute, flagTimeoutDescription) + return cmd +} diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-list-hosts-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-list-hosts-with-help.golden new file mode 100644 index 000000000..52fb96841 --- /dev/null +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-list-hosts-with-help.golden @@ -0,0 +1,20 @@ +List bare metal host(s). + +Usage: + list-hosts [flags] + +Examples: + + Retrieve list of baremetal hosts, default output option is 'table' + # airshipctl baremetal list-hosts + # airshipctl baremetal list-hosts --namespace default + # airshipctl baremetal list-hosts --namespace default --output table + # airshipctl baremetal list-hosts --output yaml + + +Flags: + -h, --help help for list-hosts + -l, --labels string label(s) to filter desired bare metal host from site manifest documents + -n, --namespace string airshipctl phase that contains the desired bare metal host from site manifest document(s) + -o, --output string output formats. Supported options are 'table' and 'yaml' (default "table") + --timeout duration timeout on bare metal action (default 10m0s) diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden index 2dda1d574..63e4a298c 100644 --- a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden @@ -7,6 +7,7 @@ Usage: Available Commands: ejectmedia Airshipctl command to eject virtual media attached to a bare metal host help Help about any command + list-hosts Airshipctl command to list bare metal host(s) poweroff Airshipctl command to shutdown bare metal host(s) poweron Airshipctl command to power on host(s) powerstatus Airshipctl command to retrieve the power status of a bare metal host diff --git a/docs/source/cli/baremetal/airshipctl_baremetal.rst b/docs/source/cli/baremetal/airshipctl_baremetal.rst index 46bba02a7..defbc55de 100644 --- a/docs/source/cli/baremetal/airshipctl_baremetal.rst +++ b/docs/source/cli/baremetal/airshipctl_baremetal.rst @@ -33,6 +33,7 @@ SEE ALSO * :ref:`airshipctl ` - A unified command line tool for management of end-to-end kubernetes cluster deployment on cloud infrastructure environments. * :ref:`airshipctl baremetal ejectmedia ` - Airshipctl command to eject virtual media attached to a bare metal host +* :ref:`airshipctl baremetal list-hosts ` - Airshipctl command to list bare metal host(s) * :ref:`airshipctl baremetal poweroff ` - Airshipctl command to shutdown bare metal host(s) * :ref:`airshipctl baremetal poweron ` - Airshipctl command to power on host(s) * :ref:`airshipctl baremetal powerstatus ` - Airshipctl command to retrieve the power status of a bare metal host diff --git a/docs/source/cli/baremetal/airshipctl_baremetal_list-hosts.rst b/docs/source/cli/baremetal/airshipctl_baremetal_list-hosts.rst new file mode 100644 index 000000000..7180e9a0e --- /dev/null +++ b/docs/source/cli/baremetal/airshipctl_baremetal_list-hosts.rst @@ -0,0 +1,54 @@ +.. _airshipctl_baremetal_list-hosts: + +airshipctl baremetal list-hosts +------------------------------- + +Airshipctl command to list bare metal host(s) + +Synopsis +~~~~~~~~ + + +List bare metal host(s). + +:: + + airshipctl baremetal list-hosts [flags] + +Examples +~~~~~~~~ + +:: + + + Retrieve list of baremetal hosts, default output option is 'table' + # airshipctl baremetal list-hosts + # airshipctl baremetal list-hosts --namespace default + # airshipctl baremetal list-hosts --namespace default --output table + # airshipctl baremetal list-hosts --output yaml + + +Options +~~~~~~~ + +:: + + -h, --help help for list-hosts + -l, --labels string label(s) to filter desired bare metal host from site manifest documents + -n, --namespace string airshipctl phase that contains the desired bare metal host from site manifest document(s) + -o, --output string output formats. Supported options are 'table' and 'yaml' (default "table") + --timeout duration timeout on bare metal action (default 10m0s) + +Options inherited from parent commands +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:: + + --airshipconf string path to the airshipctl configuration file. Defaults to "$HOME/.airship/config" + --debug enable verbose output + +SEE ALSO +~~~~~~~~ + +* :ref:`airshipctl baremetal ` - Airshipctl command to manage bare metal host(s) + diff --git a/docs/source/cli/baremetal/index.rst b/docs/source/cli/baremetal/index.rst index 7be005f57..91437eb3f 100644 --- a/docs/source/cli/baremetal/index.rst +++ b/docs/source/cli/baremetal/index.rst @@ -7,6 +7,7 @@ baremetal airshipctl_baremetal airshipctl_baremetal_ejectmedia + airshipctl_baremetal_list-hosts airshipctl_baremetal_poweroff airshipctl_baremetal_poweron airshipctl_baremetal_powerstatus diff --git a/manifests/function/airshipctl-schemas/airshipit.org_baremetalmanagers.yaml b/manifests/function/airshipctl-schemas/airshipit.org_baremetalmanagers.yaml index e6069d5b1..b225f1e6a 100644 --- a/manifests/function/airshipctl-schemas/airshipit.org_baremetalmanagers.yaml +++ b/manifests/function/airshipctl-schemas/airshipit.org_baremetalmanagers.yaml @@ -68,6 +68,8 @@ spec: required: - isoURL type: object + required: + - remoteDirect type: object timeout: description: Timeout in seconds diff --git a/manifests/function/airshipctl-schemas/airshipit.org_clusterctls.yaml b/manifests/function/airshipctl-schemas/airshipit.org_clusterctls.yaml index c317e27a6..910df5b52 100644 --- a/manifests/function/airshipctl-schemas/airshipit.org_clusterctls.yaml +++ b/manifests/function/airshipctl-schemas/airshipit.org_clusterctls.yaml @@ -35,10 +35,6 @@ spec: of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' type: string - env-vars: - description: EnvVars if set to true, allows to source variables for cluster-api - components for environment variables. - type: boolean images: additionalProperties: description: ImageMeta is part of clusterctl config @@ -53,66 +49,24 @@ spec: description: InitOptions container with exposed clusterctl InitOptions properties: bootstrap-providers: - description: BootstrapProviders and versions (e.g. kubeadm:v0.3.0) - to add to the management cluster. If unspecified, the kubeadm bootstrap - provider's latest release is used. - items: - type: string - type: array + description: BootstrapProviders and versions (comma separated, e.g. + kubeadm:v0.3.0) to add to the management cluster. If unspecified, + the kubeadm bootstrap provider's latest release is used. + type: string control-plane-providers: - description: ControlPlaneProviders and versions (e.g. kubeadm:v0.3.0) - to add to the management cluster. If unspecified, the kubeadm control - plane provider latest release is used. - items: - type: string - type: array + description: ControlPlaneProviders and versions (comma separated, + e.g. kubeadm:v0.3.0) to add to the management cluster. If unspecified, + the kubeadm control plane provider latest release is used. + type: string core-provider: description: CoreProvider version (e.g. cluster-api:v0.3.0) to add to the management cluster. If unspecified, the cluster-api core provider's latest release is used. type: string infrastructure-providers: - description: InfrastructureProviders and versions (e.g. aws:v0.5.0) - to add to the management cluster. - items: - type: string - type: array - kubeConfigRef: - description: KubeConfigRef reference to KubeConfig document - properties: - apiVersion: - description: API version of the referent. - type: string - fieldPath: - description: 'If referring to a piece of an object instead of - an entire object, this string should contain a valid JSON/Go - field access statement, such as desiredState.manifest.containers[2]. - For example, if the object reference is to a container within - a pod, this would take on a value like: "spec.containers{name}" - (where "name" refers to the name of the container that triggered - the event) or if no container name is specified "spec.containers[2]" - (container with index 2 in this pod). This syntax is chosen - only to have some well-defined way of referencing a part of - an object. TODO: this design is not final and this field is - subject to change in the future.' - type: string - kind: - description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' - type: string - namespace: - description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' - type: string - resourceVersion: - description: 'Specific resourceVersion to which this reference - is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' - type: string - uid: - description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' - type: string - type: object + description: InfrastructureProviders and versions (comma separated, + e.g. aws:v0.5.0,metal3:v0.4.0) to add to the management cluster. + type: string type: object kind: description: 'Kind is a string value representing the REST resource this @@ -125,39 +79,22 @@ spec: description: MoveOptions carries the options supported by move. properties: namespace: - description: The namespace where the workload cluster is hosted. If - unspecified, the target context's namespace is used. + description: Namespace where the objects describing the workload cluster + exists. If unspecified, the current namespace will be used. type: string type: object providers: items: description: Provider is part of clusterctl config properties: - clusterctl-repository: - description: IsClusterctlRepository if set to true, clusterctl provider's - repository implementation will be used if omitted or set to false, - airshipctl repository implementation will be used. - type: boolean name: type: string type: type: string url: + description: URL can contain remote URL of upstream Provider or + relative to target path of the manifest type: string - variable-substitution: - description: VariableSubstitution indicates weather you want to - substitute variables in the cluster-api manifests if set to true, - variables will be substituted only if they are defined either - in Environment or in AdditionalComponentVariables, if not they - will be left as is. - type: boolean - versions: - additionalProperties: - type: string - description: Map of versions where each key is a version and value - is path relative to target path of the manifest ignored if IsClusterctlRepository - is set to true - type: object required: - name - type diff --git a/manifests/function/airshipctl-schemas/airshipit.org_hosts.yaml b/manifests/function/airshipctl-schemas/airshipit.org_hosts.yaml new file mode 100644 index 000000000..2bf92f7a8 --- /dev/null +++ b/manifests/function/airshipctl-schemas/airshipit.org_hosts.yaml @@ -0,0 +1,51 @@ + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.6.1 + creationTimestamp: null + name: hosts.airshipit.org +spec: + group: airshipit.org + names: + kind: Host + listKind: HostList + plural: hosts + singular: host + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: Host object to represent baremetal host + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + nodeid: + type: string + nodename: + type: string + required: + - nodeid + - nodename + type: object + served: true + storage: true +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/manifests/function/airshipctl-schemas/airshipit.org_isoconfigurations.yaml b/manifests/function/airshipctl-schemas/airshipit.org_isoconfigurations.yaml index ba122c92b..de14ccc5f 100644 --- a/manifests/function/airshipctl-schemas/airshipit.org_isoconfigurations.yaml +++ b/manifests/function/airshipctl-schemas/airshipit.org_isoconfigurations.yaml @@ -51,6 +51,11 @@ spec: type: string group: type: string + isClusterScoped: + description: isClusterScoped is true if the object is known, + per the openapi data in use, to be cluster scoped, and false + otherwise. + type: boolean kind: type: string labelSelector: @@ -59,8 +64,11 @@ spec: It matches with the resource labels. type: string name: + description: Name of the resource. type: string namespace: + description: Namespace the resource belongs to, if it can + belong to a namespace. type: string version: type: string @@ -90,6 +98,11 @@ spec: type: string group: type: string + isClusterScoped: + description: isClusterScoped is true if the object is known, + per the openapi data in use, to be cluster scoped, and false + otherwise. + type: boolean kind: type: string labelSelector: @@ -98,8 +111,11 @@ spec: It matches with the resource labels. type: string name: + description: Name of the resource. type: string namespace: + description: Namespace the resource belongs to, if it can + belong to a namespace. type: string version: type: string diff --git a/manifests/function/airshipctl-schemas/airshipit.org_kubeconfigs.yaml b/manifests/function/airshipctl-schemas/airshipit.org_kubeconfigs.yaml index d7493dcb4..738786af1 100644 --- a/manifests/function/airshipctl-schemas/airshipit.org_kubeconfigs.yaml +++ b/manifests/function/airshipctl-schemas/airshipit.org_kubeconfigs.yaml @@ -75,10 +75,27 @@ spec: for the server's certificate. This will make your HTTPS connections insecure. type: boolean + proxy-url: + description: "ProxyURL is the URL to the proxy to be used + for all requests made by this client. URLs with \"http\", + \"https\", and \"socks5\" schemes are supported. If this + configuration is not provided or the empty string, the + client attempts to construct a proxy configuration from + http_proxy and https_proxy environment variables. If these + environment variables are not set, the client does not + attempt to proxy requests. \n socks5 proxying does not + currently support spdy streaming endpoints (exec, attach, + port forward)." + type: string server: description: Server is the address of the kubernetes cluster (https://hostname:port). type: string + tls-server-name: + description: TLSServerName is used to check server certificate. + If TLSServerName is empty, the hostname used to contact + the server is used. + type: string required: - server type: object @@ -288,8 +305,24 @@ spec: - value type: object type: array + installHint: + description: This text is shown to the user when the + executable doesn't seem to be present. For example, + `brew install foo-cli` might be a good InstallHint + for foo-cli on Mac OS systems. + type: string + provideClusterInfo: + description: ProvideClusterInfo determines whether or + not to provide cluster information, which could potentially + contain very large CA data, to this exec plugin as + a part of the KUBERNETES_EXEC_INFO environment variable. + By default, it is set to false. Package k8s.io/client-go/tools/auth/exec + provides helper methods for reading this environment + variable. + type: boolean required: - command + - provideClusterInfo type: object extensions: description: Extensions holds additional information. This diff --git a/manifests/function/airshipctl-schemas/airshipit.org_kubernetesapplies.yaml b/manifests/function/airshipctl-schemas/airshipit.org_kubernetesapplies.yaml index c4e14c107..b055522cd 100644 --- a/manifests/function/airshipctl-schemas/airshipit.org_kubernetesapplies.yaml +++ b/manifests/function/airshipctl-schemas/airshipit.org_kubernetesapplies.yaml @@ -31,6 +31,16 @@ spec: description: ApplyConfig provides instructions on how to apply resources to kubernetes cluster properties: + context: + type: string + debug: + type: boolean + druRun: + type: boolean + kubeconfig: + type: string + phaseName: + type: string pruneOptions: description: ApplyPruneOptions provides instructions how to prune for kubernetes resources @@ -42,6 +52,35 @@ spec: description: ApplyWaitOptions provides instructions how to wait for kubernetes resources properties: + conditions: + items: + description: Condition is a jsonpath for particular TypeMeta + which indicates what state to wait + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of + this representation of an object. Servers should convert + recognized schemas to the latest internal value, and may + reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + jsonPath: + type: string + kind: + description: 'Kind is a string value representing the REST + resource this object represents. Servers may infer this + from the endpoint the client submits requests to. Cannot + be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + value: + description: Value is desired state to wait for, if no value + specified - just existence of provided jsonPath will be + checked + type: string + type: object + type: array + pollInterval: + description: PollInterval in seconds + type: integer timeout: description: Timeout in seconds type: integer diff --git a/manifests/function/airshipctl-schemas/airshipit.org_phases.yaml b/manifests/function/airshipctl-schemas/airshipit.org_phases.yaml index c952cfb39..200971a7a 100644 --- a/manifests/function/airshipctl-schemas/airshipit.org_phases.yaml +++ b/manifests/function/airshipctl-schemas/airshipit.org_phases.yaml @@ -34,8 +34,30 @@ spec: documentEntryPoint: type: string executorRef: - description: ObjectReference contains enough information to let you - inspect or modify the referred object. + description: 'ObjectReference contains enough information to let you + inspect or modify the referred object. --- New uses of this type + are discouraged because of difficulty describing its usage when + embedded in APIs. 1. Ignored fields. It includes many fields which + are not generally honored. For instance, ResourceVersion and FieldPath + are both very rarely valid in actual usage. 2. Invalid usage help. It + is impossible to add specific help for individual usage. In most + embedded usages, there are particular restrictions like, "must + refer only to types A and B" or "UID not honored" or "name must + be restricted". Those cannot be well described when embedded. 3. + Inconsistent validation. Because the usages are different, the + validation rules are different by usage, which makes it hard for + users to predict what will happen. 4. The fields are both imprecise + and overly precise. Kind is not a precise mapping to a URL. This + can produce ambiguity during interpretation and require a REST + mapping. In most cases, the dependency is on the group,resource + tuple and the version of the actual struct is irrelevant. 5. + We cannot easily change it. Because this type is embedded in many + locations, updates to this type will affect numerous schemas. Don''t + make new APIs embed an underspecified API type they do not control. + Instead of using this type, create a locally provided and used type + that is well-focused on your reference. For example, ServiceReferences + for admission registration: https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 + .' properties: apiVersion: description: API version of the referent. diff --git a/manifests/function/airshipctl-schemas/airshipit.org_replacementtransformers.yaml b/manifests/function/airshipctl-schemas/airshipit.org_replacementtransformers.yaml index bbfe10ee2..8e86f592f 100644 --- a/manifests/function/airshipctl-schemas/airshipit.org_replacementtransformers.yaml +++ b/manifests/function/airshipctl-schemas/airshipit.org_replacementtransformers.yaml @@ -98,8 +98,11 @@ spec: It matches with the resource labels. type: string name: + description: Name of the resource. type: string namespace: + description: Namespace the resource belongs to, if it can + belong to a namespace. type: string version: type: string @@ -133,8 +136,11 @@ spec: It matches with the resource labels. type: string name: + description: Name of the resource. type: string namespace: + description: Namespace the resource belongs to, if it + can belong to a namespace. type: string version: type: string diff --git a/manifests/function/airshipctl-schemas/airshipit.org_versionscatalogues.yaml b/manifests/function/airshipctl-schemas/airshipit.org_versionscatalogues.yaml index 8e012c169..43a60c91c 100644 --- a/manifests/function/airshipctl-schemas/airshipit.org_versionscatalogues.yaml +++ b/manifests/function/airshipctl-schemas/airshipit.org_versionscatalogues.yaml @@ -83,9 +83,9 @@ spec: description: capi_images defines collections of images used by cluster API. The name of each key in this section should correspond to the airshipctl function in which the images will be used, such as "capm3". - Each capi_image object must have a "manager" object, which must have - "repository" and "tag" properties defined. capi_images may also include - an optional "ipam-manager" or "auth_proxy" object, + Each capi_image object must have a "manager" object, which must + have "repository" and "tag" properties defined. capi_images may + also include an optional "ipam-manager" or "auth_proxy" object, which must also have "repository" and "tag" properties defined. type: object charts: diff --git a/pkg/api/v1alpha1/host_types.go b/pkg/api/v1alpha1/host_types.go new file mode 100644 index 000000000..1f4b27fcc --- /dev/null +++ b/pkg/api/v1alpha1/host_types.go @@ -0,0 +1,37 @@ +/* + 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 v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// +kubebuilder:object:root=true + +// Host object to represent baremetal host +type Host struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + NodeName string `json:"nodename"` + NodeID string `json:"nodeid"` +} + +// DefaultHost can be used to safely unmarshal phase object without nil pointers +func DefaultHost() *Host { + return &Host{ + NodeName: "defaultName", + NodeID: "defaultID", + } +} diff --git a/pkg/api/v1alpha1/zz_generated.deepcopy.go b/pkg/api/v1alpha1/zz_generated.deepcopy.go index 23f5ba554..1943732a1 100644 --- a/pkg/api/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/api/v1alpha1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* @@ -708,6 +709,31 @@ func (in *Gvk) DeepCopy() *Gvk { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Host) DeepCopyInto(out *Host) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Host. +func (in *Host) DeepCopy() *Host { + if in == nil { + return nil + } + out := new(Host) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *Host) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HostNetworkingSpec) DeepCopyInto(out *HostNetworkingSpec) { *out = *in diff --git a/pkg/inventory/baremetal/baremetal.go b/pkg/inventory/baremetal/baremetal.go index a720ad3b8..152873ed1 100644 --- a/pkg/inventory/baremetal/baremetal.go +++ b/pkg/inventory/baremetal/baremetal.go @@ -149,7 +149,9 @@ func (i Inventory) newHost(doc document.Document) (Host, error) { } } - client, err := clientFactory( + nodeName := doc.GetName() + + client, err := clientFactory(nodeName, address, i.mgmtCfg.Insecure, i.mgmtCfg.UseProxy, diff --git a/pkg/inventory/command.go b/pkg/inventory/command.go index 13fabc54e..fbfe8ad92 100644 --- a/pkg/inventory/command.go +++ b/pkg/inventory/command.go @@ -20,8 +20,18 @@ import ( "io" "time" + "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/inventory/ifc" remoteifc "opendev.org/airship/airshipctl/pkg/remote/ifc" + "opendev.org/airship/airshipctl/pkg/util" + "opendev.org/airship/airshipctl/pkg/util/yaml" +) + +const ( + // TableOutputFormat table + TableOutputFormat = "table" + // YamlOutputFormat yaml + YamlOutputFormat = "yaml" ) // CommandOptions is used to store common variables from cmd flags for baremetal command group @@ -37,6 +47,18 @@ type CommandOptions struct { Inventory ifc.Inventory } +// ListHostsCommand is used to store common variables from cmd flags for list-hots command +type ListHostsCommand struct { + Writer io.Writer + Options *CommandOptions + OutputFormat string +} + +//NewListHostsCommand ListHostsCommand constructor +func NewListHostsCommand(options *CommandOptions) *ListHostsCommand { + return &ListHostsCommand{Options: options} +} + // NewOptions options constructor func NewOptions(i ifc.Inventory) *CommandOptions { return &CommandOptions{ @@ -61,6 +83,22 @@ func (o *CommandOptions) validateSingleHostAction() error { return nil } +//RunE method returns list of hots from BaremetalInventory +func (l *ListHostsCommand) RunE() error { + if l.OutputFormat != TableOutputFormat && l.OutputFormat != YamlOutputFormat { + return ErrInvalidOptions{Message: "output formats. Supported options are 'table' and 'yaml'"} + } + + hostClients, err := l.Options.getAllHost() + if err != nil { + return err + } + if len(hostClients) == 0 { + return fmt.Errorf("No hosts present in the hostInventory") + } + return l.Write(hostClients) +} + // BMHAction performs an action against BaremetalHost objects func (o *CommandOptions) BMHAction(op ifc.BaremetalOperation) error { if err := o.validateBMHAction(); err != nil { @@ -124,9 +162,32 @@ func (o *CommandOptions) getHost() (remoteifc.Client, error) { return bmhInventory.SelectOne(o.selector()) } +func (o *CommandOptions) getAllHost() ([]remoteifc.Client, error) { + bmhInventory, err := o.Inventory.BaremetalInventory() + if err != nil { + return nil, err + } + + return bmhInventory.Select(o.selector()) +} + func (o *CommandOptions) selector() ifc.BaremetalHostSelector { return (ifc.BaremetalHostSelector{}). ByLabel(o.Labels). ByName(o.Name). ByNamespace(o.Namespace) } + +func (l *ListHostsCommand) Write(clients []remoteifc.Client) error { + hostList := []*v1alpha1.Host{} + for _, client := range clients { + host := v1alpha1.DefaultHost() + host.NodeID = client.NodeID() + host.NodeName = client.NodeName() + hostList = append(hostList, host) + } + if l.OutputFormat == YamlOutputFormat { + return yaml.WriteOut(l.Writer, hostList) + } + return util.PrintObjects(hostList, util.HostListFormat, l.Writer, false) +} diff --git a/pkg/inventory/command_test.go b/pkg/inventory/command_test.go index c112c9507..29c57e1b3 100644 --- a/pkg/inventory/command_test.go +++ b/pkg/inventory/command_test.go @@ -24,13 +24,68 @@ import ( "opendev.org/airship/airshipctl/pkg/inventory" "opendev.org/airship/airshipctl/pkg/inventory/ifc" + remoteifc "opendev.org/airship/airshipctl/pkg/remote/ifc" "opendev.org/airship/airshipctl/pkg/remote/power" + "opendev.org/airship/airshipctl/pkg/remote/redfish" mockinventory "opendev.org/airship/airshipctl/testutil/inventory" "opendev.org/airship/airshipctl/testutil/redfishutils" ) const testNode = "node-0" +func TestListHostsCommand(t *testing.T) { + t.Run("success ListHosts", func(t *testing.T) { + var hosts []remoteifc.Client + + c1, err := redfish.ClientFactory("node-0", "redfish+http://nolocalhost:32201/redfish/v1/Systems/node00", + true, true, "username", "password", 6, 300) + if err != nil { + assert.Equal(t, nil, err) + } + c2, err := redfish.ClientFactory("node-1", "redfish+http://nolocalhost:32201/redfish/v1/Systems/node01", + true, true, "username", "password", 6, 300) + if err != nil { + assert.Equal(t, nil, err) + } + + hosts = append(hosts, c1, c2) + + bmhInv := &mockinventory.MockBMHInventory{} + bmhInv.On("Select").Return(hosts, nil) + + inv := &mockinventory.MockInventory{} + inv.On("BaremetalInventory").Once().Return(bmhInv, nil) + + co := inventory.NewOptions(inv) + l := inventory.NewListHostsCommand(co) + buf := bytes.NewBuffer([]byte{}) + l.Writer = buf + l.OutputFormat = "yaml" + actualErr := l.RunE() + assert.Equal(t, nil, actualErr) + assert.Contains(t, buf.String(), "node-0") + assert.Contains(t, buf.String(), "node-1") + }) + + t.Run("error ListHosts", func(t *testing.T) { + expectedErr := fmt.Errorf("No hosts present in the hostInventory") + var hosts []remoteifc.Client + + bmhInv := &mockinventory.MockBMHInventory{} + bmhInv.On("Select").Return(hosts, nil) + + inv := &mockinventory.MockInventory{} + inv.On("BaremetalInventory").Once().Return(bmhInv, nil) + + co := inventory.NewOptions(inv) + l := inventory.NewListHostsCommand(co) + buf := bytes.NewBuffer([]byte{}) + l.Writer = buf + l.OutputFormat = "yaml" + actualErr := l.RunE() + assert.Equal(t, expectedErr, actualErr) + }) +} func TestCommandOptions(t *testing.T) { t.Run("error BMHAction bmh inventory", func(t *testing.T) { inv := &mockinventory.MockInventory{} diff --git a/pkg/inventory/ifc/interface.go b/pkg/inventory/ifc/interface.go index 0dc8b751a..bfa0e94a9 100644 --- a/pkg/inventory/ifc/interface.go +++ b/pkg/inventory/ifc/interface.go @@ -44,6 +44,8 @@ const ( BaremetalOperationPowerOn BaremetalOperation = "power-on" // BaremetalOperationEjectVirtualMedia eject virtual media BaremetalOperationEjectVirtualMedia BaremetalOperation = "eject-virtual-media" + // BaremetalOperationListHosts list hosts + BaremetalOperationListHosts BaremetalOperation = "list-hosts" ) // BaremetalBatchRunOptions are options to be passed to RunOperation, this is to be diff --git a/pkg/inventory/testdata/hosts.yaml b/pkg/inventory/testdata/hosts.yaml index d59696264..9bc798115 100644 --- a/pkg/inventory/testdata/hosts.yaml +++ b/pkg/inventory/testdata/hosts.yaml @@ -10,4 +10,5 @@ spec: bootMACAddress: 00:3b:8b:0c:ec:8b bmc: address: redfish+http://nolocalhost:32201/redfish/v1/Systems/ephemeral - credentialsName: node-0-bmc-secret \ No newline at end of file + credentialsName: node-0-bmc-secret +--- diff --git a/pkg/remote/ifc/ifc.go b/pkg/remote/ifc/ifc.go index 4c841eff7..05527b4f1 100644 --- a/pkg/remote/ifc/ifc.go +++ b/pkg/remote/ifc/ifc.go @@ -25,6 +25,7 @@ import ( type Client interface { EjectVirtualMedia(context.Context) error NodeID() string + NodeName() string RebootSystem(context.Context) error SetBootSourceByType(context.Context) error SystemPowerOff(context.Context) error @@ -38,7 +39,7 @@ type Client interface { } // ClientFactory is a function to be used -type ClientFactory func( +type ClientFactory func(name string, redfishURL string, insecure bool, useProxy bool, username string, password string, diff --git a/pkg/remote/redfish/client.go b/pkg/remote/redfish/client.go index 1507cbd7d..74fc1d507 100644 --- a/pkg/remote/redfish/client.go +++ b/pkg/remote/redfish/client.go @@ -36,6 +36,7 @@ const ( // Client holds details about a Redfish out-of-band system required for out-of-band management. type Client struct { nodeID string + nodeName string username string password string redfishURL string @@ -53,6 +54,11 @@ func (c *Client) NodeID() string { return c.nodeID } +// NodeName retrieves the ephemeral node ID. +func (c *Client) NodeName() string { + return c.nodeName +} + // SystemActionRetries returns number of attempts to reach host during reboot process and ejecting virtual media func (c *Client) SystemActionRetries() int { return c.systemActionRetries @@ -355,7 +361,7 @@ func RemoteDirect(ctx context.Context, isoURL, redfishURL string, c ifc.Client) } // NewClient returns a client with the capability to make Redfish requests. -func NewClient(redfishURL string, +func NewClient(nodeName string, redfishURL string, insecure bool, useProxy bool, username string, @@ -410,6 +416,7 @@ func NewClient(redfishURL string, c := &Client{ nodeID: systemID, + nodeName: nodeName, RedfishAPI: redfishClient.NewAPIClient(cfg).DefaultApi, RedfishCFG: cfg, systemActionRetries: systemActionRetries, @@ -427,13 +434,13 @@ func NewClient(redfishURL string, } // ClientFactory is a constructor for redfish ifc.Client implementation -var ClientFactory ifc.ClientFactory = func(redfishURL string, +var ClientFactory ifc.ClientFactory = func(nodeName string, redfishURL string, insecure bool, useProxy bool, username string, password string, systemActionRetries int, systemRebootDelay int) (ifc.Client, error) { - return NewClient(redfishURL, insecure, useProxy, + return NewClient(nodeName, redfishURL, insecure, useProxy, username, password, systemActionRetries, systemRebootDelay) } diff --git a/pkg/remote/redfish/client_test.go b/pkg/remote/redfish/client_test.go index 18a97234d..e4ba19985 100644 --- a/pkg/remote/redfish/client_test.go +++ b/pkg/remote/redfish/client_test.go @@ -31,6 +31,7 @@ import ( ) const ( + nodeName = "node-0" nodeID = "System.Embedded.1" isoPath = "http://localhost:8099/ubuntu-focal.iso" redfishURL = "redfish+https://localhost:2224/Systems/System.Embedded.1" @@ -39,13 +40,13 @@ const ( ) func TestNewClient(t *testing.T) { - c, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + c, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) assert.NotNil(t, c) } func TestNewClientInterface(t *testing.T) { - c, err := ClientFactory(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + c, err := ClientFactory(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) assert.NotNil(t, c) } @@ -53,7 +54,7 @@ func TestNewClientInterface(t *testing.T) { func TestNewClientDefaultValues(t *testing.T) { sysActRetr := 111 sysRebDel := 999 - c, err := NewClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel) + c, err := NewClient(nodeName, redfishURL, false, false, "", "", sysActRetr, sysRebDel) assert.Equal(t, c.systemActionRetries, sysActRetr) assert.Equal(t, c.systemRebootDelay, sysRebDel) assert.NoError(t, err) @@ -62,7 +63,7 @@ func TestNewClientDefaultValues(t *testing.T) { func TestNewClientMissingSystemID(t *testing.T) { badURL := "redfish+https://localhost:2224" - _, err := NewClient(badURL, false, false, "", "", systemActionRetries, systemRebootDelay) + _, err := NewClient(nodeName, badURL, false, false, "", "", systemActionRetries, systemRebootDelay) _, ok := err.(ErrRedfishMissingConfig) assert.True(t, ok) } @@ -70,20 +71,20 @@ func TestNewClientMissingSystemID(t *testing.T) { func TestNewClientNoRedfishMarking(t *testing.T) { url := "https://localhost:2224/Systems/System.Embedded.1" - _, err := NewClient(url, false, false, "", "", systemActionRetries, systemRebootDelay) + _, err := NewClient(nodeName, url, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) } func TestNewClientEmptyRedfishURL(t *testing.T) { // Redfish URL cannot be empty when creating a client. - _, err := NewClient("", false, false, "", "", systemActionRetries, systemRebootDelay) + _, err := NewClient(nodeName, "", false, false, "", "", systemActionRetries, systemRebootDelay) assert.Error(t, err) } func TestEjectVirtualMedia(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries+1, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries+1, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -140,7 +141,7 @@ func TestEjectVirtualMediaRetriesExceeded(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -179,7 +180,7 @@ func TestRebootSystem(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -215,7 +216,7 @@ func TestRebootSystemShutdownError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -242,7 +243,7 @@ func TestRebootSystemStartupError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -282,7 +283,7 @@ func TestRebootSystemTimeout(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -310,7 +311,7 @@ func TestSetBootSourceByTypeGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -333,7 +334,7 @@ func TestSetBootSourceByTypeSetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -366,7 +367,7 @@ func TestSetBootSourceByTypeBootSourceUnavailable(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -404,7 +405,7 @@ func TestSetVirtualMediaEjectExistingMedia(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -450,7 +451,7 @@ func TestSetVirtualMediaEjectExistingMediaFailure(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -486,7 +487,7 @@ func TestSetVirtualMediaGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -509,7 +510,7 @@ func TestSetVirtualMediaInsertVirtualMediaError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -545,7 +546,7 @@ func TestSystemPowerOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -577,7 +578,7 @@ func TestSystemPowerOffResetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -600,7 +601,7 @@ func TestSystemPowerOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -633,7 +634,7 @@ func TestSystemPowerOnResetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -656,7 +657,7 @@ func TestSystemPowerStatusUnknown(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -678,7 +679,7 @@ func TestSystemPowerStatusOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -701,7 +702,7 @@ func TestSystemPowerStatusOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -724,7 +725,7 @@ func TestSystemPowerStatusPoweringOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -747,7 +748,7 @@ func TestSystemPowerStatusPoweringOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -770,7 +771,7 @@ func TestSystemPowerStatusGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -788,7 +789,7 @@ func TestWaitForPowerStateGetSystemFailed(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -811,7 +812,7 @@ func TestWaitForPowerStateNoRetries(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -837,7 +838,7 @@ func TestWaitForPowerStateWithRetries(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -867,7 +868,7 @@ func TestWaitForPowerStateRetriesExceeded(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -898,7 +899,7 @@ func TestWaitForPowerStateDifferentPowerState(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := SetAuth(context.Background(), "", "") @@ -923,7 +924,7 @@ func TestWaitForPowerStateDifferentPowerState(t *testing.T) { func TestRemoteDirect(t *testing.T) { m := &redfishMocks.RedfishAPI{} - client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.RedfishAPI = m diff --git a/pkg/remote/redfish/vendors/dell/client.go b/pkg/remote/redfish/vendors/dell/client.go index c9f7b8648..9d2be25b8 100644 --- a/pkg/remote/redfish/vendors/dell/client.go +++ b/pkg/remote/redfish/vendors/dell/client.go @@ -135,14 +135,14 @@ func (c *Client) RemoteDirect(ctx context.Context, isoURL string) error { } // newClient returns a client with the capability to make Redfish requests. -func newClient(redfishURL string, +func newClient(nodeName string, redfishURL string, insecure bool, useProxy bool, username string, password string, systemActionRetries int, systemRebootDelay int) (*Client, error) { - genericClient, err := redfish.NewClient(redfishURL, insecure, useProxy, username, password, + genericClient, err := redfish.NewClient(nodeName, redfishURL, insecure, useProxy, username, password, systemActionRetries, systemRebootDelay) if err != nil { return nil, err @@ -154,13 +154,13 @@ func newClient(redfishURL string, } // ClientFactory is a constructor for redfish ifc.Client implementation -var ClientFactory ifc.ClientFactory = func(redfishURL string, +var ClientFactory ifc.ClientFactory = func(nodeName, redfishURL string, insecure bool, useProxy bool, username string, password string, systemActionRetries int, systemRebootDelay int) (ifc.Client, error) { - return newClient(redfishURL, insecure, useProxy, + return newClient(nodeName, redfishURL, insecure, useProxy, username, password, systemActionRetries, systemRebootDelay) } diff --git a/pkg/remote/redfish/vendors/dell/client_test.go b/pkg/remote/redfish/vendors/dell/client_test.go index 79ddacbda..4624c16e3 100644 --- a/pkg/remote/redfish/vendors/dell/client_test.go +++ b/pkg/remote/redfish/vendors/dell/client_test.go @@ -28,18 +28,19 @@ import ( ) const ( + nodeName = "node-0" redfishURL = "redfish+https://localhost/Systems/System.Embedded.1" systemActionRetries = 0 systemRebootDelay = 0 ) func TestNewClient(t *testing.T) { - _, err := newClient(redfishURL, false, false, "username", "password", systemActionRetries, systemRebootDelay) + _, err := newClient(nodeName, redfishURL, false, false, "username", "password", systemActionRetries, systemRebootDelay) assert.NoError(t, err) } func TestNewClientInterface(t *testing.T) { - c, err := ClientFactory(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + c, err := ClientFactory(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) assert.NotNil(t, c) } @@ -47,7 +48,7 @@ func TestNewClientInterface(t *testing.T) { func TestNewClientDefaultValues(t *testing.T) { sysActRetr := 222 sysRebDel := 555 - c, err := newClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel) + c, err := newClient(nodeName, redfishURL, false, false, "", "", sysActRetr, sysRebDel) assert.Equal(t, c.SystemActionRetries(), sysActRetr) assert.Equal(t, c.SystemRebootDelay(), sysRebDel) assert.NoError(t, err) @@ -56,7 +57,7 @@ func TestSetBootSourceByTypeGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - client, err := newClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := newClient(nodeName, redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) ctx := redfish.SetAuth(context.Background(), "", "") diff --git a/pkg/util/tableprinter.go b/pkg/util/tableprinter.go index c8edf0366..4fc07e8dc 100644 --- a/pkg/util/tableprinter.go +++ b/pkg/util/tableprinter.go @@ -31,6 +31,8 @@ const ( "EXECUTOR:config.executorRef.kind,DOC ENTRYPOINT:config.documentEntryPoint" // PlanListFormat is used to print tables with plan list PlanListFormat = "NAMESPACE:metadata.namespace,NAME:metadata.name,DESCRIPTION:description" + // HostListFormat is used to print tables with host list + HostListFormat = "NodeName:nodename,NodeID:nodeid" ) // PrintObjects prints suitable diff --git a/testutil/redfishutils/mock_client.go b/testutil/redfishutils/mock_client.go index a958f6675..ae590a22a 100644 --- a/testutil/redfishutils/mock_client.go +++ b/testutil/redfishutils/mock_client.go @@ -24,7 +24,8 @@ import ( // MockClient is a fake Redfish client for unit testing. type MockClient struct { mock.Mock - nodeID string + nodeID string + nodeName string } // NodeID provides a stubbed method that can be mocked to test functions that use the Redfish client without @@ -40,6 +41,19 @@ func (m *MockClient) NodeID() string { return args.String(0) } +// NodeName 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("NodeName").Return() +// +// err := client.NodeName() +func (m *MockClient) NodeName() string { + args := m.Called() + return args.String(0) +} + // EjectVirtualMedia 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. @@ -147,7 +161,7 @@ func (m *MockClient) RemoteDirect(ctx context.Context, isoURL string) error { // NewClient returns a mocked Redfish client in order to test functions that use the Redfish client without making any // Redfish API calls. -func NewClient(redfishURL string, insecure bool, useProxy bool, username string, +func NewClient(nodeName string, redfishURL string, insecure bool, useProxy bool, username string, password string) (*MockClient, error) { if redfishURL == "" { return nil, redfish.ErrRedfishMissingConfig{What: "Redfish URL"} @@ -159,6 +173,6 @@ func NewClient(redfishURL string, insecure bool, useProxy bool, username string, return nil, redfish.ErrRedfishMissingConfig{What: "management URL system ID"} } - m := &MockClient{nodeID: systemID} + m := &MockClient{nodeID: systemID, nodeName: nodeName} return m, nil }