Bug 1825418: ovirt: clean up prompts and logging

pkg/asset/installconfig/ovirt/cluster.go

@@ -29,8 +29,8 @@ func askCluster(c *ovirtsdk4.Connection, p *ovirt.Platform) (string, error) {
 		clusterNames = append(clusterNames, cluster.MustName())
 	}
 	err = survey.AskOne(&survey.Select{
-		Message: "Select the oVirt cluster",
-		Help:    "The oVirt cluster where the VMs will be created",
+		Message: "oVirt cluster",
+		Help:    "The oVirt cluster where the VMs will be created.",
 		Options: clusterNames,
 	},
 		&clusterName,

pkg/asset/installconfig/ovirt/credentials.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/url"
 
+	"github.com/sirupsen/logrus"
 	"gopkg.in/AlecAivazis/survey.v1"
 )
 
@@ -12,8 +13,8 @@ func askCredentials() (Config, error) {
 	err := survey.Ask([]*survey.Question{
 		{
 			Prompt: &survey.Input{
-				Message: "Enter oVirt's api endpoint URL",
-				Help:    "oVirt engine api url, for example https://ovirt-engine-fqdn/ovirt-engine/api",
+				Message: "oVirt API endpoint URL",
+				Help:    "The URL of the oVirt engine API. For example, https://ovirt-engine-fqdn/ovirt-engine/api.",
 			},
 			Validate: survey.ComposeValidators(survey.Required),
 		},
@@ -25,9 +26,9 @@ func askCredentials() (Config, error) {
 	var ovirtCertTrusted bool
 	err = survey.AskOne(
 		&survey.Confirm{
-			Message: "Is the installed oVirt certificate trusted?",
+			Message: "Is the oVirt CA trusted locally?",
 			Default: true,
-			Help:    "",
+			Help:    "In order to securly communicate with the oVirt engine, the certificate authority must be trusted by the local system.",
 		},
 		&ovirtCertTrusted,
 		nil)
@@ -48,21 +49,23 @@ func askCredentials() (Config, error) {
 			ovirtURL.Host)
 
 		err = survey.AskOne(&survey.Multiline{
-			Message: "Enter oVirt's CA bundle",
-			Help:    "Obtain oVirt CA bundle from " + pemURL,
+			Message: "oVirt certificate bundle",
+			Help:    fmt.Sprintf("The oVirt certificate bundle can be downloaded from %s.", pemURL),
 		},
 			&c.CABundle,
 			survey.ComposeValidators(survey.Required))
 		if err != nil {
 			return c, err
 		}
+	} else {
+		logrus.Warning("Communication with the oVirt engine will be insecure.")
 	}
 
 	err = survey.Ask([]*survey.Question{
 		{
 			Prompt: &survey.Input{
-				Message: "Enter ovirt-engine username",
-				Help:    "The user must have permissions to create VMs and disks on the Storage Domain with the same name as the OpenShift cluster",
+				Message: "oVirt engine username",
+				Help:    "The user must have permissions to create VMs and disks on the Storage Domain with the same name as the OpenShift cluster.",
 				Default: "admin@internal",
 			},
 			Validate: survey.ComposeValidators(survey.Required),
@@ -75,7 +78,7 @@ func askCredentials() (Config, error) {
 	err = survey.Ask([]*survey.Question{
 		{
 			Prompt: &survey.Password{
-				Message: "Enter password",
+				Message: "oVirt engine password",
 				Help:    "",
 			},
 			Validate: survey.ComposeValidators(survey.Required, authenticated(&c)),

pkg/asset/installconfig/ovirt/network.go

@@ -29,9 +29,8 @@ func askNetwork(c *ovirtsdk4.Connection, p *ovirt.Platform) error {
 		networkNames = append(networkNames, network.MustName())
 	}
 	err = survey.AskOne(&survey.Select{
-		Message: "Select the oVirt network",
-		Help: "The oVirt network of the deployed VMs. 'ovirtmgmt' is the default network - it is recommended " +
-			"to work with a dedicated network per OpenShift cluster",
+		Message: "oVirt network",
+		Help:    "The oVirt network of the deployed VMs. 'ovirtmgmt' is the default network. It is recommended to use a dedicated network for each OpenShift cluster.",
 		Options: networkNames,
 	},
 		&networkName,

pkg/asset/installconfig/ovirt/ovirt.go

@@ -55,8 +55,8 @@ func Platform() (*ovirt.Platform, error) {
 	err = survey.Ask([]*survey.Question{
 		{
 			Prompt: &survey.Input{
-				Message: "Enter the internal API Virtual IP",
-				Help:    "Make sure the IP address is not in use",
+				Message: "Internal API virtual IP",
+				Help:    "This is the virtual IP address that will be used to address the OpenShift control plane. Make sure the IP address is not in use.",
 				Default: "",
 			},
 			Validate: survey.ComposeValidators(survey.Required),
@@ -69,8 +69,8 @@ func Platform() (*ovirt.Platform, error) {
 	err = survey.Ask([]*survey.Question{
 		{
 			Prompt: &survey.Input{
-				Message: "Enter the internal DNS Virtual IP",
-				Help:    "Make sure the IP address is not in use",
+				Message: "Internal DNS virtual IP",
+				Help:    "This is the virtual IP address that will be used to address the DNS server internal to the cluster. Make sure the IP address is not in use.",
 				Default: "",
 			},
 			Validate: survey.ComposeValidators(survey.Required),
@@ -83,8 +83,8 @@ func Platform() (*ovirt.Platform, error) {
 	err = survey.Ask([]*survey.Question{
 		{
 			Prompt: &survey.Input{
-				Message: "Enter the ingress IP ",
-				Help:    "Make sure the IP address is not in use",
+				Message: "Ingress virtual IP",
+				Help:    "This is the virtual IP address that will be used to address the OpenShift ingress routers. Make sure the IP address is not in use.",
 				Default: "",
 			},
 			Validate: survey.ComposeValidators(survey.Required),

pkg/asset/installconfig/ovirt/storage.go

@@ -29,8 +29,8 @@ func askStorage(c *ovirtsdk4.Connection, p *ovirt.Platform, clusterName string)
 		domainNames = append(domainNames, domain.MustName())
 	}
 	err = survey.AskOne(&survey.Select{
-		Message: "Select the oVirt storage domain",
-		Help:    "The storage domain will be used to create the disks of all the cluster nodes",
+		Message: "oVirt storage domain",
+		Help:    "The storage domain will be used to create the disks of all the cluster nodes.",
 		Options: domainNames,
 	},
 		&storageDomainName,

pkg/destroy/ovirt/destroyer.go

@@ -33,27 +33,31 @@ func (uninstaller *ClusterUninstaller) Run() error {
 	}
 	defer con.Close()
 
-	err = uninstaller.removeVms(con)
-	uninstaller.Logger.Errorf("Removing VMs - error: %s", err)
-	err = uninstaller.removeTag(con)
-	uninstaller.Logger.Errorf("Removing Tag - error: %s", err)
-	err = uninstaller.removeTemplate(con)
-	uninstaller.Logger.Errorf("Removing Template - error: %s", err)
+	if err := uninstaller.removeVMs(con); err != nil {
+		uninstaller.Logger.Errorf("Failed to remove VMs: %s", err)
+	}
+	if err := uninstaller.removeTag(con); err != nil {
+		uninstaller.Logger.Errorf("Failed to remove tag: %s", err)
+	}
+	if err := uninstaller.removeTemplate(con); err != nil {
+		uninstaller.Logger.Errorf("Failed to remove template: %s", err)
+	}
+
 	return nil
 }
 
-func (uninstaller *ClusterUninstaller) removeVms(con *ovirtsdk.Connection) error {
+func (uninstaller *ClusterUninstaller) removeVMs(con *ovirtsdk.Connection) error {
 	// - find all vms by tag name=infraID
 	vmsService := con.SystemService().VmsService()
 	searchTerm := fmt.Sprintf("tag=%s", uninstaller.Metadata.InfraID)
-	uninstaller.Logger.Infof("searching VMs by %s", searchTerm)
+	uninstaller.Logger.Debugf("Searching VMs by %s", searchTerm)
 	vmsResponse, err := vmsService.List().Search(searchTerm).Send()
 	if err != nil {
 		return err
 	}
 	// - stop + delete VMS
 	vms := vmsResponse.MustVms().Slice()
-	uninstaller.Logger.Infof("Found %s VMs", len(vms))
+	uninstaller.Logger.Debugf("Found %d VMs", len(vms))
 	wg := sync.WaitGroup{}
 	wg.Add(len(vms))
 	for _, vm := range vms {
@@ -77,10 +81,9 @@ func (uninstaller *ClusterUninstaller) removeTag(con *ovirtsdk.Connection) error
 	if tagsServiceListResponse != nil {
 		for _, t := range tagsServiceListResponse.MustTags().Slice() {
 			if t.MustName() == uninstaller.Metadata.InfraID {
+				uninstaller.Logger.Infof("Removing tag %s", t.MustName())
 				_, err := tagsService.TagService(t.MustId()).Remove().Send()
-				uninstaller.Logger.Infof("Removing tag %s : %s", t.MustName(), "errors: %s", err)
 				if err != nil {
-					uninstaller.Logger.Debugf("Failed removing tag %s : %s", t.MustName(), err)
 					return err
 				}
 			}
@@ -93,23 +96,27 @@ func (uninstaller *ClusterUninstaller) stopVM(vmsService *ovirtsdk.VmsService, v
 	vmService := vmsService.VmService(vm.MustId())
 	// this is a teardown, stopping instead of shutting down.
 	_, err := vmService.Stop().Send()
-	uninstaller.Logger.Infof("Stopping VM %s : %s", vm.MustName(), "errors: %s", err)
-	if err != nil {
-		uninstaller.Logger.Debugf("Failed stopping VM %s : %s", vm.MustName(), err)
+	if err == nil {
+		uninstaller.Logger.Infof("Stopping VM %s", vm.MustName())
+	} else {
+		uninstaller.Logger.Errorf("Failed to stop VM %s: %s", vm.MustName(), err)
 	}
 	waitForDownDuration := time.Minute * 10
 	err = vmService.Connection().WaitForVM(vm.MustId(), ovirtsdk.VMSTATUS_DOWN, waitForDownDuration)
-	if err != nil {
-		uninstaller.Logger.Warnf("Waiting %d for VM %s to power-off", waitForDownDuration, vm.MustName())
+	if err == nil {
+		uninstaller.Logger.Infof("VM %s powered off", vm.MustName())
+	} else {
+		uninstaller.Logger.Warnf("Waited %d for VM %s to power off: %s", waitForDownDuration, vm.MustName(), err)
 	}
 }
 
 func (uninstaller *ClusterUninstaller) removeVM(vmsService *ovirtsdk.VmsService, vm *ovirtsdk.Vm) {
 	vmService := vmsService.VmService(vm.MustId())
 	_, err := vmService.Remove().Send()
-	uninstaller.Logger.Infof("Removing VM %s : %s", vm.MustName(), "errors: %s", err)
-	if err != nil {
-		uninstaller.Logger.Debugf("Failed removing VM %s : %s", vm.MustName(), err)
+	if err == nil {
+		uninstaller.Logger.Infof("Removing VM %s", vm.MustName())
+	} else {
+		uninstaller.Logger.Errorf("Failed to remove VM %s: %s", vm.MustName(), err)
 	}
 }
 
@@ -118,8 +125,7 @@ func (uninstaller *ClusterUninstaller) removeTemplate(con *ovirtsdk.Connection)
 		search, err := con.SystemService().TemplatesService().
 			List().Search(fmt.Sprintf("name=%s", uninstaller.Metadata.InfraID)).Send()
 		if err != nil {
-			uninstaller.Logger.Errorf("Couldn't find a template with name %s", uninstaller.Metadata.InfraID)
-			return nil
+			return fmt.Errorf("couldn't find a template with name %s", uninstaller.Metadata.InfraID)
 		}
 		if result, ok := search.Templates(); ok {
 			// the results can potentially return a list of template
@@ -129,7 +135,6 @@ func (uninstaller *ClusterUninstaller) removeTemplate(con *ovirtsdk.Connection)
 				service := con.SystemService().TemplatesService().TemplateService(tmp.MustId())
 				_, err := service.Remove().Send()
 				if err != nil {
-					uninstaller.Logger.Errorf("Failed to remove Template %s %s", tmp.MustName(), tmp.MustId())
 					return err
 				}
 			}