Make HostCloudRegion optional for non bootstrapping process on cmd level

sdg0919 · May 24, 2019 · b91dfb2 · b91dfb2
1 parent e101e1f
commit b91dfb2
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 31 deletions.
diff --git a/caas/kubernetes/provider/bootstrap.go b/caas/kubernetes/provider/bootstrap.go
@@ -348,7 +348,10 @@ func (c *controllerStack) getControllerSvcSpec(cloudType string) (*controllerSer
 
 func (c *controllerStack) createControllerService() error {
 	svcName := c.resourceNameService
-	cloudType, _ := cloud.SplitHostCloudRegion(c.pcfg.Bootstrap.ControllerCloud.HostCloudRegion)
+	cloudType, _, err := cloud.SplitHostCloudRegion(c.pcfg.Bootstrap.ControllerCloud.HostCloudRegion)
+	if err != nil {
+		return errors.Trace(err)
+	}
 	controllerSvcSpec, err := c.getControllerSvcSpec(cloudType)
 	if err != nil {
 		return errors.Trace(err)

diff --git a/caas/kubernetes/provider/cloud.go b/caas/kubernetes/provider/cloud.go
@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"reflect"
-	"strings"
 
 	"github.com/juju/errors"
 	"github.com/juju/utils"
@@ -41,6 +40,7 @@ type KubeCloudParams struct {
 type KubeCloudStorageParams struct {
 	WorkloadStorage        string
 	HostCloudRegion        string
+	IsBootstrap            bool
 	MetadataChecker        caas.ClusterMetadataChecker
 	GetClusterMetadataFunc GetClusterMetadataFunc
 }
@@ -115,22 +115,19 @@ func UpdateKubeCloudWithStorage(k8sCloud *cloud.Cloud, storageParams KubeCloudSt
 			clusterMetadata.Regions.SortedValues()[0],
 		)
 	}
-	if storageParams.HostCloudRegion == "" {
-		return fail(ClusterQueryError{})
-	}
-	_, region, err := ParseCloudRegion(storageParams.HostCloudRegion)
-	if err != nil {
-		return fail(errors.Annotatef(err, "validating cloud region %q", storageParams.HostCloudRegion))
+
+	cloudType, region, err := cloud.SplitHostCloudRegion(storageParams.HostCloudRegion)
+	if err != nil && storageParams.IsBootstrap {
+		// a valid host cloud region is required for bootstrapping to k8s.
+		return fail(errors.New("host cloud region is required for bootstrapping to k8s"))
 	}
 	k8sCloud.HostCloudRegion = storageParams.HostCloudRegion
 	k8sCloud.Regions = []cloud.Region{{
 		Name: region,
 	}}
 
 	// If the user has not specified storage, check that the cluster has Juju's opinionated defaults.
-	cloudType, _ := cloud.SplitHostCloudRegion(storageParams.HostCloudRegion)
 	err = storageParams.MetadataChecker.CheckDefaultWorkloadStorage(cloudType, clusterMetadata.NominatedStorageClass)
-
 	if storageParams.WorkloadStorage == "" {
 		if errors.IsNotFound(err) {
 			return fail(UnknownClusterError{CloudName: cloudType})
@@ -207,15 +204,6 @@ func UpdateKubeCloudWithStorage(k8sCloud *cloud.Cloud, storageParams KubeCloudSt
 	return storageMsg, nil
 }
 
-// ParseCloudRegion breaks apart a clusters cloud region.
-func ParseCloudRegion(cloudRegion string) (string, string, error) {
-	fields := strings.SplitN(cloudRegion, "/", 2)
-	if len(fields) != 2 || fields[0] == "" || fields[1] == "" {
-		return "", "", errors.NotValidf("cloud region %q", cloudRegion)
-	}
-	return fields[0], fields[1], nil
-}
-
 // BaseKubeCloudOpenParams provides a basic OpenParams for a cluster
 func BaseKubeCloudOpenParams(cloud cloud.Cloud, credential cloud.Credential) (environs.OpenParams, error) {
 	// To get a k8s client, we need a config with minimal information.
@@ -276,6 +264,7 @@ func (p kubernetesEnvironProvider) FinalizeCloud(ctx environs.FinalizeCloudConte
 		return cloud.Cloud{}, errors.Trace(err)
 	}
 	storageUpdateParams := KubeCloudStorageParams{
+		IsBootstrap:     true,
 		MetadataChecker: broker,
 		GetClusterMetadataFunc: func(broker caas.ClusterMetadataChecker) (*caas.ClusterMetadata, error) {
 			clusterMetadata, err := broker.GetClusterMetadata("")

diff --git a/caas/kubernetes/provider/metadata.go b/caas/kubernetes/provider/metadata.go
@@ -122,21 +122,22 @@ func (k *kubernetesClient) GetClusterMetadata(storageClass string) (*caas.Cluste
 	}
 
 	// We may have the workload storage but still need to look for operator storage.
-	preferredOperatorStorage, havePreferredOperatorStorage := jujuPreferredOperatorStorage[result.Cloud]
 	storageClasses, err := k.client().StorageV1().StorageClasses().List(v1.ListOptions{})
 	if err != nil {
 		return nil, errors.Annotate(err, "listing storage classes")
 	}
 
 	if discoverCDKStoragePreferences(storageClasses.Items, &result) {
+		// This is a CDK cluster with preferred storage class defined.
 		return &result, nil
 	}
 
 	var possibleWorkloadStorage, possibleOperatorStorage []*caas.StorageProvisioner
+	preferredOperatorStorage, hasPreferredOperatorStorage := jujuPreferredOperatorStorage[result.Cloud]
 	for _, sc := range storageClasses.Items {
 		caasSC := caasStorageProvisioner(sc)
 		isDefaultSc := isDefaultStorageClass(sc)
-		if result.OperatorStorageClass == nil && havePreferredOperatorStorage {
+		if result.OperatorStorageClass == nil && hasPreferredOperatorStorage {
 			if err := storageClassMatches(preferredOperatorStorage, caasSC); err == nil {
 				if isDefaultSc {
 					// Prefer operator storage from the default storage class.
@@ -193,10 +194,10 @@ func (k *kubernetesClient) listHostCloudRegions() (string, set.Strings, error) {
 }
 
 // CheckDefaultWorkloadStorage implements ClusterMetadataChecker.
-func (k *kubernetesClient) CheckDefaultWorkloadStorage(cluster string, storageProvisioner *caas.StorageProvisioner) error {
-	preferredStorage, ok := jujuPreferredWorkloadStorage[cluster]
+func (k *kubernetesClient) CheckDefaultWorkloadStorage(cloudType string, storageProvisioner *caas.StorageProvisioner) error {
+	preferredStorage, ok := jujuPreferredWorkloadStorage[cloudType]
 	if !ok {
-		return errors.NotFoundf("cluster %q", cluster)
+		return errors.NotFoundf("preferred workload storage for cloudType %q", cloudType)
 	}
 	return storageClassMatches(preferredStorage, storageProvisioner)
 }

diff --git a/cloud/clouds.go b/cloud/clouds.go
@@ -168,12 +168,12 @@ type Cloud struct {
 }
 
 // SplitHostCloudRegion splits host cloud region to cloudType and region.
-func SplitHostCloudRegion(hostCloudRegion string) (string, string) {
-	r := strings.Split(hostCloudRegion, "/")
-	if len(r) >= 2 {
-		return r[0], r[1]
+func SplitHostCloudRegion(hostCloudRegion string) (string, string, error) {
+	fields := strings.SplitN(hostCloudRegion, "/", 2)
+	if len(fields) != 2 || fields[0] == "" || fields[1] == "" {
+		return "", "", errors.NotValidf("cloud region %q", hostCloudRegion)
 	}
-	return r[0], ""
+	return fields[0], fields[1], nil
 }
 
 // BuildHostCloudRegion combines cloudType with region to host cloud region.

diff --git a/cmd/juju/caas/add.go b/cmd/juju/caas/add.go
@@ -379,6 +379,7 @@ func (c *AddCAASCommand) Run(ctx *cmd.Context) error {
 	storageParams := provider.KubeCloudStorageParams{
 		WorkloadStorage:        c.workloadStorage,
 		HostCloudRegion:        c.hostCloudRegion,
+		IsBootstrap:			c.Local,
 		MetadataChecker:        broker,
 		GetClusterMetadataFunc: c.getClusterMetadataFunc(ctx),
 	}
@@ -471,7 +472,7 @@ func (c *AddCAASCommand) newK8sClusterBroker(cloud jujucloud.Cloud, credential j
 func (c *AddCAASCommand) validateCloudRegion(cloudRegion string) (_ string, err error) {
 	defer errors.DeferredAnnotatef(&err, "validating cloud region %q", cloudRegion)
 
-	cloudNameOrType, region, err := provider.ParseCloudRegion(cloudRegion)
+	cloudNameOrType, region, err := jujucloud.SplitHostCloudRegion(cloudRegion)
 	if err != nil {
 		return "", errors.Annotate(err, "parsing cloud region")
 	}
@@ -514,8 +515,9 @@ func (c *AddCAASCommand) getClusterMetadataFunc(ctx *cmd.Context) provider.GetCl
 			clusterMetadata, err := broker.GetClusterMetadata(c.workloadStorage)
 			if err != nil {
 				errChan <- err
+			} else {
+				result <- clusterMetadata
 			}
-			result <- clusterMetadata
 		}()
 
 		timeout := 30 * time.Second