Improve the liqoctl install command

cmd/liqoctl/cmd/install.go

@@ -122,7 +122,7 @@ func newInstallCommand(ctx context.Context, f *factory.Factory) *cobra.Command {
 
 	cmd.PersistentFlags().StringVar(&options.Version, "version", "",
 		"The version of Liqo to be installed, among releases and commit SHAs. Defaults to the latest stable release")
-	cmd.PersistentFlags().StringVar(&options.RepoURL, "repo-url", "https://github.com/liqotech/liqo.git",
+	cmd.PersistentFlags().StringVar(&options.RepoURL, "repo-url", "https://github.com/liqotech/liqo",
 		"The URL of the git repository used to retrieve the Helm chart, if a non released version is specified")
 	cmd.PersistentFlags().StringVar(&options.ChartPath, "local-chart-path", "",
 		"The local path used to retrieve the Helm chart, instead of the upstream one")

cmd/liqoctl/main.go

@@ -19,7 +19,6 @@ import (
 	"os"
 	"os/signal"
 	"syscall"
-	"time"
 
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -32,10 +31,6 @@ import (
 	liqocmd "github.com/liqotech/liqo/cmd/liqoctl/cmd"
 )
 
-const (
-	terminationTimeout = 5 * time.Second
-)
-
 func init() {
 	utilruntime.Must(discoveryv1alpha1.AddToScheme(scheme.Scheme))
 	utilruntime.Must(netv1alpha1.AddToScheme(scheme.Scheme))
@@ -48,10 +43,7 @@ func main() {
 
 	go func() {
 		<-ctx.Done()
-
 		stop() // Reset the default behavior if further signals are received.
-		<-time.After(terminationTimeout)
-		os.Exit(1)
 	}()
 
 	cmd := liqocmd.NewRootCommand(ctx)

docs/installation/install.md

@@ -300,6 +300,9 @@ Additionally, **arbitrary parameters** available in the Helm *values* file (the
 The main global flags, besides those concerning the installation of [development versions](InstallationDevelopmentVersions), include:
 
 * `--enable-ha`: whether to enable the support for **high-availability of the Liqo components**, starting two replicas (in an active/standby configuration) of the **gateway** to ensure no cross-cluster connectivity downtime in case one of the replicas is restarted, as well as of the **controller manager**, which embeds the Liqo control plane logic.
+* `--timeout`: configures the timeout for the completion of the installation/upgrade process.
+  Once expired, the process is aborted and Liqo is rolled back to the previous version.
+* `--verbose`: whether to enable verbose logs, providing additional information concerning the installation/upgrade process (i.e., for troubleshooting purposes).
 
 ### Control plane
 

go.mod

@@ -17,9 +17,10 @@ require (
 	github.com/gruntwork-io/terratest v0.40.7
 	github.com/jinzhu/copier v0.3.5
 	github.com/julienschmidt/httprouter v1.3.0
+	github.com/mattn/go-isatty v0.0.14
 	github.com/metal-stack/go-ipam v1.8.6
 	github.com/miekg/dns v1.1.47
-	github.com/mittwald/go-helm-client v0.8.3-0.20211026133933-a26889186afc
+	github.com/mittwald/go-helm-client v0.11.1
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.19.0
 	github.com/openshift/api v0.0.0-20210521075222-e273a339932a
@@ -58,7 +59,6 @@ require (
 	sigs.k8s.io/aws-iam-authenticator v0.5.7
 	sigs.k8s.io/controller-runtime v0.11.2
 	sigs.k8s.io/sig-storage-lib-external-provisioner/v7 v7.0.1
-	sigs.k8s.io/yaml v1.3.0
 )
 
 require (
@@ -161,7 +161,6 @@ require (
 	github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/mattn/go-colorable v0.1.12 // indirect
-	github.com/mattn/go-isatty v0.0.14 // indirect
 	github.com/mattn/go-runewidth v0.0.13 // indirect
 	github.com/mattn/go-zglob v0.0.3 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
@@ -233,8 +232,9 @@ require (
 	sigs.k8s.io/kustomize/api v0.11.4 // indirect
 	sigs.k8s.io/kustomize/kyaml v0.13.6 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect
+	sigs.k8s.io/yaml v1.3.0 // indirect
 )
 
 replace github.com/grandcat/zeroconf => github.com/liqotech/zeroconf v1.0.1-0.20201020081245-6384f3f21ffb
 
-replace github.com/mittwald/go-helm-client v0.8.3-0.20211026133933-a26889186afc => github.com/liqotech/go-helm-client v0.8.3-0.20211106110643-962712c10b2d
+replace github.com/mittwald/go-helm-client v0.11.1 => github.com/liqotech/go-helm-client v0.11.1-liqo

go.sum

@@ -1041,8 +1041,8 @@ github.com/lib/pq v1.10.4/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
 github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhnIaL+V+BEER86oLrvS+kWobKpbJuye0=
 github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9B/r2mtpb6U+EI2rYA5OAXxsYw6wTamcNW+zcE=
 github.com/linuxkit/virtsock v0.0.0-20201010232012-f8cee7dfc7a3/go.mod h1:3r6x7q95whyfWQpmGZTu3gk3v2YkMi05HEzl7Tf7YEo=
-github.com/liqotech/go-helm-client v0.8.3-0.20211106110643-962712c10b2d h1:JQfdDRGLiFL9+EzX8kj/BvSlntIWsSgDYznN5dfmmns=
-github.com/liqotech/go-helm-client v0.8.3-0.20211106110643-962712c10b2d/go.mod h1:DVtyXr7KDPIT44X320DkxF39nfbZn8KdjvibnN7iHnQ=
+github.com/liqotech/go-helm-client v0.11.1-liqo h1:dzLpNptHpVWNj0vYNGLs63CIsmqfic4e1AG4LsJXIQA=
+github.com/liqotech/go-helm-client v0.11.1-liqo/go.mod h1:Zs+xgOODq1/ILHj61foCI07EBNB7IoBHaW+VjJN2bcU=
 github.com/liqotech/zeroconf v1.0.1-0.20201020081245-6384f3f21ffb h1:FokBo/2VZRYP72Y030Z5bQXiIr7gs/nt/ISqnuCsiq0=
 github.com/liqotech/zeroconf v1.0.1-0.20201020081245-6384f3f21ffb/go.mod h1:lTKmG1zh86XyCoUeIHSA4FJMBwCJiQmGfcP2PdzytEs=
 github.com/lithammer/dedent v1.1.0/go.mod h1:jrXYCQtgg0nJiN+StA2KgR7w6CiQNv9Fd/Z9BP0jIOc=

pkg/liqoctl/factory/output.go

@@ -20,11 +20,19 @@ import (
 	"strings"
 	"time"
 
+	"github.com/mattn/go-isatty"
 	"github.com/pterm/pterm"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/kubectl/pkg/cmd/util"
 )
 
+func init() {
+	// Disable styling if we are not in a standard terminal, as control sequences would not work.
+	if !isatty.IsTerminal(os.Stdout.Fd()) {
+		pterm.DisableStyling()
+	}
+}
+
 const (
 	localClusterName  = "local"
 	remoteClusterName = "remote"
@@ -56,7 +64,7 @@ func (p *Printer) StartSpinner(text ...interface{}) *pterm.SpinnerPrinter {
 // Verbosef outputs verbose messages guarded by the corresponding flag.
 func (p *Printer) Verbosef(format string, args ...interface{}) {
 	if p.verbose {
-		p.Info.Printf(format, args...)
+		p.Info.Printfln(strings.TrimRight(format, "\n"), args...)
 	}
 }
 

pkg/liqoctl/install/handler.go

@@ -16,21 +16,21 @@ package install
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
+	"strings"
 	"time"
 
 	helm "github.com/mittwald/go-helm-client"
+	"github.com/pterm/pterm"
 	"github.com/spf13/cobra"
 	"golang.org/x/mod/semver"
 	"gopkg.in/yaml.v3"
 	"helm.sh/helm/pkg/strvals"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/repo"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	k8syaml "sigs.k8s.io/yaml"
 
 	"github.com/liqotech/liqo/pkg/consts"
 	"github.com/liqotech/liqo/pkg/liqoctl/factory"
@@ -156,10 +156,19 @@ func (o *Options) Run(ctx context.Context, provider Provider) error {
 		return nil
 	}
 
-	s = o.Printer.StartSpinner("Installing or upgrading Liqo... (this may take few minutes)")
-	err = o.installOrUpdate(ctx, string(rawValues))
+	s = o.Printer.StartSpinner("Installing or upgrading Liqo... (this may take a few minutes)")
+	err = o.installOrUpdate(ctx, string(rawValues), s)
 	if err != nil {
-		s.Fail("Error installing or upgrading Liqo: ", err)
+		msg := strings.Replace(err.Error(), context.DeadlineExceeded.Error(), "timed out waiting for the condition", 1)
+		s.Fail("Error installing or upgrading Liqo: ", msg)
+		if strings.Contains(msg, "timed out waiting for the condition") {
+			o.Printer.Info.Println("Likely causes for the installation/upgrade timeout could include:")
+			o.Printer.Info.Println("* One or more pods failed to start (e.g., they are in the ImagePullBackOff status)")
+			o.Printer.Info.Println("* A service of type LoadBalancer has been configured, but no provider is available")
+			o.Printer.Info.Println("You can add the --verbose flag for debug information concerning the failing resources")
+			o.Printer.Info.Println("Additionally, if necessary, you can increase the timeout value with the --timeout flag")
+		}
+
 		return err
 	}
 
@@ -199,8 +208,12 @@ func (o *Options) initialize(ctx context.Context, provider Provider) error {
 
 	// Otherwise, clone the repository and configure it as a local chart path.
 	default:
-		o.Printer.Warning.Printfln("Non-released version selected. Downloading repository...")
+		o.Printer.Warning.Printfln("Non-released version selected. Downloading repository from %q...", o.RepoURL)
 		if err = o.cloneRepository(ctx); err != nil {
+			if strings.Contains(err.Error(), "object not found") {
+				return errors.New("commit not found. Did you select the correct repository?")
+			}
+
 			return err
 		}
 	}
@@ -212,6 +225,10 @@ func (o *Options) initialize(ctx context.Context, provider Provider) error {
 		return err
 	}
 	o.chartValues = chart.Values
+	// Explicitly set the version to the retrieved one, if not previously set.
+	if o.Version == "" {
+		o.Version = chart.Metadata.Version
+	}
 
 	// Retrieve the cluster name used for previous installations, in case it was not specified.
 	if o.ClusterName == "" {
@@ -229,62 +246,37 @@ func (o *Options) initialize(ctx context.Context, provider Provider) error {
 	return nil
 }
 
-func (o *Options) installOrUpdate(ctx context.Context, rawValues string) error {
+func (o *Options) installOrUpdate(ctx context.Context, rawValues string, s *pterm.SpinnerPrinter) error {
 	chartSpec := helm.ChartSpec{
 		ReleaseName: LiqoReleaseName,
 		ChartName:   o.ChartPath,
 		Version:     o.Version,
 
 		Namespace:       o.LiqoNamespace,
 		CreateNamespace: true,
+		UpgradeCRDs:     true,
 		ValuesYaml:      rawValues,
 
-		Timeout: o.Timeout,
-		DryRun:  o.DryRun,
-		Wait:    true,
+		Timeout:       o.Timeout,
+		DryRun:        o.DryRun,
+		Atomic:        true,
+		Wait:          true,
+		CleanupOnFail: true,
 	}
 
-	// install or update CRDs
-	chart, _, err := o.HelmClient().GetChart(o.ChartPath, &action.ChartPathOptions{Version: o.Version})
-	if err != nil {
-		return fmt.Errorf("unable to get the helm chart: %w", err)
-	}
-	crds := chart.CRDObjects()
-	for i := range crds {
-		crdObj := apiextensionsv1.CustomResourceDefinition{}
-		if err = k8syaml.Unmarshal(crds[i].File.Data, &crdObj); err != nil {
-			return fmt.Errorf("unable to unmarshal CRD yaml file %q: %w", crds[i].File.Name, err)
-		}
-		err = o.CRClient.Create(ctx, &crdObj)
-		switch {
-		case apierrors.IsAlreadyExists(err):
-			var existingCrd apiextensionsv1.CustomResourceDefinition
-			if err = o.CRClient.Get(ctx, client.ObjectKeyFromObject(&crdObj), &existingCrd); err != nil {
-				return fmt.Errorf("unable to get CRD %q: %w", crdObj.Name, err)
-			}
-			existingCrd.Spec = *crdObj.Spec.DeepCopy()
-			if err = o.CRClient.Update(ctx, &existingCrd); err != nil {
-				return fmt.Errorf("unable to update CRD %q: %w", crdObj.Name, err)
-			}
-		case err != nil:
-			return fmt.Errorf("unable to create CRD %q: %w", crdObj.Name, err)
-		}
-	}
-
-	// provide the possibility to exit installation on context cancellation
-	errCh := make(chan error)
-	defer close(errCh)
+	// Update the text in case the parent context is canceled, to give a feedback that it was considered.
+	ctxp, cancel := context.WithCancel(ctx)
+	defer cancel()
 	go func() {
-		_, err := o.HelmClient().InstallOrUpgradeChart(ctx, &chartSpec)
-		errCh <- err
+		<-ctxp.Done()
+		if s != nil {
+			s.UpdateText("Operation canceled: rolling back...")
+		}
 	}()
 
-	select {
-	case err := <-errCh:
-		return err
-	case <-ctx.Done():
-		return ctx.Err()
-	}
+	_, err := o.HelmClient().InstallOrUpgradeChart(ctx, &chartSpec, nil)
+	s = nil // Do not print the message in case installation/upgrade succeeded.
+	return err
 }
 
 func (o *Options) isRelease() bool {